From 89f011ba1348b1b7ec60b73650343be34d2d525c Mon Sep 17 00:00:00 2001 From: Banana Duck <56518282+Alhanaqtah@users.noreply.github.com> Date: Wed, 9 Jul 2025 16:37:55 +0300 Subject: [PATCH] fix: unlock of unlocked mutex (#16332) * fix: unlock on unlocked mutex Signed-off-by: Usama Alhanaqtah * test coverage Signed-off-by: Usama Alhanaqtah --------- Signed-off-by: Usama Alhanaqtah Co-authored-by: alhanaqtah.usama --- tsdb/head_append.go | 9 +++++++++ tsdb/head_test.go | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/tsdb/head_append.go b/tsdb/head_append.go index 03800b2455..05299f048d 100644 --- a/tsdb/head_append.go +++ b/tsdb/head_append.go @@ -799,13 +799,17 @@ func (a *headAppender) AppendHistogramCTZeroSample(ref storage.SeriesRef, lset l if errors.Is(err, storage.ErrOutOfOrderSample) { return 0, storage.ErrOutOfOrderCT } + + return 0, err } + // OOO is not allowed because after the first scrape, CT will be the same for most (if not all) future samples. // This is to prevent the injected zero from being marked as OOO forever. if isOOO { s.Unlock() return 0, storage.ErrOutOfOrderCT } + s.pendingCommit = true s.Unlock() a.histograms = append(a.histograms, record.RefHistogramSample{ @@ -832,13 +836,17 @@ func (a *headAppender) AppendHistogramCTZeroSample(ref storage.SeriesRef, lset l if errors.Is(err, storage.ErrOutOfOrderSample) { return 0, storage.ErrOutOfOrderCT } + + return 0, err } + // OOO is not allowed because after the first scrape, CT will be the same for most (if not all) future samples. // This is to prevent the injected zero from being marked as OOO forever. if isOOO { s.Unlock() return 0, storage.ErrOutOfOrderCT } + s.pendingCommit = true s.Unlock() a.floatHistograms = append(a.floatHistograms, record.RefFloatHistogramSample{ @@ -852,6 +860,7 @@ func (a *headAppender) AppendHistogramCTZeroSample(ref storage.SeriesRef, lset l if ct > a.maxt { a.maxt = ct } + return storage.SeriesRef(s.ref), nil } diff --git a/tsdb/head_test.go b/tsdb/head_test.go index 485b8b7b1f..fd3690e065 100644 --- a/tsdb/head_test.go +++ b/tsdb/head_test.go @@ -6717,6 +6717,50 @@ func TestHeadAppender_AppendCT(t *testing.T) { } } +func TestHeadAppender_AppendHistogramCTZeroSample(t *testing.T) { + testHistogram := tsdbutil.GenerateTestHistogram(1) + type appendableSamples struct { + ts int64 + h *histogram.Histogram + fh *histogram.FloatHistogram + ct int64 + } + for _, tc := range []struct { + name string + appendableSamples []appendableSamples + expectedError error + }{ + { + name: "CT lower than minValidTime initiates ErrOutOfBounds", + appendableSamples: []appendableSamples{ + {ts: 100, h: testHistogram, ct: -1}, + }, + expectedError: storage.ErrOutOfBounds, + }, + } { + t.Run(tc.name, func(t *testing.T) { + h, _ := newTestHead(t, DefaultBlockDuration, compression.None, false) + + defer func() { + require.NoError(t, h.Close()) + }() + + a := h.Appender(context.Background()) + lbls := labels.FromStrings("foo", "bar") + + for _, sample := range tc.appendableSamples { + ref, err := a.AppendHistogramCTZeroSample(0, lbls, sample.ts, sample.ct, sample.h, sample.fh) + require.ErrorIs(t, err, tc.expectedError) + + _, err = a.AppendHistogram(ref, lbls, sample.ts, sample.h, sample.fh) + require.NoError(t, err) + } + + require.NoError(t, a.Commit()) + }) + } +} + func TestHeadCompactableDoesNotCompactEmptyHead(t *testing.T) { // Use a chunk range of 1 here so that if we attempted to determine if the head // was compactable using default values for min and max times, `Head.compactable()`