mirrors/prometheus
1
0
Fork 0
mirror of https://github.com/prometheus/prometheus.git synced 2026-05-04 12:01:06 +02:00
Code Issues Projects Releases Wiki Activity

test(tsdb): add OOO error coverage for ST zero sample appends (#18554)

Browse Source 
* test(tsdb): add OOO error coverage for ST zero sample appends

Add unit tests exercising the out-of-order error paths in
AppendSTZeroSample, AppendHistogramSTZeroSample (AppenderV1), and
the best-effort ST injection in AppenderV2.Append.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

* make format

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

* test(tsdb): add TestHeadAppenderV2_BestEffortSTZeroSample_OOO

The three OOO cases added to TestHeadAppenderV2_Append_EnableSTAsZeroSample
use a single appender so headChunks is nil at append time; the zero sample
enters the batch and is rejected silently in commitFloats, never reaching
the error-handling branch at line 374 of bestEffortAppendSTZeroSample.

Add a dedicated test that commits the first sample before appending the
second. This makes headChunks non-nil, so appendFloat/appendHistogram/
appendFloatHistogram returns ErrOutOfOrderSample at append time and the
branch at line 374 is actually executed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>

---------

Signed-off-by: György Krajcsovits <gyorgy.krajcsovits@grafana.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
George Krajcsovits 2026-04-23 09:48:12 +02:00 committed by GitHub
parent be5db19c73
commit c84b0acdb4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 185 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

103
tsdb/head_append_v2_test.go
View File

@ -4436,6 +4436,59 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
}
}(),
},
{
name: "ST is out of order/float",
appendableSamples: []appendableSamples{
{ts: 200, fSample: 10},
{ts: 300, fSample: 10, st: 100},
},
// ST results ErrOutOfOrderSample, but ST append is best effort, so
// ST should be ignored, but sample appended.
expectedSamples: func() []chunks.Sample {
return []chunks.Sample{
sample{t: 200, f: 10},
sample{t: 300, f: 10},
}
}(),
},
{
name: "ST is out of order/histogram",
appendableSamples: []appendableSamples{
{ts: 200, h: testHistogram},
{ts: 300, h: testHistogram, st: 100},
},
// ST results ErrOutOfOrderSample, but ST append is best effort, so
// ST should be ignored, but sample appended.
expectedSamples: func() []chunks.Sample {
// NOTE: Without ST, on query, first histogram sample will get
// CounterReset adjusted to UnknownCounterReset.
firstSample := testHistogram.Copy()
firstSample.CounterResetHint = histogram.UnknownCounterReset
return []chunks.Sample{
sample{t: 200, h: firstSample},
sample{t: 300, h: testHistogram},
}
}(),
},
{
name: "ST is out of order/floathistogram",
appendableSamples: []appendableSamples{
{ts: 200, fh: testFloatHistogram},
{ts: 300, fh: testFloatHistogram, st: 100},
},
// ST results ErrOutOfOrderSample, but ST append is best effort, so
// ST should be ignored, but sample appended.
expectedSamples: func() []chunks.Sample {
// NOTE: Without ST, on query, first histogram sample will get
// CounterReset adjusted to UnknownCounterReset.
firstSample := testFloatHistogram.Copy()
firstSample.CounterResetHint = histogram.UnknownCounterReset
return []chunks.Sample{
sample{t: 200, fh: firstSample},
sample{t: 300, fh: testFloatHistogram},
}
}(),
},
} {
t.Run(tc.name, func(t *testing.T) {
opts := newTestHeadDefaultOptions(DefaultBlockDuration, false)
@ -4462,6 +4515,56 @@ func TestHeadAppenderV2_Append_EnableSTAsZeroSample(t *testing.T) {
}
}
func TestHeadAppenderV2_BestEffortSTZeroSample_OOO(t *testing.T) {
testHistogram := tsdbutil.GenerateTestHistogram(1)
testHistogram.CounterResetHint = histogram.NotCounterReset
testFloatHistogram := tsdbutil.GenerateTestFloatHistogram(1)
testFloatHistogram.CounterResetHint = histogram.NotCounterReset
for _, tc := range []struct {
name string
v float64
h *histogram.Histogram
fh *histogram.FloatHistogram
}{
{name: "float", v: 10},
{name: "histogram", h: testHistogram},
{name: "floathistogram", fh: testFloatHistogram},
} {
t.Run(tc.name, func(t *testing.T) {
opts := newTestHeadDefaultOptions(DefaultBlockDuration, false)
opts.EnableSTAsZeroSample = true
head, _ := newTestHeadWithOptions(t, compression.None, opts)
lbls := labels.FromStrings("foo", "bar")
// First appender: commit a sample at ts=200 to establish headMaxt and
// populate headChunks on the series.
app := head.AppenderV2(context.Background())
_, err := app.Append(0, lbls, 0, 200, tc.v, tc.h, tc.fh, storage.AOptions{})
require.NoError(t, err)
require.NoError(t, app.Commit())
// Second appender: st=100 is OOO (headMaxt=200, OOO disabled). Because the
// series now has committed data, bestEffortAppendSTZeroSample calls
// appendFloat/appendHistogram/appendFloatHistogram with fastRejectOOO=true,
// gets ErrOutOfOrderSample, and silently ignores it. The main sample must
// still be appended successfully.
app = head.AppenderV2(context.Background())
_, err = app.Append(0, lbls, 100, 300, tc.v, tc.h, tc.fh, storage.AOptions{})
require.NoError(t, err)
require.NoError(t, app.Commit())
q, err := NewBlockQuerier(head, math.MinInt64, math.MaxInt64)
require.NoError(t, err)
result := query(t, q, labels.MustNewMatcher(labels.MatchEqual, "foo", "bar"))
// Only the two main samples should be present; the OOO zero sample at ts=100
// must have been dropped.
require.Len(t, result[`{foo="bar"}`], 2)
})
}
}
// Regression test for data race https://github.com/prometheus/prometheus/issues/15139.
func TestHeadAppenderV2_Append_HistogramAndCommitConcurrency(t *testing.T) {
h := tsdbutil.GenerateTestHistogram(1)

82
tsdb/head_test.go
View File

@ -7172,6 +7172,22 @@ func TestHeadAppender_AppendHistogramSTZeroSample(t *testing.T) {
},
expectedError: storage.ErrDuplicateSampleForTimestamp,
},
{
name: "integer histogram ST is out of order",
appendableSamples: []appendableSamples{
{ts: 200, h: tsdbutil.GenerateTestHistogram(1)},
{ts: 300, h: tsdbutil.GenerateTestHistogram(1), st: 100},
},
expectedError: storage.ErrOutOfOrderST,
},
{
name: "float histogram ST is out of order",
appendableSamples: []appendableSamples{
{ts: 200, fh: tsdbutil.GenerateTestFloatHistogram(1)},
{ts: 300, fh: tsdbutil.GenerateTestFloatHistogram(1), st: 100},
},
expectedError: storage.ErrOutOfOrderST,
},
} {
t.Run(tc.name, func(t *testing.T) {
h, _ := newTestHead(t, DefaultBlockDuration, compression.None, false)
@ -7195,6 +7211,72 @@ func TestHeadAppender_AppendHistogramSTZeroSample(t *testing.T) {
}
}
func TestHeadAppender_AppendSTZeroSample(t *testing.T) {
type appendableSample struct {
ts int64
fSample float64
st int64
}
for _, tc := range []struct {
name string
oooEnabled bool
appendableSamples []appendableSample
expectedError error
}{
{
name: "ST lower than minValidTime returns ErrOutOfBounds",
appendableSamples: []appendableSample{
{ts: 100, fSample: 1.0, st: -1},
},
expectedError: storage.ErrOutOfBounds,
},
{
name: "ST duplicates an existing sample",
appendableSamples: []appendableSample{
{ts: 100, fSample: 1.0},
{ts: 200, fSample: 2.0, st: 100},
},
expectedError: storage.ErrDuplicateSampleForTimestamp,
},
{
name: "ST is out of order when OOO is disabled",
appendableSamples: []appendableSample{
{ts: 200, fSample: 2.0},
{ts: 300, fSample: 3.0, st: 100},
},
expectedError: storage.ErrOutOfOrderSample,
},
{
name: "ST is out of order when OOO is enabled",
oooEnabled: true,
appendableSamples: []appendableSample{
{ts: 200, fSample: 2.0},
{ts: 300, fSample: 3.0, st: 100},
},
expectedError: storage.ErrOutOfOrderST,
},
} {
t.Run(tc.name, func(t *testing.T) {
h, _ := newTestHead(t, DefaultBlockDuration, compression.None, tc.oooEnabled)
lbls := labels.FromStrings("foo", "bar")
var ref storage.SeriesRef
for _, sample := range tc.appendableSamples {
a := h.Appender(context.Background())
var err error
if sample.st != 0 {
ref, err = a.AppendSTZeroSample(ref, lbls, sample.ts, sample.st)
require.ErrorIs(t, err, tc.expectedError)
}
ref, err = a.Append(ref, lbls, sample.ts, sample.fSample)
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()`