Eason09053360 opened a new pull request, #11062:
URL: https://github.com/apache/ozone/pull/11062
## What changes were proposed in this pull request?
`testPipelineExclusionWithPipelineFailure` intermittently fails on:
```
Expecting empty but was: [c587cf9d-ea5d-466b-a23b-e96f6cfca97f(null/null)]
```
The test shuts down two of the three datanodes in the pipeline, then asserts
that the failed pipeline lands in the exclude list. Alongside that it also
asserts `getExcludeList().getDatanodes()` is empty, which is not an invariant.
In `KeyOutputStream.handleExceptionInternal`, failed servers are added to
the exclude list independently of the container-vs-pipeline branch:
```java
Collection<DatanodeDetails> failedServers = streamEntry.getFailedServers();
if (!failedServers.isEmpty()) {
excludeList.addDatanodes(failedServers);
}
if (containerExclusionException) {
excludeList.addConatinerId(...);
} else {
excludeList.addPipeline(pipelineId);
}
```
`failedServers` is populated by `BlockOutputStream.checkReply` from the
watch reply. Under the default ALL_COMMITTED watch level,
`XceiverClientRatis.watchForCommit` falls back to MAJORITY_COMMITTED when the
ALL_COMMITTED watch fails, and `handleFailedAllCommit` returns the lagging
datanodes - which are exactly the two nodes the test shut down. Whether that
watch reply lands before the write failure surfaces is timing dependent (the
test configures both the write request timeout and the watch timeout at 3s), so
the datanode set is sometimes empty and sometimes not.
This patch drops the `getDatanodes()` assertion. The property under test -
the failed pipeline is added to the exclude list - is still asserted.
There is a direct precedent:
`testContainerExclusionWithClosedContainerException` in the same file had its
datanode and pipeline assertions removed for exactly this reason in HDDS-15823
(commit a8fcb09c02). This is the other half of the same problem, so the same
approach is applied here for consistency.
Watch-level datanode exclusion remains covered by
`testDatanodeExclusionWithMajorityCommit`, which pins the watch level to
MAJORITY_COMMITTED and is therefore deterministic.
## What is the link to the Apache Jira
https://issues.apache.org/jira/browse/HDDS-16134
## How was this patch tested?
Ran `TestFailureHandlingByClient#testPipelineExclusionWithPipelineFailure`
15 times locally: 15 passed, 0 failed. `checkstyle.sh` reports no violations.
Note that the local runs do not by themselves prove the flake is gone, since
the failure depends on CI timing that is hard to reproduce on an unloaded
machine; the correctness of the change rests on the code-path analysis above.
Generated-by: Claude Code (Claude Opus 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
