From e555469ba1f718498eb1a816772d22bdb1adbeac Mon Sep 17 00:00:00 2001 From: Ganesh Vernekar Date: Wed, 28 Dec 2022 14:31:55 +0530 Subject: [PATCH 1/3] tsdb: Remove isHistogramSeries from memSeries Signed-off-by: Ganesh Vernekar --- tsdb/head.go | 4 ---- tsdb/head_append.go | 15 ++++++++++----- tsdb/head_wal.go | 2 -- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/tsdb/head.go b/tsdb/head.go index 37e12d8a4..8ec8a010d 100644 --- a/tsdb/head.go +++ b/tsdb/head.go @@ -1862,10 +1862,6 @@ type memSeries struct { // txs is nil if isolation is disabled. txs *txRing - // TODO(beorn7): The only reason we track this is to create a staleness - // marker as either histogram or float sample. Perhaps there is a better way. - isHistogramSeries bool - pendingCommit bool // Whether there are samples waiting to be committed to this series. } diff --git a/tsdb/head_append.go b/tsdb/head_append.go index e541f013f..72a57af3c 100644 --- a/tsdb/head_append.go +++ b/tsdb/head_append.go @@ -350,7 +350,7 @@ func (a *headAppender) Append(ref storage.SeriesRef, lset labels.Labels, t int64 } } - if value.IsStaleNaN(v) && s.isHistogramSeries { + if value.IsStaleNaN(v) && s.lastHistogramValue != nil { // TODO(marctc): do we have do to the same for float histograms? return a.AppendHistogram(ref, lset, t, &histogram.Histogram{Sum: v}, nil) } @@ -555,8 +555,12 @@ func (a *headAppender) AppendHistogram(ref storage.SeriesRef, lset labels.Labels if err != nil { return 0, err } - s.isHistogramSeries = true if created { + if h != nil { + s.lastHistogramValue = &histogram.Histogram{} + } else if fh != nil { + s.lastFloatHistogramValue = &histogram.FloatHistogram{} + } a.series = append(a.series, record.RefSeries{ Ref: s.ref, Labels: lset, @@ -1115,11 +1119,12 @@ func (s *memSeries) append(t int64, v float64, appendID uint64, chunkDiskMapper return sampleInOrder, chunkCreated } s.app.Append(t, v) - s.isHistogramSeries = false c.maxTime = t s.lastValue = v + s.lastHistogramValue = nil + s.lastFloatHistogramValue = nil if appendID > 0 { s.txs.add(appendID) @@ -1184,11 +1189,11 @@ func (s *memSeries) appendHistogram(t int64, h *histogram.Histogram, appendID ui } s.app.AppendHistogram(t, h) - s.isHistogramSeries = true c.maxTime = t s.lastHistogramValue = h + s.lastFloatHistogramValue = nil if appendID > 0 { s.txs.add(appendID) @@ -1252,11 +1257,11 @@ func (s *memSeries) appendFloatHistogram(t int64, fh *histogram.FloatHistogram, } s.app.AppendFloatHistogram(t, fh) - s.isHistogramSeries = true c.maxTime = t s.lastFloatHistogramValue = fh + s.lastHistogramValue = nil if appendID > 0 { s.txs.add(appendID) diff --git a/tsdb/head_wal.go b/tsdb/head_wal.go index df1896b25..97f2dcc14 100644 --- a/tsdb/head_wal.go +++ b/tsdb/head_wal.go @@ -605,7 +605,6 @@ func (wp *walSubsetProcessor) processWALSamples(h *Head, mmappedChunks, oooMmapp if s.T <= ms.mmMaxTime { continue } - ms.isHistogramSeries = false if s.T <= ms.mmMaxTime { continue } @@ -634,7 +633,6 @@ func (wp *walSubsetProcessor) processWALSamples(h *Head, mmappedChunks, oooMmapp unknownHistogramRefs++ continue } - ms.isHistogramSeries = true if s.t <= ms.mmMaxTime { continue } From 2820e327db03461b394d8d1677f0780e024af653 Mon Sep 17 00:00:00 2001 From: Ganesh Vernekar Date: Wed, 28 Dec 2022 14:48:39 +0530 Subject: [PATCH 2/3] tsdb: Add staleness handling for FloatHistogram Signed-off-by: Ganesh Vernekar --- tsdb/head_append.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tsdb/head_append.go b/tsdb/head_append.go index 72a57af3c..c09027ae4 100644 --- a/tsdb/head_append.go +++ b/tsdb/head_append.go @@ -350,9 +350,12 @@ func (a *headAppender) Append(ref storage.SeriesRef, lset labels.Labels, t int64 } } - if value.IsStaleNaN(v) && s.lastHistogramValue != nil { - // TODO(marctc): do we have do to the same for float histograms? - return a.AppendHistogram(ref, lset, t, &histogram.Histogram{Sum: v}, nil) + if value.IsStaleNaN(v) { + if s.lastHistogramValue != nil { + return a.AppendHistogram(ref, lset, t, &histogram.Histogram{Sum: v}, nil) + } else if s.lastFloatHistogramValue != nil { + return a.AppendHistogram(ref, lset, t, nil, &histogram.FloatHistogram{Sum: v}) + } } s.Lock() From c155c0e312a7750c9aa1fbc61137748f9067f8ad Mon Sep 17 00:00:00 2001 From: Ganesh Vernekar Date: Wed, 28 Dec 2022 14:48:56 +0530 Subject: [PATCH 3/3] tsdb: Test staleness handling of FloatHistogram Signed-off-by: Ganesh Vernekar --- tsdb/head_test.go | 87 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 67 insertions(+), 20 deletions(-) diff --git a/tsdb/head_test.go b/tsdb/head_test.go index b8c68eddc..6d7b82e1b 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -3361,8 +3361,16 @@ func TestHistogramMetrics(t *testing.T) { } func TestHistogramStaleSample(t *testing.T) { - // TODO(marctc): Add similar test for float histograms + t.Run("integer histogram", func(t *testing.T) { + testHistogramStaleSampleHelper(t, false) + }) + t.Run("float histogram", func(t *testing.T) { + testHistogramStaleSampleHelper(t, true) + }) +} +func testHistogramStaleSampleHelper(t *testing.T, floatHistogram bool) { + t.Helper() l := labels.FromStrings("a", "b") numHistograms := 20 head, _ := newTestHead(t, 100000, false, false) @@ -3372,8 +3380,9 @@ func TestHistogramStaleSample(t *testing.T) { require.NoError(t, head.Init(0)) type timedHistogram struct { - t int64 - h *histogram.Histogram + t int64 + h *histogram.Histogram + fh *histogram.FloatHistogram } expHistograms := make([]timedHistogram, 0, numHistograms) @@ -3392,9 +3401,15 @@ func TestHistogramStaleSample(t *testing.T) { it := s.Iterator(nil) actHistograms := make([]timedHistogram, 0, len(expHistograms)) - for it.Next() == chunkenc.ValHistogram { - t, h := it.AtHistogram() - actHistograms = append(actHistograms, timedHistogram{t, h}) + for typ := it.Next(); typ != chunkenc.ValNone; typ = it.Next() { + switch typ { + case chunkenc.ValHistogram: + t, h := it.AtHistogram() + actHistograms = append(actHistograms, timedHistogram{t: t, h: h}) + case chunkenc.ValFloatHistogram: + t, h := it.AtFloatHistogram() + actHistograms = append(actHistograms, timedHistogram{t: t, fh: h}) + } } // We cannot compare StaleNAN with require.Equal, hence checking each histogram manually. @@ -3402,15 +3417,27 @@ func TestHistogramStaleSample(t *testing.T) { actNumStale := 0 for i, eh := range expHistograms { ah := actHistograms[i] - if value.IsStaleNaN(eh.h.Sum) { - actNumStale++ - require.True(t, value.IsStaleNaN(ah.h.Sum)) - // To make require.Equal work. - ah.h.Sum = 0 - eh.h = eh.h.Copy() - eh.h.Sum = 0 + if floatHistogram { + if value.IsStaleNaN(eh.fh.Sum) { + actNumStale++ + require.True(t, value.IsStaleNaN(ah.fh.Sum)) + // To make require.Equal work. + ah.fh.Sum = 0 + eh.fh = eh.fh.Copy() + eh.fh.Sum = 0 + } + require.Equal(t, eh, ah) + } else { + if value.IsStaleNaN(eh.h.Sum) { + actNumStale++ + require.True(t, value.IsStaleNaN(ah.h.Sum)) + // To make require.Equal work. + ah.h.Sum = 0 + eh.h = eh.h.Copy() + eh.h.Sum = 0 + } + require.Equal(t, eh, ah) } - require.Equal(t, eh, ah) } require.Equal(t, numStale, actNumStale) } @@ -3418,14 +3445,24 @@ func TestHistogramStaleSample(t *testing.T) { // Adding stale in the same appender. app := head.Appender(context.Background()) for _, h := range GenerateTestHistograms(numHistograms) { - _, err := app.AppendHistogram(0, l, 100*int64(len(expHistograms)), h, nil) + var err error + if floatHistogram { + _, err = app.AppendHistogram(0, l, 100*int64(len(expHistograms)), nil, h.ToFloat()) + expHistograms = append(expHistograms, timedHistogram{t: 100 * int64(len(expHistograms)), fh: h.ToFloat()}) + } else { + _, err = app.AppendHistogram(0, l, 100*int64(len(expHistograms)), h, nil) + expHistograms = append(expHistograms, timedHistogram{t: 100 * int64(len(expHistograms)), h: h}) + } require.NoError(t, err) - expHistograms = append(expHistograms, timedHistogram{100 * int64(len(expHistograms)), h}) } // +1 so that delta-of-delta is not 0. _, err := app.Append(0, l, 100*int64(len(expHistograms))+1, math.Float64frombits(value.StaleNaN)) require.NoError(t, err) - expHistograms = append(expHistograms, timedHistogram{100*int64(len(expHistograms)) + 1, &histogram.Histogram{Sum: math.Float64frombits(value.StaleNaN)}}) + if floatHistogram { + expHistograms = append(expHistograms, timedHistogram{t: 100*int64(len(expHistograms)) + 1, fh: &histogram.FloatHistogram{Sum: math.Float64frombits(value.StaleNaN)}}) + } else { + expHistograms = append(expHistograms, timedHistogram{t: 100*int64(len(expHistograms)) + 1, h: &histogram.Histogram{Sum: math.Float64frombits(value.StaleNaN)}}) + } require.NoError(t, app.Commit()) // Only 1 chunk in the memory, no m-mapped chunk. @@ -3437,9 +3474,15 @@ func TestHistogramStaleSample(t *testing.T) { // Adding stale in different appender and continuing series after a stale sample. app = head.Appender(context.Background()) for _, h := range GenerateTestHistograms(2 * numHistograms)[numHistograms:] { - _, err := app.AppendHistogram(0, l, 100*int64(len(expHistograms)), h, nil) + var err error + if floatHistogram { + _, err = app.AppendHistogram(0, l, 100*int64(len(expHistograms)), nil, h.ToFloat()) + expHistograms = append(expHistograms, timedHistogram{t: 100 * int64(len(expHistograms)), fh: h.ToFloat()}) + } else { + _, err = app.AppendHistogram(0, l, 100*int64(len(expHistograms)), h, nil) + expHistograms = append(expHistograms, timedHistogram{t: 100 * int64(len(expHistograms)), h: h}) + } require.NoError(t, err) - expHistograms = append(expHistograms, timedHistogram{100 * int64(len(expHistograms)), h}) } require.NoError(t, app.Commit()) @@ -3447,7 +3490,11 @@ func TestHistogramStaleSample(t *testing.T) { // +1 so that delta-of-delta is not 0. _, err = app.Append(0, l, 100*int64(len(expHistograms))+1, math.Float64frombits(value.StaleNaN)) require.NoError(t, err) - expHistograms = append(expHistograms, timedHistogram{100*int64(len(expHistograms)) + 1, &histogram.Histogram{Sum: math.Float64frombits(value.StaleNaN)}}) + if floatHistogram { + expHistograms = append(expHistograms, timedHistogram{t: 100*int64(len(expHistograms)) + 1, fh: &histogram.FloatHistogram{Sum: math.Float64frombits(value.StaleNaN)}}) + } else { + expHistograms = append(expHistograms, timedHistogram{t: 100*int64(len(expHistograms)) + 1, h: &histogram.Histogram{Sum: math.Float64frombits(value.StaleNaN)}}) + } require.NoError(t, app.Commit()) // Total 2 chunks, 1 m-mapped.