Skip to content

HDDS-16934. Fix intermittent failure in TestSCMNodeManager#testDatanodeFinalizedCounterExcludesNonHealthyNodes - #11482

Merged
jojochuang merged 1 commit into
apache:masterfrom
smengcl:HDDS-16934-flaky
Oct 12, 2026
Merged

jojochuang merged 1 commit into
apache:masterfrom
smengcl:HDDS-16934-flaky

Conversation

@smengcl

@smengcl smengcl commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Generated-by: Claude Code (Opus 5.5)

What changes were proposed in this pull request?

TestSCMNodeManager#testDatanodeFinalizedCounterExcludesNonHealthyNodes fails intermittently for the DEAD parameter:

IN_SERVICE(no expiry)-DEAD node should be excluded from total count ==> expected: <1> but was: <0>

The test sets the stale node interval to 3s and drives node state with fixed Thread.sleep calls. In the DEAD case the last heartbeat of the baseline node is followed by Thread.sleep(3000), so that heartbeat is at least 3s old when the assertions run. If the node health check (100ms period) fires between the end of the sleep and the assertions, the baseline node itself is marked STALE and the healthy datanode count is 0 instead of 1.

This PR replaces the fixed sleeps with GenericTestUtils.waitFor that keeps heartbeating the baseline node until the other node reaches the expected state, the same pattern other tests in this class already use. The baseline heartbeat is then at most about 100ms old at assertion time, far below the 3s stale node interval. Test only change.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-16934

How was this patch tested?

  • Confirmed the cause: with a temporary 150ms sleep added before the assertions, the DEAD case failed every time with the baseline node reported as IN_SERVICE-STALE.
  • With the fix, 15 consecutive local runs passed: mvn -pl :hdds-server-scm test -Dtest='TestSCMNodeManager#testDatanodeFinalizedCounterExcludesNonHealthyNodes' -DskipShade -DskipRecon -DskipDocs
  • 10x10 flaky-test-check on this branch: passed 100/100

🤖 Generated with Claude Code

…deFinalizedCounterExcludesNonHealthyNodes

In the DEAD case the last heartbeat of the baseline node was followed by
Thread.sleep(3000), which equals the stale node interval. If the node health
check ran between the end of the sleep and the assertions, the baseline node
was marked STALE and the healthy datanode count was 0 instead of 1.

Replace the fixed sleeps with GenericTestUtils.waitFor that keeps heartbeating
the baseline node until the other node reaches the expected state.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings October 12, 2026 05:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The test-only fix addresses the intermittent failure and has been validated by repeated runs.

0 open findings

What changed in this PR

Fixes an intermittent SCM node health test failure by replacing timing-sensitive sleeps with polling while keeping the baseline node heartbeating.

Changes:

  • Uses GenericTestUtils.waitFor for state transitions.
  • Adds timeout handling for asynchronous waits.
File Description
hadoop-hdds/​server-scm/​src/​test/​java/​org/​apache/​hadoop/​hdds/​scm/​node/​TestSCMNodeManager.java Makes stale/dead node transition testing deterministic.

🧠 Review effort: Lite


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions github-actions Bot added the scm label Oct 12, 2026
@smengcl
smengcl marked this pull request as ready for review October 12, 2026 06:45

@jojochuang jojochuang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

@jojochuang
jojochuang merged commit 52fa36e into apache:master Oct 12, 2026
40 checks passed
@jojochuang

Copy link
Copy Markdown
Contributor

Merged. Thanks @smengcl

@smengcl

smengcl commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @jojochuang for the review.

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.

3 participants