Fix populateWithDelChunkSeriesIterator and gauge histograms (#12330)

Use AppendableGauge to detect corrupt chunk with gauge histograms.
Detect if first sample is a gauge but the chunk is not set up to contain
gauge histograms.

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Signed-off-by: George Krajcsovits <krajorama@users.noreply.github.com>
This commit is contained in:
George Krajcsovits 2023-05-19 10:24:06 +02:00 committed by GitHub
parent f731a90a7f
commit 92d6980360
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 404 additions and 39 deletions

View file

@ -331,7 +331,7 @@ func (h *headChunkReader) chunk(meta chunks.Meta, copyLastChunk bool) (chunkenc.
}
s.Unlock()
return &safeChunk{
return &safeHeadChunk{
Chunk: chk,
s: s,
cid: cid,
@ -627,15 +627,15 @@ func (b boundedIterator) Seek(t int64) chunkenc.ValueType {
return b.Iterator.Seek(t)
}
// safeChunk makes sure that the chunk can be accessed without a race condition
type safeChunk struct {
// safeHeadChunk makes sure that the chunk can be accessed without a race condition
type safeHeadChunk struct {
chunkenc.Chunk
s *memSeries
cid chunks.HeadChunkID
isoState *isolationState
}
func (c *safeChunk) Iterator(reuseIter chunkenc.Iterator) chunkenc.Iterator {
func (c *safeHeadChunk) Iterator(reuseIter chunkenc.Iterator) chunkenc.Iterator {
c.s.Lock()
it := c.s.iterator(c.cid, c.Chunk, c.isoState, reuseIter)
c.s.Unlock()

View file

@ -785,14 +785,35 @@ func (p *populateWithDelChunkSeriesIterator) Next() bool {
if app, err = newChunk.Appender(); err != nil {
break
}
if hc, ok := p.currChkMeta.Chunk.(*chunkenc.HistogramChunk); ok {
switch hc := p.currChkMeta.Chunk.(type) {
case *chunkenc.HistogramChunk:
newChunk.(*chunkenc.HistogramChunk).SetCounterResetHeader(hc.GetCounterResetHeader())
case *safeHeadChunk:
if unwrapped, ok := hc.Chunk.(*chunkenc.HistogramChunk); ok {
newChunk.(*chunkenc.HistogramChunk).SetCounterResetHeader(unwrapped.GetCounterResetHeader())
} else {
err = fmt.Errorf("internal error, could not unwrap safeHeadChunk to histogram chunk: %T", hc.Chunk)
}
default:
err = fmt.Errorf("internal error, unknown chunk type %T when expecting histogram", p.currChkMeta.Chunk)
}
if err != nil {
break
}
var h *histogram.Histogram
t, h = p.currDelIter.AtHistogram()
p.curr.MinTime = t
// Detect missing gauge reset hint.
if h.CounterResetHint == histogram.GaugeType && newChunk.(*chunkenc.HistogramChunk).GetCounterResetHeader() != chunkenc.GaugeType {
err = fmt.Errorf("found gauge histogram in non gauge chunk")
break
}
app.AppendHistogram(t, h)
for vt := p.currDelIter.Next(); vt != chunkenc.ValNone; vt = p.currDelIter.Next() {
if vt != chunkenc.ValHistogram {
err = fmt.Errorf("found value type %v in histogram chunk", vt)
@ -801,6 +822,20 @@ func (p *populateWithDelChunkSeriesIterator) Next() bool {
t, h = p.currDelIter.AtHistogram()
// Defend against corrupted chunks.
if h.CounterResetHint == histogram.GaugeType {
pI, nI, bpI, bnI, _, _, okToAppend := app.(*chunkenc.HistogramAppender).AppendableGauge(h)
if !okToAppend {
err = errors.New("unable to append histogram due to unexpected schema change")
break
}
if len(pI)+len(nI)+len(bpI)+len(bnI) > 0 {
err = fmt.Errorf(
"bucket layout has changed unexpectedly: forward %d positive, %d negative, backward %d positive %d negative bucket interjections required",
len(pI), len(nI), len(bpI), len(bnI),
)
break
}
} else {
pI, nI, okToAppend, counterReset := app.(*chunkenc.HistogramAppender).Appendable(h)
if len(pI)+len(nI) > 0 {
err = fmt.Errorf(
@ -817,7 +852,7 @@ func (p *populateWithDelChunkSeriesIterator) Next() bool {
err = errors.New("unable to append histogram due to unexpected schema change")
break
}
}
app.AppendHistogram(t, h)
}
case chunkenc.ValFloat:
@ -842,14 +877,35 @@ func (p *populateWithDelChunkSeriesIterator) Next() bool {
if app, err = newChunk.Appender(); err != nil {
break
}
if hc, ok := p.currChkMeta.Chunk.(*chunkenc.FloatHistogramChunk); ok {
switch hc := p.currChkMeta.Chunk.(type) {
case *chunkenc.FloatHistogramChunk:
newChunk.(*chunkenc.FloatHistogramChunk).SetCounterResetHeader(hc.GetCounterResetHeader())
case *safeHeadChunk:
if unwrapped, ok := hc.Chunk.(*chunkenc.FloatHistogramChunk); ok {
newChunk.(*chunkenc.FloatHistogramChunk).SetCounterResetHeader(unwrapped.GetCounterResetHeader())
} else {
err = fmt.Errorf("internal error, could not unwrap safeHeadChunk to float histogram chunk: %T", hc.Chunk)
}
default:
err = fmt.Errorf("internal error, unknown chunk type %T when expecting float histogram", p.currChkMeta.Chunk)
}
if err != nil {
break
}
var h *histogram.FloatHistogram
t, h = p.currDelIter.AtFloatHistogram()
p.curr.MinTime = t
// Detect missing gauge reset hint.
if h.CounterResetHint == histogram.GaugeType && newChunk.(*chunkenc.FloatHistogramChunk).GetCounterResetHeader() != chunkenc.GaugeType {
err = fmt.Errorf("found float gauge histogram in non gauge chunk")
break
}
app.AppendFloatHistogram(t, h)
for vt := p.currDelIter.Next(); vt != chunkenc.ValNone; vt = p.currDelIter.Next() {
if vt != chunkenc.ValFloatHistogram {
err = fmt.Errorf("found value type %v in histogram chunk", vt)
@ -858,6 +914,20 @@ func (p *populateWithDelChunkSeriesIterator) Next() bool {
t, h = p.currDelIter.AtFloatHistogram()
// Defend against corrupted chunks.
if h.CounterResetHint == histogram.GaugeType {
pI, nI, bpI, bnI, _, _, okToAppend := app.(*chunkenc.FloatHistogramAppender).AppendableGauge(h)
if !okToAppend {
err = errors.New("unable to append histogram due to unexpected schema change")
break
}
if len(pI)+len(nI)+len(bpI)+len(bnI) > 0 {
err = fmt.Errorf(
"bucket layout has changed unexpectedly: forward %d positive, %d negative, backward %d positive %d negative bucket interjections required",
len(pI), len(nI), len(bpI), len(bnI),
)
break
}
} else {
pI, nI, okToAppend, counterReset := app.(*chunkenc.FloatHistogramAppender).Appendable(h)
if len(pI)+len(nI) > 0 {
err = fmt.Errorf(
@ -874,6 +944,7 @@ func (p *populateWithDelChunkSeriesIterator) Next() bool {
err = errors.New("unable to append histogram due to unexpected schema change")
break
}
}
app.AppendFloatHistogram(t, h)
}

View file

@ -29,6 +29,7 @@ import (
"github.com/pkg/errors"
"github.com/stretchr/testify/require"
"github.com/prometheus/prometheus/model/histogram"
"github.com/prometheus/prometheus/model/labels"
"github.com/prometheus/prometheus/storage"
"github.com/prometheus/prometheus/tsdb/chunkenc"
@ -907,6 +908,202 @@ func TestPopulateWithTombSeriesIterators(t *testing.T) {
sample{3, 5, nil, nil}, sample{6, 1, nil, nil}, sample{7, 89, nil, nil},
},
},
{
name: "one histogram chunk",
chks: [][]tsdbutil.Sample{
{
sample{1, 0, tsdbutil.GenerateTestHistogram(1), nil},
sample{2, 0, tsdbutil.GenerateTestHistogram(2), nil},
sample{3, 0, tsdbutil.GenerateTestHistogram(3), nil},
sample{6, 0, tsdbutil.GenerateTestHistogram(6), nil},
},
},
expected: []tsdbutil.Sample{
sample{1, 0, tsdbutil.GenerateTestHistogram(1), nil},
sample{2, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(2)), nil},
sample{3, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(3)), nil},
sample{6, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(6)), nil},
},
expectedChks: []chunks.Meta{
tsdbutil.ChunkFromSamples([]tsdbutil.Sample{
sample{1, 0, tsdbutil.GenerateTestHistogram(1), nil},
sample{2, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(2)), nil},
sample{3, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(3)), nil},
sample{6, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(6)), nil},
}),
},
},
{
name: "one histogram chunk intersect with deletion interval",
chks: [][]tsdbutil.Sample{
{
sample{1, 0, tsdbutil.GenerateTestHistogram(1), nil},
sample{2, 0, tsdbutil.GenerateTestHistogram(2), nil},
sample{3, 0, tsdbutil.GenerateTestHistogram(3), nil},
sample{6, 0, tsdbutil.GenerateTestHistogram(6), nil},
},
},
intervals: tombstones.Intervals{{Mint: 5, Maxt: 20}},
expected: []tsdbutil.Sample{
sample{1, 0, tsdbutil.GenerateTestHistogram(1), nil},
sample{2, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(2)), nil},
sample{3, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(3)), nil},
},
expectedChks: []chunks.Meta{
tsdbutil.ChunkFromSamples([]tsdbutil.Sample{
sample{1, 0, tsdbutil.GenerateTestHistogram(1), nil},
sample{2, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(2)), nil},
sample{3, 0, tsdbutil.SetHistogramNotCounterReset(tsdbutil.GenerateTestHistogram(3)), nil},
}),
},
},
{
name: "one float histogram chunk",
chks: [][]tsdbutil.Sample{
{
sample{1, 0, nil, tsdbutil.GenerateTestFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.GenerateTestFloatHistogram(2)},
sample{3, 0, nil, tsdbutil.GenerateTestFloatHistogram(3)},
sample{6, 0, nil, tsdbutil.GenerateTestFloatHistogram(6)},
},
},
expected: []tsdbutil.Sample{
sample{1, 0, nil, tsdbutil.GenerateTestFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(2))},
sample{3, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(3))},
sample{6, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(6))},
},
expectedChks: []chunks.Meta{
tsdbutil.ChunkFromSamples([]tsdbutil.Sample{
sample{1, 0, nil, tsdbutil.GenerateTestFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(2))},
sample{3, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(3))},
sample{6, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(6))},
}),
},
},
{
name: "one float histogram chunk intersect with deletion interval",
chks: [][]tsdbutil.Sample{
{
sample{1, 0, nil, tsdbutil.GenerateTestFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.GenerateTestFloatHistogram(2)},
sample{3, 0, nil, tsdbutil.GenerateTestFloatHistogram(3)},
sample{6, 0, nil, tsdbutil.GenerateTestFloatHistogram(6)},
},
},
intervals: tombstones.Intervals{{Mint: 5, Maxt: 20}},
expected: []tsdbutil.Sample{
sample{1, 0, nil, tsdbutil.GenerateTestFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(2))},
sample{3, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(3))},
},
expectedChks: []chunks.Meta{
tsdbutil.ChunkFromSamples([]tsdbutil.Sample{
sample{1, 0, nil, tsdbutil.GenerateTestFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(2))},
sample{3, 0, nil, tsdbutil.SetFloatHistogramNotCounterReset(tsdbutil.GenerateTestFloatHistogram(3))},
}),
},
},
{
name: "one gauge histogram chunk",
chks: [][]tsdbutil.Sample{
{
sample{1, 0, tsdbutil.GenerateTestGaugeHistogram(1), nil},
sample{2, 0, tsdbutil.GenerateTestGaugeHistogram(2), nil},
sample{3, 0, tsdbutil.GenerateTestGaugeHistogram(3), nil},
sample{6, 0, tsdbutil.GenerateTestGaugeHistogram(6), nil},
},
},
expected: []tsdbutil.Sample{
sample{1, 0, tsdbutil.GenerateTestGaugeHistogram(1), nil},
sample{2, 0, tsdbutil.GenerateTestGaugeHistogram(2), nil},
sample{3, 0, tsdbutil.GenerateTestGaugeHistogram(3), nil},
sample{6, 0, tsdbutil.GenerateTestGaugeHistogram(6), nil},
},
expectedChks: []chunks.Meta{
tsdbutil.ChunkFromSamples([]tsdbutil.Sample{
sample{1, 0, tsdbutil.GenerateTestGaugeHistogram(1), nil},
sample{2, 0, tsdbutil.GenerateTestGaugeHistogram(2), nil},
sample{3, 0, tsdbutil.GenerateTestGaugeHistogram(3), nil},
sample{6, 0, tsdbutil.GenerateTestGaugeHistogram(6), nil},
}),
},
},
{
name: "one gauge histogram chunk intersect with deletion interval",
chks: [][]tsdbutil.Sample{
{
sample{1, 0, tsdbutil.GenerateTestGaugeHistogram(1), nil},
sample{2, 0, tsdbutil.GenerateTestGaugeHistogram(2), nil},
sample{3, 0, tsdbutil.GenerateTestGaugeHistogram(3), nil},
sample{6, 0, tsdbutil.GenerateTestGaugeHistogram(6), nil},
},
},
intervals: tombstones.Intervals{{Mint: 5, Maxt: 20}},
expected: []tsdbutil.Sample{
sample{1, 0, tsdbutil.GenerateTestGaugeHistogram(1), nil},
sample{2, 0, tsdbutil.GenerateTestGaugeHistogram(2), nil},
sample{3, 0, tsdbutil.GenerateTestGaugeHistogram(3), nil},
},
expectedChks: []chunks.Meta{
tsdbutil.ChunkFromSamples([]tsdbutil.Sample{
sample{1, 0, tsdbutil.GenerateTestGaugeHistogram(1), nil},
sample{2, 0, tsdbutil.GenerateTestGaugeHistogram(2), nil},
sample{3, 0, tsdbutil.GenerateTestGaugeHistogram(3), nil},
}),
},
},
{
name: "one gauge float histogram",
chks: [][]tsdbutil.Sample{
{
sample{1, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(2)},
sample{3, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(3)},
sample{6, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(6)},
},
},
expected: []tsdbutil.Sample{
sample{1, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(2)},
sample{3, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(3)},
sample{6, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(6)},
},
expectedChks: []chunks.Meta{
tsdbutil.ChunkFromSamples([]tsdbutil.Sample{
sample{1, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(2)},
sample{3, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(3)},
sample{6, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(6)},
}),
},
},
{
name: "one gauge float histogram chunk intersect with deletion interval",
chks: [][]tsdbutil.Sample{
{
sample{1, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(2)},
sample{3, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(3)},
sample{6, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(6)},
},
},
intervals: tombstones.Intervals{{Mint: 5, Maxt: 20}},
expected: []tsdbutil.Sample{
sample{1, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(2)},
sample{3, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(3)},
},
expectedChks: []chunks.Meta{
tsdbutil.ChunkFromSamples([]tsdbutil.Sample{
sample{1, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(1)},
sample{2, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(2)},
sample{3, 0, nil, tsdbutil.GenerateTestGaugeFloatHistogram(3)},
}),
},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
@ -2411,3 +2608,80 @@ func BenchmarkHeadQuerier(b *testing.B) {
require.NoError(b, ss.Err())
}
}
// This is a regression test for the case where gauge histograms were not handled by
// populateWithDelChunkSeriesIterator correctly.
func TestQueryWithDeletedHistograms(t *testing.T) {
testcases := map[string]func(int) (*histogram.Histogram, *histogram.FloatHistogram){
"intCounter": func(i int) (*histogram.Histogram, *histogram.FloatHistogram) {
return tsdbutil.GenerateTestHistogram(i), nil
},
"intgauge": func(i int) (*histogram.Histogram, *histogram.FloatHistogram) {
return tsdbutil.GenerateTestGaugeHistogram(rand.Int() % 1000), nil
},
"floatCounter": func(i int) (*histogram.Histogram, *histogram.FloatHistogram) {
return nil, tsdbutil.GenerateTestFloatHistogram(i)
},
"floatGauge": func(i int) (*histogram.Histogram, *histogram.FloatHistogram) {
return nil, tsdbutil.GenerateTestGaugeFloatHistogram(rand.Int() % 1000)
},
}
for name, tc := range testcases {
t.Run(name, func(t *testing.T) {
db := openTestDB(t, nil, nil)
defer func() {
require.NoError(t, db.Close())
}()
db.EnableNativeHistograms()
appender := db.Appender(context.Background())
var (
err error
seriesRef storage.SeriesRef
)
lbs := labels.FromStrings("__name__", "test", "type", name)
for i := 0; i < 100; i++ {
h, fh := tc(i)
seriesRef, err = appender.AppendHistogram(seriesRef, lbs, int64(i), h, fh)
require.NoError(t, err)
}
err = appender.Commit()
require.NoError(t, err)
matcher, err := labels.NewMatcher(labels.MatchEqual, "__name__", "test")
require.NoError(t, err)
// Delete the last 20.
err = db.Delete(80, 100, matcher)
require.NoError(t, err)
chunkQuerier, err := db.ChunkQuerier(context.Background(), 0, 100)
require.NoError(t, err)
css := chunkQuerier.Select(false, nil, matcher)
seriesCount := 0
for css.Next() {
seriesCount++
series := css.At()
sampleCount := 0
it := series.Iterator(nil)
for it.Next() {
chk := it.At()
for cit := chk.Chunk.Iterator(nil); cit.Next() != chunkenc.ValNone; {
sampleCount++
}
}
require.NoError(t, it.Err())
require.Equal(t, 80, sampleCount)
}
require.NoError(t, css.Err())
require.Equal(t, 1, seriesCount)
})
}
}

View file

@ -71,9 +71,19 @@ func ChunkFromSamplesGeneric(s Samples) chunks.Meta {
case chunkenc.ValFloat:
ca.Append(s.Get(i).T(), s.Get(i).F())
case chunkenc.ValHistogram:
ca.AppendHistogram(s.Get(i).T(), s.Get(i).H())
h := s.Get(i).H()
ca.AppendHistogram(s.Get(i).T(), h)
if i == 0 && h.CounterResetHint == histogram.GaugeType {
hc := c.(*chunkenc.HistogramChunk)
hc.SetCounterResetHeader(chunkenc.GaugeType)
}
case chunkenc.ValFloatHistogram:
ca.AppendFloatHistogram(s.Get(i).T(), s.Get(i).FH())
fh := s.Get(i).FH()
ca.AppendFloatHistogram(s.Get(i).T(), fh)
if i == 0 && fh.CounterResetHint == histogram.GaugeType {
hc := c.(*chunkenc.FloatHistogramChunk)
hc.SetCounterResetHeader(chunkenc.GaugeType)
}
default:
panic(fmt.Sprintf("unknown sample type %s", sampleType.String()))
}

View file

@ -108,3 +108,13 @@ func GenerateTestGaugeFloatHistogram(i int) *histogram.FloatHistogram {
h.CounterResetHint = histogram.GaugeType
return h
}
func SetHistogramNotCounterReset(h *histogram.Histogram) *histogram.Histogram {
h.CounterResetHint = histogram.NotCounterReset
return h
}
func SetFloatHistogramNotCounterReset(h *histogram.FloatHistogram) *histogram.FloatHistogram {
h.CounterResetHint = histogram.NotCounterReset
return h
}