HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException

[smengcl]

Generated-by: Claude Code (Opus 4.8)

What changes were proposed in this pull request?

TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException fails intermittently with Expecting empty but was: [Pipeline-...] on the assertion that the client exclude list holds no pipeline IDs (seen in upstream CI on 2026-07-06, after HDDS-15605 removed the analogous datanodes assertion).

The test forces a container to close and writes again, expecting a ClosedContainerException that adds only the container to the exclude list. Under the default ALL_COMMITTED watch level, a slow or unavailable follower can instead surface a WATCH RaftRetryFailureException. In KeyOutputStream.handleException, checkForRetryFailure returns true for that exception, so the container exclusion branch is skipped and the else branch adds the pipeline to the exclude list. That is intended client behavior, so an empty pipeline set is not an invariant for this configuration.

This PR removes the getPipelineIds().isEmpty() assertion, consistent with how HDDS-15605 handled the datanodes case, and adds a comment explaining why the pipeline set is not an invariant here. The container ID exclusion assertion, which is the actual property under test, is retained. The change is limited to the test.

What is the link to the Apache JIRA

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

How was this patch tested?

Ran the test 100 times locally with 0 failures, each run confirmed to execute the method (Tests run: 1, Failures: 0, Errors: 0). Ran the full TestFailureHandlingByClient class (6 tests, 0 failures). Checkstyle clean.

[Copilot]

Pull request overview

This PR stabilizes an intermittent integration test failure in the Ozone RPC client tests by removing an assertion that is not a valid invariant under the default Ratis ALL_COMMITTED watch level, while keeping the core container-exclusion behavior under test.

Changes:

• Removed the assertion that the client exclude list must have an empty pipeline-ID set in testContainerExclusionWithClosedContainerException.
• Added an explanatory comment documenting why pipeline IDs may legitimately appear in the exclude list under ALL_COMMITTED.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[adoroszlai]

@chihsuan could you please review?

[chihsuan]

Thanks for the patch! @smengcl

I just looked into this a bit more and agree, under ALL_COMMITTED the pipeline can legitimately get excluded, so an empty set isn't guaranteed. LGTM.

One other option would be to pin this test to MAJORITY_COMMITTED so the assertion still holds, but that changes the config being tested, and watch-level exclusion is already covered by testDatanodeExclusionWithMajorityCommit, so dropping it seems better.

Left one small optional note on the doc comment.

[chihsuan]

nit: maybe fold this into the datanode comment above so it's one comment, not two? Something like:

// Only the container-id exclusion is asserted here. Under the default
// ALL_COMMITTED watch level neither the datanode nor the pipeline set is an
// invariant: a slow-but-healthy follower can be recorded as a failed
// datanode, and a WATCH RaftRetryFailureException takes the else-branch in
// KeyOutputStream.handleException and excludes the pipeline. Watch-level
// exclusion is covered by testDatanodeExclusionWithMajorityCommit.

Optional, feel free to skip.