Skip to content

[Test][History Server] Fix Build historyserver flaky unit test: TestShutdown_DrainsRetriedTasks - #5245

Open
markwu7 wants to merge 1 commit into
ray-project:masterfrom
markwu7:test/historyserver-deflake-unit-test
Open

[Test][History Server] Fix Build historyserver flaky unit test: TestShutdown_DrainsRetriedTasks#5245
markwu7 wants to merge 1 commit into
ray-project:masterfrom
markwu7:test/historyserver-deflake-unit-test

Conversation

@markwu7

@markwu7 markwu7 commented Sep 5, 2026

Copy link
Copy Markdown

Why are these changes needed?

After checking the log, I found that TestShutdown_DrainsRetriedTasks was flaky in the Build historyserver job. Before the fix, the test waited on !writer.failPending() before calling shutdown(). That flag becomes false inside the failed WriteFile, before retryProcess registers the backoff goroutine on consumerWG. If shutdown() closes stopProducers in that window, retryProcess logs Giving up on retrying ... during shutdown and never uploaded.
So I suggested driving the first failed attempt synchronously via processRotatedFile, ensuring the retry is registered with consumerWG before shutdown() is called. The test then starts the upload worker and runs the real shutdown path: closing stopProducers cuts the backoff short, and consumerWG waits for the final attempt. there is no longer a window where shutdown() can race with retry registration.

Related issue number

Labels

  • If this PR has user-facing changes that require documentation updates at release time, I have added the doc-updates-required label.
  • If this PR contains breaking changes, I have added the breaking-change label.

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

Manual test instructions

I reproduced the CI failure locally by inserting a 50ms sleep between the failed WriteFile and retryProcess in historyserver/pkg/collector/eventcollector/eventcollector.go, which widens the same race. From historyserver/, I ran:
go test -race -count=20 ./pkg/collector/eventcollector/ -run TestShutdown_DrainsRetriedTasks -v
The original test then failed consistently (20/20 runs) with Giving up on retrying ... during shutdown and retried task never uploaded, matching the CI failure log.
After the fix, the same command with the same sleep passed consistently (20/20 runs) because the retry is already registered before shutdown().

Signed-off-by: Mark <markhww.jobs@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant