From 1cdee43726a7b8797144fe4855db6f7612aa5ec9 Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Tue, 5 May 2026 14:01:50 -0400 Subject: [PATCH 1/2] tsdb: fix init race that lets initialized() return true before maxTime is set initTime previously set minTime first and maxTime second. Because Head.initialized() keys only off minTime, a concurrent Head.Appender call could observe initialized() == true while maxTime was still math.MinInt64. h.appender() then computes appendableMinValidTime as MaxTime() - chunkRange/2, which underflows to a large positive number and rejects in-range samples with ErrOutOfBounds. Set maxTime first, then minTime. The CAS-loser wait now spins on minTime instead of maxTime, preserving the existing anti-deadlock timeout. AppenderV2 shares the same gate, so this single change covers both paths. The TestHead_InitAppenderRace_ErrOutOfBounds test added in #17963 is now stable across 1000 iterations (and 100 iterations under -race). Relates to #17941 Builds on #17963 Co-Authored-By: Claude Opus 4.7 (1M context) Signed-off-by: Owen Williams --- tsdb/head_append.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tsdb/head_append.go b/tsdb/head_append.go index 558c39292c..f2b89653af 100644 --- a/tsdb/head_append.go +++ b/tsdb/head_append.go @@ -117,11 +117,17 @@ func (a *initAppender) AppendSTZeroSample(ref storage.SeriesRef, lset labels.Lab // initTime initializes a head with the first timestamp. This only needs to be called // for a completely fresh head with an empty WAL. func (h *Head) initTime(t int64) { - if !h.minTime.CompareAndSwap(math.MaxInt64, t) { - // Concurrent appends that are initializing. - // Wait until h.maxTime is swapped to avoid minTime/maxTime races. + // maxTime must be set before minTime, because initialized() keys off minTime. + // If minTime were set first, a concurrent Head.Appender call could observe + // initialized() == true while maxTime is still math.MinInt64; the resulting + // underflow in appendableMinValidTime would reject in-range samples with + // ErrOutOfBounds. + if !h.maxTime.CompareAndSwap(math.MinInt64, t) { + // Another goroutine already won the init race. Wait until it also sets + // minTime, so callers that next read initialized() can rely on both + // bounds being valid. antiDeadlockTimeout := time.After(500 * time.Millisecond) - for h.maxTime.Load() == math.MinInt64 { + for h.minTime.Load() == math.MaxInt64 { select { case <-antiDeadlockTimeout: return @@ -130,8 +136,7 @@ func (h *Head) initTime(t int64) { } return } - // Ensure that max time is initialized to at least the min time we just set. - h.maxTime.CompareAndSwap(math.MinInt64, t) + h.minTime.CompareAndSwap(math.MaxInt64, t) } func (a *initAppender) GetRef(lset labels.Labels, hash uint64) (storage.SeriesRef, labels.Labels) { From 9ec9b1e3c64420ef4be77ba7bd51ab429aadc53b Mon Sep 17 00:00:00 2001 From: Owen Williams Date: Tue, 12 May 2026 11:00:12 -0400 Subject: [PATCH 2/2] Comments to explain what's going on Signed-off-by: Owen Williams --- tsdb/head.go | 7 +++++-- tsdb/head_append.go | 8 ++++---- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tsdb/head.go b/tsdb/head.go index 4840f0cae5..5d447698ad 100644 --- a/tsdb/head.go +++ b/tsdb/head.go @@ -73,7 +73,7 @@ type Head struct { numSeries atomic.Uint64 numStaleSeries atomic.Uint64 minOOOTime, maxOOOTime atomic.Int64 // TODO(jesusvazquez) These should be updated after garbage collection. - minTime, maxTime atomic.Int64 // Current min and max of the samples included in the head. TODO(jesusvazquez) Ensure these are properly tracked. + minTime, maxTime atomic.Int64 // Current min and max of the samples included in the head. minTime != math.MaxInt64 is used to determine whether the head has been initialized, so care must be taken that the initialized state only changes after maxTime is already updated. minValidTime atomic.Int64 // Mint allowed to be added to the head. It shouldn't be lower than the maxt of the last persisted block. lastWALTruncationTime atomic.Int64 lastMemoryTruncationTime atomic.Int64 @@ -1139,6 +1139,8 @@ func (h *Head) PostingsCardinalityStats(statsByLabelName string, limit int) *ind return h.cardinalityCache } +// Expands the Head time bounds to include the provided in-order range. +// Concurrent updates retry with compare-and-swap so the bounds only move outward. func (h *Head) updateMinMaxTime(mint, maxt int64) { for { lt := h.MinTime() @@ -1813,7 +1815,8 @@ func (h *Head) Meta() BlockMeta { } } -// MinTime returns the lowest time bound on visible data in the head. +// MinTime returns the lowest time bound on visible data in the head. If this +// value is math.MaxInt64, we consider the head to be uninitialized. func (h *Head) MinTime() int64 { return h.minTime.Load() } diff --git a/tsdb/head_append.go b/tsdb/head_append.go index f2b89653af..30ec7c8302 100644 --- a/tsdb/head_append.go +++ b/tsdb/head_append.go @@ -118,10 +118,10 @@ func (a *initAppender) AppendSTZeroSample(ref storage.SeriesRef, lset labels.Lab // for a completely fresh head with an empty WAL. func (h *Head) initTime(t int64) { // maxTime must be set before minTime, because initialized() keys off minTime. - // If minTime were set first, a concurrent Head.Appender call could observe - // initialized() == true while maxTime is still math.MinInt64; the resulting - // underflow in appendableMinValidTime would reject in-range samples with - // ErrOutOfBounds. + // If minTime were set to t first, a concurrent Head.Appender call could + // observe initialized() == true while maxTime is still math.MinInt64; the + // resulting underflow in appendableMinValidTime would reject in-range samples + // with ErrOutOfBounds. if !h.maxTime.CompareAndSwap(math.MinInt64, t) { // Another goroutine already won the init race. Wait until it also sets // minTime, so callers that next read initialized() can rely on both