Skip to content

HDDS-1159. Fix flaky testGetMatchingContainerMultipleThreads - #11048

Merged
chungen0126 merged 2 commits into
apache:masterfrom
shuan1026:HDDS-1159
Aug 23, 2026
Merged

HDDS-1159. Fix flaky testGetMatchingContainerMultipleThreads#11048
chungen0126 merged 2 commits into
apache:masterfrom
shuan1026:HDDS-1159

Conversation

@shuan1026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

TestContainerStateManagerIntegration#testGetMatchingContainerMultipleThreads was tagged @Flaky("HDDS-1159") because it intermittently failed with expected: <3> but was: <2> (occasionally <1>) at the pipeline container-count assertion.

Root cause: the test fires 100,000 CompletableFuture.supplyAsync(...) calls to ContainerManager.getMatchingContainer on a 4-thread pool, then immediately asserts that the pipeline already has numContainerPerOwnerInPipeline (3) containers without joining those futures. The loop finishing only means the tasks were submitted. When the pool has not yet allocated the 2nd/3rd container, the assertion fails. The existing Thread.sleep(5000) runs after the assertion, so it never stabilizes it.

This is a test timing issue: getMatchingContainer already serializes allocation on pipeline.getId(). Reproduced at 2.5% (5/200) via flaky-test-check (10×20), matching the historical Jira signature.

This PR waits for the work to finish before asserting:

  • Collect the 100k futures and CompletableFuture.allOf(...).join() before checking container count.
  • Remove the misplaced Thread.sleep(5000) and the unused InterruptedException on the method.

What is the link to the Apache JIRA

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

How was this patch tested?

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

LGTM!

@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 @shuan1026 Overall looks good, I left two comments.

@@ -224,7 +224,7 @@ public void testGetMatchingContainer() throws IOException {
@Test
@Flaky("HDDS-1159")

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.

This annotation @Flaky still excludes the test from regular CI. Should we remove it and its unused import?

return null;
}, executor);
}
CompletableFuture.allOf(futures).join();

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.

Could this use a bounded get(timeout, TimeUnit)? join() introduces an unbounded, non-interruptible wait over 100,000 tasks, so a stuck task may hold the test until the Surefire fork timeout.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! Replaced join() with a bounded get(60, TimeUnit.SECONDS).
Verified with a flaky-test-check run: https://github.com/shuan1026/ozone/actions/runs/32497310074

@chungen0126
chungen0126 merged commit 45dc886 into apache:master Aug 23, 2026
30 checks passed
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.

4 participants