chihsuan commented on code in PR #10721:
URL: https://github.com/apache/ozone/pull/10721#discussion_r3564186768
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestFailureHandlingByClient.java:
##########
@@ -400,7 +400,12 @@ public void
testContainerExclusionWithClosedContainerException()
// level is configurable via RatisClientConfig watchType, HDDS-2887).
// Watch-level datanode exclusion is covered by
// testDatanodeExclusionWithMajorityCommit.
- assertThat(keyOutputStream.getExcludeList().getPipelineIds()).isEmpty();
+ // Pipeline IDs are also not asserted here. Under ALL_COMMITTED a WATCH
+ // RaftRetryFailureException can fire (retryFailure=true) before the
+ // ClosedContainerException path runs; that takes the else-branch in
+ // KeyOutputStream.handleException and adds the pipeline to the exclude
+ // list, so an empty pipeline set is not an invariant for this config.
+ // The container-id assertion above is the actual property under test.
Review Comment:
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.
--
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]
