Skip to content

Navigation Menu

Sign in
Sign up

Fix flaky compactor tests: ignore user index update loop errors - #7827

Open
CharlieTLe wants to merge 1 commit into
cortexproject:master from
CharlieTLe:fix-flaky-compactor-user-index-logs
Open

Fix flaky compactor tests: ignore user index update loop errors #7827
CharlieTLe wants to merge 1 commit into
cortexproject:master from
CharlieTLe:fix-flaky-compactor-user-index-logs

Conversation

@CharlieTLe

@CharlieTLe CharlieTLe commented Sep 3, 2026

Copy link
Copy Markdown
Member

What this PR does:

Fixes a flake in pkg/compactor where background log output non-deterministically breaks the tests
that assert the compactor's complete log set.

userIndexUpdateLoop runs in the background of every sharding-enabled compactor test and ticks every
UsersScanner.UpdateInterval, which prepare() sets to 100 * time.Millisecond. Each tick calls
ownUser(userID, true), which consults the ring, so any tick that lands before the lifecycler is
ACTIVE — or after it starts LEAVING at shutdown — logs:

level=error component=compactor msg="failed to check if compactor owns updating user index" err="at least 1 live replicas required, could only find 0 - unhealthy instances: 1.2.3.4:0"

The 7 assertions in compactor_test.go and 7 in compactor_paritioning_test.go that compare the full
log output with ElementsMatch then fail with elements differ, depending purely on how many ticks
fell inside the test window.

removeIgnoredLogs already suppresses the other two lines this loop can emit
(successfully updated user index, context timeout, exit user index update loop), but not these
two. Both carry a variable err= payload, so they need a regex rather than an exact map entry —
likely why they were missed when the others were added.

This is a test-synchronisation bug, not a product bug. The errors are legitimate signal in
production, so the fix is in the test helper rather than the loop.

Which issue(s) this PR fixes:
Fixes #7826

How this was verified:

The flake is timing dependent and does not reproduce locally, so this PR adds
TestRemoveIgnoredLogs_UserIndexUpdateLoop to cover the helper directly. I confirmed it fails
without the fix:

--- FAIL: TestRemoveIgnoredLogs_UserIndexUpdateLoop (0.00s)
 Error: Should be empty, but was [level=error component=compactor msg="failed to check if
 compactor owns updating user index" err="at least 1 live replicas required, could only find 0 -
 unhealthy instances: 1.2.3.4:0" level=error component=compactor msg="failed to update user index"
 err="mocked error"]

and passes with it. The full package also passes under the CI configuration:

$ go test -race -tags "netgo slicelabels" -count=1 ./pkg/compactor/...
ok 	github.com/cortexproject/cortex/pkg/compactor	115.057s

Notes:

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]
  • docs/configuration/v1-guarantees.md updated if this PR introduces experimental flags

userIndexUpdateLoop runs in the background of every sharding-enabled
compactor test and ticks every UsersScanner.UpdateInterval, which
prepare() sets to 100ms. Each tick calls ownUser() against the ring, so
any tick landing before the lifecycler is ACTIVE, or after it starts
LEAVING, logs an error.
The tests that assert the compactor's complete log output with
ElementsMatch then fail with "elements differ" depending purely on how
many ticks fell inside the test window. This showed up as
TestCompactor_ShouldCompactAllUsersOnShardingEnabledButOnlyOneInstanceRunning
failing twice in a row on test (arm64), with a different number of extra
log lines each time.
removeIgnoredLogs already suppresses the other two lines this loop can
emit, but not these two, which carry a variable err= payload and so need
a regex rather than an exact match.
Add a unit test for the helper, since the flake itself is timing
dependent and does not reproduce reliably.
Fixes cortexproject#7826
Signed-off-by: Charlie Le <charlie_le@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@friedrichg friedrichg Awaiting requested review from friedrichg friedrichg is a code owner automatically assigned from cortexproject/maintainers

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

1 participant

AltStyle によって変換されたページ (->オリジナル) /