Skip to content

test: wait for handoff notice completion in supervisor state test - #20453

Merged
FrankChen021 merged 1 commit into
apache:masterfrom
gitedmond:test/fix-20429-notice-race
Sep 30, 2026
Merged

FrankChen021 merged 1 commit into
apache:masterfrom
gitedmond:test/fix-20429-notice-race

Conversation

@gitedmond

Copy link
Copy Markdown
Contributor

Fixes #20429.

Description

testSupervisorStopTaskGroupEarly waited for an empty notice queue before its second runInternal() call. The notice thread removes a handoff notice from the queue before handling it, so the test could run before handoffEarly was set and miss the expected task shutdown.

The test now waits on a latch released from emitNoticeProcessTime only after the handoff_task_group_notice handler completes. The wait is bounded to five seconds under the existing ten-second test timeout. Production behavior is unchanged.

Validation

  • Baseline on upstream master, JDK 25, Surefire retries disabled: the target test passed 1/1 locally. The intermittent failure was documented in the issue's CI jobs and did not reproduce in this single local run.
  • After the change, the target test passed in 10/10 independent Maven runs with -Dsurefire.rerunFailingTestsCount=0.
  • The full SeekableStreamSupervisorStateTest class passed: 61 tests, 0 failures, 0 errors, 0 skipped, with retries disabled.
  • mvn -pl indexing-service checkstyle:check: 0 violations.
  • git diff --check: passed.
  • mvn validate -pl indexing-service -am -T1C did not reach indexing-service: PMD in the AWS and GCP common modules could not resolve the reactor's druid-processing snapshot jar during the validate phase. The changed module's Checkstyle check was run directly as noted above.

The test commands used -pl indexing-service -am -Dsurefire.failIfNoSpecifiedTests=false -Dsurefire.rerunFailingTestsCount=0 -Pskip-static-checks -Dweb.console.skip=true -T1C, selecting the target method for the repeated runs and the full class for the class run.

Key changed classes
  • SeekableStreamSupervisorStateTest: wait for the handoff notice to finish before asserting its effect.

This PR has:

  • been self-reviewed, including the concurrency checklist.
  • updated the existing regression test for the notice handling race.

@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 the reviewed change. The new latch synchronizes the test with completion of the handoff notice handler, so the subsequent run observes the intended state transition instead of relying on queue size.

Reviewed 1 of 1 changed files.

Validation: GitHub reported the pull request as MERGEABLE with CLEAN merge state, local git merge-tree verification succeeded against origin/master, and git diff --check passed. No test commands were run during this review.


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

@FrankChen021
FrankChen021 merged commit e5a0931 into apache:master Sep 30, 2026
54 checks passed
@github-actions github-actions Bot added this to the 39.0.0 milestone Sep 30, 2026
@gitedmond
gitedmond deleted the test/fix-20429-notice-race branch September 30, 2026 13:09
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: SeekableStreamSupervisorStateTest.testSupervisorStopTaskGroupEarly

2 participants