Skip to content

[#2780] fix(client): Fix prefetch executor thread leaks - #2781

Merged
zuston merged 1 commit into
apache:masterfrom
wForget:UINIFFLE-2780
Aug 17, 2026
Merged

[#2780] fix(client): Fix prefetch executor thread leaks#2781
zuston merged 1 commit into
apache:masterfrom
wForget:UINIFFLE-2780

Conversation

@wForget

@wForget wForget commented Aug 17, 2026

Copy link
Copy Markdown
Member

What changes were proposed in this pull request?

This PR fixes the prefetch executor thread leak in client read handlers.

The following changes are included:

  • Invoke super.close() in HadoopShuffleReadHandler.close() to shut down the executor owned by PrefetchableClientReadHandler.

  • Implement MultiReplicaClientReadHandler.close() to close all underlying replica read handlers.

  • Initialize prefetchExecutors with a named daemon thread factory:

    ThreadUtils.getThreadFactory("PrefetchableClientReadHandler")

    The generated thread names follow the PrefetchableClientReadHandler-%d format.

Why are the changes needed?

When prefetch is enabled, each PrefetchableClientReadHandler creates a dedicated executor.

HadoopShuffleReadHandler overrides close() without invoking the parent implementation. In addition, MultiReplicaClientReadHandler does not propagate close() to its underlying handlers. Consequently, the prefetch executors may remain active after shuffle reading finishes.

Under sustained workloads, the leaked threads can exhaust the process's native-thread resources and cause the following error:

java.lang.OutOfMemoryError: unable to create native thread: possibly out of memory or process/resource limits reached
    at java.base/java.lang.Thread.start0(Native Method)
    at java.base/java.lang.Thread.start(Thread.java:809)
    at java.base/java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:945)
    at java.base/java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1353)
    at java.base/java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:123)
    at org.apache.uniffle.storage.handler.impl.PrefetchableClientReadHandler.readShuffleData(PrefetchableClientReadHandler.java:96)
    at org.apache.uniffle.storage.handler.impl.HadoopClientReadHandler.readShuffleData(HadoopClientReadHandler.java:228)
    at org.apache.uniffle.storage.handler.impl.ComposedClientReadHandler.readShuffleData(ComposedClientReadHandler.java:122)
    at org.apache.uniffle.storage.handler.impl.MultiReplicaClientReadHandler.readShuffleData(MultiReplicaClientReadHandler.java:66)

Closing the complete handler hierarchy ensures that the executors are shut down correctly. Using named daemon threads also improves thread identification and prevents prefetch threads from keeping the JVM alive.

Does this PR introduce any user-facing changes?

No.

How was this patch tested?

@github-actions

Copy link
Copy Markdown

Test Results

 3 833 files  ±0   3 833 suites  ±0   8h 5m 20s ⏱️ - 7m 33s
 1 266 tests ±0   1 255 ✅ ±0  11 💤 ±0  0 ❌ ±0 
19 156 runs  ±0  19 108 ✅ ±0  48 💤 ±0  0 ❌ ±0 

Results for commit 7244721. ± Comparison against base commit a080644.

@wForget

wForget commented Aug 17, 2026

Copy link
Copy Markdown
Member Author

@zuston Could you please take a look?

@roryqi
roryqi requested a review from zuston August 17, 2026 07:50

@zuston zuston left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch. thanks @wForget

@zuston
zuston merged commit b1ac32f into apache:master Aug 17, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants