Skip to content

test: avoid racing supervisor history read - #20458

Merged
FrankChen021 merged 1 commit into
apache:masterfrom
gitedmond:test/avoid-supervisor-history-race
Oct 1, 2026
Merged

FrankChen021 merged 1 commit into
apache:masterfrom
gitedmond:test/avoid-supervisor-history-race

Conversation

@gitedmond

Copy link
Copy Markdown
Contributor

Fixes #20445.

Description

This embedded test runs the killSupervisors duty every 100 ms with zero retention. After terminating a supervisor, it tried to read the two history entries before the duty deleted them. The reported CI failure was a 404 at that intermediate read.

Remove the timing-dependent read. The test still verifies the initial supervisor history, waits for the cleanup metric to count both deleted entries, and checks that the history endpoint then returns 404. Tombstone behavior is covered by the separate supervisor and metadata tests.

Validation

  • Before the change, the cited JDK 25 CI job failed at the intermediate history read. I did not reproduce this intermittent race locally.
  • After the change, the full KillSupervisorsCustomDutyTest class passed in four Maven reactor runs with -Dsurefire.rerunFailingTestsCount=0: 4/4 runs passed.
  • mvn -B checkstyle:check -pl embedded-tests -DskipTests=true -Dweb.console.skip=true: passed with 0 violations.
  • git diff --check: passed.

This changes only the embedded test; no production behavior or public API changes.

This PR has:

  • been self-reviewed.
  • updated an existing integration test to remove a timing-dependent assertion.

@FrankChen021 FrankChen021 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No actionable issues found in this one-file review. The removed read was timing-dependent under the 100 ms cleanup duty; the test still validates the initial history, waits for the cleanup metric to report both rows removed, and verifies that the history endpoint returns 404. Existing surrounding unit and resource tests cover NoopSupervisorSpec tombstone behavior and history serialization.

Reviewed 1 of 1 changed files.

Validation: Static inspection and git diff --check 0ad366d5d4a2bf3b1d35dd7f515c2302c5731e7e 500609b28df662ddf915c752b22229f3429f7448 -- embedded-tests/src/test/java/org/apache/druid/testing/embedded/server/KillSupervisorsCustomDutyTest.java passed. No builds or tests were run.


This is an automated review by Codex GPT-5.6-Luna(max)

@FrankChen021
FrankChen021 merged commit 631f05d into apache:master Oct 1, 2026
27 checks passed
@github-actions github-actions Bot added this to the 39.0.0 milestone Oct 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky test: KillSupervisorsCustomDutyTest.test_customDuty_removesHistoryOfTerminatedSupervisor

2 participants