Skip to content

HDDS-16134. Fix flaky testPipelineExclusionWithPipelineFailure - #11062

Open
Eason09053360 wants to merge 1 commit into
apache:masterfrom
Eason09053360:HDDS-16134
Open

HDDS-16134. Fix flaky testPipelineExclusionWithPipelineFailure#11062
Eason09053360 wants to merge 1 commit into
apache:masterfrom
Eason09053360:HDDS-16134

Conversation

@Eason09053360

@Eason09053360 Eason09053360 commented Aug 19, 2026

Copy link
Copy Markdown

What changes were proposed in this pull request?

testPipelineExclusionWithPipelineFailure intermittently fails on the getDatanodes() assertion:

Expecting empty but was: [c587cf9d-ea5d-466b-a23b-e96f6cfca97f(null/null)]

The test shuts down two of the three datanodes, so watchForCommit can time out and report them as failed servers. KeyOutputStream.handleExceptionInternal adds those to the exclude list independently of the pipeline exclusion the test is actually checking, so an empty datanode set is not an invariant here - it just depends on whether the watch reply lands first.

This drops the getDatanodes() assertion. The pipeline assertion, which is the property under test, stays.

Same fix as HDDS-15823 (a8fcb09), which removed the equivalent assertions from testContainerExclusionWithClosedContainerException in this file.

What is the link to the Apache Jira

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

How was this patch tested?

flaky-test-check 10x10 (100 runs), all splits green:
https://github.com/Eason09053360/ozone/actions/runs/32264781480

checkstyle clean.

Generated-by: Claude Code (Claude Opus 5)

Copilot AI lite review requested due to automatic review settings August 19, 2026 12:23
@Eason09053360 Eason09053360 changed the title HDDS-16134. Fix flaky TestFailureHandlingByClient#testPipelineExclusionWithPipelineFailure HDDS-16134. Fix flaky testPipelineExclusionWithPipelineFailure Aug 19, 2026

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.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR reduces test flakiness and corrects Recon container-health test setup by removing a non-invariant assertion and ensuring an “empty container” scenario is exercised deterministically.

Changes:

  • Removes a flaky assertion that the exclude list’s datanodes set is always empty in testPipelineExclusionWithPipelineFailure.
  • Updates Recon container-health test documentation to reflect the “empty container” reasoning more accurately.
  • Adjusts Recon test setup to create an empty container without writing a block.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestFailureHandlingByClient.java Drops a non-deterministic datanode-exclude assertion and documents why.
hadoop-ozone/integration-test-recon/src/test/java/org/apache/hadoop/ozone/recon/TestReconTasks.java Clarifies test rationale and changes setup to avoid writing blocks so the container remains “empty.”

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

@chihsuan chihsuan 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.

Thanks for the patch! LGTM +1 @Eason09053360 I checked the exclude list logic and can confirm your analysis, that assertion was never an invariant here.

One thing: could we run flaky-test-check GH workflow (10x10) and link the run in the PR description? That would help confirm the test is stable.

@Eason09053360

Copy link
Copy Markdown
Author

Thanks for the patch! LGTM +1 @Eason09053360 I checked the exclude list logic and can confirm your analysis, that assertion was never an invariant here.

One thing: could we run flaky-test-check GH workflow (10x10) and link the run in the PR description? That would help confirm the test is stable.

Thanks for reviewing !
Sure, I will take a look at it later :)

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.

3 participants