HADOOP-19986. MiniDFSCluster.shutdownDataNodes() should signal all DataNodes before joining any - #8723
Merged
Conversation
…taNodes before joining any shutdownDataNodes() stopped and joined each DataNode in turn, so the ones not yet reached kept retrying a NameNode the test had already killed. DataNodes in one JVM share an ipc.Client through ClientCache, and so its per-address Connection objects. A surviving BPServiceActor holds a Connection monitor across its connect-retry sleeps, while an actor of the DataNode being joined sits BLOCKED on that monitor. Interrupts do not reach a BLOCKED thread, so stop() is ineffective and the join waits on scheduling luck: measured at 165s, overrunning the 180s timeout of TestBalancerWithHANameNodes#testBalancerWithObserverWithFailedNode. Signal every DataNode before joining any, so the monitor holder aborts its sleep and releases. Adds BlockPoolManager#signalShutDownAll and DataNode#signalBlockPoolShutdown; stop() is idempotent, so the per-DataNode shutdown path is unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
|
@pan3793 @slfan1989 This PR is Green and ready to be reviewed. This one solves race conditions on minicluster when asked to be shutdown, it hanged on the closing datanodes. This surfaced because up to a month ago minicluster was not shutdown in the tests, leaked. I worked in the jiras to fix that. Properly closing the miniclusters and eliminating this race condition improves CI testing. It is a prerrequisite for HADOOP-19979, that will fix 4 flaky conditions so that we reduce later the GHA exclude list, as explained on HADOOP-19981 [umbrella for all GHA reactivation candidates from excluded-tests.txt] and its subtasks [specific identified candidates for reactivation]. |
Contributor
|
Committed to trunk. Thanks @joseluisll |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
https://issues.apache.org/jira/browse/HADOOP-19986
MiniDFSCluster.shutdownDataNodes()tears DataNodes down strictly serially - stop one, join it, move to the next. The DataNodes not yet reached keep retrying a NameNode the test has already killed.DataNodes in a single JVM share an
ipc.ClientthroughClientCache, and therefore share its per-addressConnectionobjects. A surviving DataNode'sBPServiceActorholds aConnectionmonitor across its connect-retry sleeps inhandleConnectionFailure, while an actor of the DataNode being joined sits BLOCKED on that same monitor inClient.addCall.Thread.interrupt()cannot dislodge a BLOCKED thread, so thestop()issued byBlockPoolManager.shutDownAllis ineffective and the join waits on scheduling luck: the holder releases and re-acquires roughly every 2s, and unfair monitors starve the blocked thread.Measured on a 2-core runner: DataNode 2 took 165s to shut down, after which DN1 and DN0 finished in ~10ms. That overran the 180s timeout of
TestBalancerWithHANameNodes#testBalancerWithObserverWithFailedNode, and runs that stayed under the deadline still burned ~115s in teardown.The fix signals every DataNode before joining any, so the monitor holder aborts its sleep and releases it:
BlockPoolManager#signalShutDownAll- the stop-without-join half of the existingshutDownAll, which now delegates to it.DataNode#signalBlockPoolShutdown-@VisibleForTesting, null-safe, signals every block pool service without waiting.MiniDFSCluster#shutdownDataNodes- signals all DataNodes up front, then runs the existing per-DataNode shutdown loop unchanged.stop()is idempotent, so the per-DataNode shutdown path behaves exactly as before; the only change is that the interrupts now all land before the first join.How was this patch tested?
20 runs of
TestBalancerWithHANameNodes#testBalancerWithObserverWithFailedNodeon a 2-core runner. Before: 4 of 20 anomalous (182.3s, 181.3s, 116.7s, 113.9s). After: 20 of 20 passed within 44-48s.Full CI on the fork, all jobs green -
common,hdfs - other,hdfs - slow,hdfs-rbf,mr,other,yarn-server-rmon Java 17, plus build-only on Java 21 and Java 25:https://github.com/joseluisll/hadoop/actions/runs/34147776139
That run was on commit
1700e04b; this branch has since been rebased onto current trunk with no change to the patch itself.For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
Contains content generated by Claude Code.