From 8a118d2d6053a0ef39662728a1bb3cb165322e24 Mon Sep 17 00:00:00 2001 From: Krunal Jain Date: Wed, 8 Jul 2026 15:47:52 -0700 Subject: [PATCH] fix(queryeviction): fix flaky TestPrometheusMetrics_IncrementedCorrectly The eviction signal (closing the `evicted` channel) and the evictionsTotal metric increment both happen in QueryEvictor.running(), but Cancel() runs before Inc(). waitEvicted() only synchronizes on the signal, so the final assertion could observe the metric before the third increment lands, causing an intermittent failure (expected: 3, actual: 2). Poll for the metric with require.Eventually instead of asserting immediately. Fixes #7604. AI-assistance note: drafted with Claude Code (root-cause analysis of the reported race and the test fix); reviewed and validated by me before submitting, per GENAI_POLICY.md. Signed-off-by: Krunal Jain --- CHANGELOG.md | 1 + pkg/util/queryeviction/evictor_test.go | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fccc3b8682..19c07400a9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -53,6 +53,7 @@ * [BUGFIX] Ingester: Fix inflight query counter leak when resource-based query protection rejects a request. #7539 * [BUGFIX] Ingester: Release the TSDB appender on every early-return path in `Push` (e.g. out-of-order label set) by deferring `Rollback`. Previously such requests leaked TSDB head series references, mmap'd chunks and pending state per request, causing the `cortex_ingester_tsdb_head_active_appenders` gauge to grow unbounded. #7528 * [BUGFIX] Ingester: Fix `panic: send on closed channel` in `ActiveQueriedSeriesService` on shutdown by removing the redundant channel close in `stopping()` and relying on `ctx.Done()` to signal worker exit. #7533 +* [BUGFIX] Query Eviction: Fix flaky `TestPrometheusMetrics_IncrementedCorrectly` by polling for the `evictionsTotal` metric instead of asserting immediately after the eviction signal, which fires before the metric is incremented in the evictor's background loop. #7680 * [BUGFIX] Ring: Fix ring token conflict resolution only applied to updated instance and make constantly token conflict check during instance observe period. * [BUGFIX] Distributor: Fix a panic (`slice bounds out of range`) in the stream push path when the context deadline expires while the worker goroutine is still marshalling a `WriteRequest`. #7541 * [BUGFIX] Query Frontend: Fix native histogram responses not being handled correctly in `minTime()` sort ordering for split_by_interval merge. #7555 diff --git a/pkg/util/queryeviction/evictor_test.go b/pkg/util/queryeviction/evictor_test.go index a6bbe924919..82b0a6afa16 100644 --- a/pkg/util/queryeviction/evictor_test.go +++ b/pkg/util/queryeviction/evictor_test.go @@ -207,7 +207,13 @@ func TestPrometheusMetrics_IncrementedCorrectly(t *testing.T) { waitEvicted(t, evicted) } - assert.Equal(t, float64(3), promtest.ToFloat64(evictor.evictionsTotal.WithLabelValues(string(resource.CPU)))) + // The eviction signal (closing `evicted`) and the evictionsTotal increment + // both happen in the evictor's background loop, but the signal fires first + // (see running() in evictor.go: Cancel() is called before Inc()). Waiting on + // the signal alone races with the metric write, so poll for the metric too. + require.Eventually(t, func() bool { + return promtest.ToFloat64(evictor.evictionsTotal.WithLabelValues(string(resource.CPU))) == float64(3) + }, 500*time.Millisecond, 5*time.Millisecond, "expected evictionsTotal to reach 3") } func TestNewQueryEvictor_ReturnsNilWhenDisabled(t *testing.T) {