mirror of
https://github.com/prometheus/prometheus.git
synced 2026-05-13 00:28:42 +02:00
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) <noreply@anthropic.com> Signed-off-by: Owen Williams <owen.williams@grafana.com>
This commit is contained in:
parent
bd4758a835
commit
1cdee43726
@ -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) {
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user