Repository navigation
HDDS-17254. Prevent client background threads from blocking JVM shutdown - #11667
Merged
smengcl merged 3 commits intoOct 22, 2026
Merged
Conversation
Change-Id: I72728254d74114c33e49f0e5e14d9c81e7577385
Contributor
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
2 open issues
What changed in this PR
Updates Ozone client background thread creation to avoid non-daemon workers keeping the JVM alive after main returns, and adds regression tests that validate JVM shutdown behavior and EC worker retirement.
Changes:
- Convert several client background workers (executors/timers) to daemon threads to prevent JVM shutdown blocking.
- Replace the default EC reconstruction executor with a single-worker executor that retires after idle timeout.
- Add isolated-JVM regression tests covering each affected worker plus EC idle retirement/reuse.
| File | Description |
|---|---|
| hadoop-hdds/client/src/test/java/org/apache/hadoop/hdds/scm/storage/TestClientThreadShutdown.java | Adds isolated-JVM tests ensuring client background workers don’t prevent process exit and EC worker retires when idle. |
| hadoop-hdds/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockInputStreamFactoryImpl.java | Switches default EC reconstruction executor to a daemon-thread ThreadPoolExecutor with idle retirement. |
| hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/DomainSocketFactory.java | Makes the factory timer a daemon timer thread. |
| hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java | Uses a daemon-thread single-thread executor for streaming write response handling. |
| hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientShortCircuit.java | Makes the short-circuit connection timer a daemon timer thread. |
Review effort: Lite
Comment on lines
+75
to
+77
| this(new ElasticByteBufferPool(), () -> new ThreadPoolExecutor(0, 1, 60, TimeUnit.SECONDS, | ||
| new LinkedBlockingQueue<>(), new ThreadFactoryBuilder() | ||
| .setNameFormat("EC-reconstruction-%d").setDaemon(true).build())); |
| OzoneClientConfig socketConfig = conf.getObject(OzoneClientConfig.class); | ||
| socketConfig.setShortCircuit(true); | ||
| conf.setFromObject(socketConfig); | ||
| conf.set(OzoneClientConfig.OZONE_DOMAIN_SOCKET_PATH, "/tmp/ozone-client-shutdown.sock"); |
Change-Id: Ieddf3905300bcc3850ede5b33ce665869df648c1
Copilot stopped reviewing on behalf of
jojochuang due to an error
October 21, 2026 21:04
smengcl
marked this pull request as ready for review
October 22, 2026 00:21
Validate the retained client reference before signaling worker readiness, declare fields before the enum, and make the Ratis launcher client a local variable. Co-authored-by: Codex (GPT-6) Change-Id: I3b8d60c739f740937fe17ac6f36fdeb94aa05755
Contributor
|
Thanks @jojochuang for the patch. |
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
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.
What changes were proposed in this pull request?
Client background executors and timers can keep the JVM alive after the application returns from
mainif the associated resources remain open. This patch makes the streaming-write response executor, short-circuit connection timer, domain socket factory timer, and default low-level EC reconstruction executor use daemon threads.The default EC reconstruction executor also retires its worker after 60 seconds of inactivity while retaining single-worker task execution. Caller-supplied executors and existing explicit cleanup paths are unchanged. Applications still need to close streams to complete writes.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-17254
How was this patch tested?
RpcClientwrite executor and aRatisBlockOutputStreamagainst the mock datanode pipeline. Reversing the original daemon-thread setting makes the child JVM stay alive past the 30-second timeout; restoring the setting makes it exit normally. The shared stream setup and exit assertion live inTestClientThreadShutdown, with the higher-level test inTestRpcClientto avoid a module dependency cycle.TestClientThreadShutdown,TestBlockDataStreamOutput,TestBlockInputStreamFactoryImpl,TestECBlockInputStream,TestECBlockReconstructedInputStream, andTestECBlockReconstructedStripeInputStream. The original five shutdown/idle cases took approximately 3 seconds in a standalone rerun. After adding Ratis coverage, all 184 tests acrossTestClientThreadShutdownand the fullTestRpcClientclass passed.Validation used JDK 21 with
-DskipShade -DskipRecon -DskipDocs.Generated-by: Codex (GPT-6)