Repository navigation
HDDS-16982. Skip stopped datanodes in TestFailureHandlingByClient#waitForBlockContainer - #11525
Conversation
…tForBlockContainer waitForBlockContainer polled every datanode, including the two that testBlockWritesWithDnFailures had just shut down. For those, the volume DB was already closed and removed from DatanodeStoreCache, so BlockUtils.getDB reopened it and kept it cached. When the next test restarted that datanode, loading the volume DB failed with "lock hold by current process", the volume check failed the only data volume, and the datanode called ExitUtil.terminate, which killed the test JVM. Skip stopped datanodes so the test never reopens their DB. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
0 open findings
What changed in this PR
Fixes a test-JVM crash caused by waitForBlockContainer polling stopped datanodes whose volume DBs have been closed/evicted, which can later prevent DB locking on restart.
Changes:
- Skip stopped datanodes while polling for a container in
waitForBlockContainer. - Add an inline comment explaining the DB-reopen/lock-failure mechanism being avoided.
| File | Description |
|---|---|
| hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestFailureHandlingByClient.java | Avoids interacting with stopped datanodes during container polling to prevent DB cache/lock issues that crash subsequent tests. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
jojochuang
left a comment
jojochuang
left a comment
There was a problem hiding this comment.
Approve with the caveat that this does not eliminate all flakiness in the class.
The root cause for the cross-test JVM crash is convincing: after HDDS-16224, waitForBlockContainer polls every DN; BlockUtils.getDB on a stopped DN reopens and caches the volume DB in the test JVM, so @BeforeEach restart hits "lock hold by current process" and ExitUtil.terminate. Skipping dn.isStopped() is the right minimal fix and matches the pattern in TestPerVolumePushReplication#isHealthyDatanode.
Fork 10×10 flaky-test-check improved a lot (74/100 failed on baseline vs 6/100 on this branch), but 6 failures remain—please keep tracking those separately (e.g. HDDS-13972) and update the PR description with the final flaky-run numbers when convenient.
| .contains(datanodes.get(0)); | ||
| } | ||
| assertThat(keyOutputStream.getExcludeList().getContainerIds()).isEmpty(); | ||
| assertThat(keyOutputStream.getExcludeList().getPipelineIds()).isEmpty(); |
|
After this PR, the flakiness is in another location: |
|
Despite there are still flakiness, they are unrelated so i went ahead merged the PR. We can fix the other problem later. |
|
Thanks @jojochuang for the review. |
Generated-by: Claude Code (Opus 5.5)
What changes were proposed in this pull request?
TestFailureHandlingByClientcrashes the forked test JVM when any test runs aftertestBlockWritesWithDnFailures.That test shuts down two datanodes, then calls
waitForBlockContainer, which since HDDS-16224 polls every datanode. For a stopped datanode the volume DB is already closed and removed fromDatanodeStoreCache, soBlockUtils.getDBreopens it and keeps it cached. WhenrestartDownDataNodesrestarts that datanode for the next test, loading the volume DB fails withlock hold by current process, the volume check fails its only data volume, and the datanode callsExitUtil.terminate.Since HDDS-15749, CI for a PR that touches this class runs it by name, which includes the
@Flakytest aftertestBlockWritesWithDnFailures, so the crash now fails CI for such PRs (e.g. #11506).The fix skips stopped datanodes in
waitForBlockContainer.What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-16982
How was this patch tested?
🤖 Generated with Claude Code