Skip to content

fix(queryeviction): fix flaky TestPrometheusMetrics_IncrementedCorrectly #7680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
krunaljain wants to merge 1 commit into from
+8 −1
Closed

fix(queryeviction): fix flaky TestPrometheusMetrics_IncrementedCorrectly #7680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion pkg/util/queryeviction/evictor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down