You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "smengcl (via GitHub)" <gi...@apache.org> on 2026/07/10 20:19:38 UTC
[PR] HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException [ozone]
smengcl opened a new pull request, #10721:
URL: https://github.com/apache/ozone/pull/10721
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.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #10721:
URL: https://github.com/apache/ozone/pull/10721#issuecomment-4977556562
Thanks @chihsuan @jojochuang for reviewing this.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #10721:
URL: https://github.com/apache/ozone/pull/10721#issuecomment-4974559917
Merged. Thanks @chihsuan and @smengcl
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #10721:
URL: https://github.com/apache/ozone/pull/10721#issuecomment-4945428750
@chihsuan could you please review?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException [ozone]
Posted by "chihsuan (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #10721:
URL: https://github.com/apache/ozone/pull/10721#discussion_r3573445724
##########
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:
Good point. Done!
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15823. Fix flaky TestFailureHandlingByClient#testContainerExclusionWithClosedContainerException [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #10721:
URL: https://github.com/apache/ozone/pull/10721
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org