xingsuo-zbz opened a new pull request, #28732:
URL: https://github.com/apache/flink/pull/28732
## What is the purpose of the change
Fixes FLINK-39984. Clicking "Thread Dump" on a JobManager or TaskManager
in the Web UI can cause the targeted
process to miss heartbeats and be marked dead by the JobManager, taking
down otherwise healthy running jobs. Two
independent issues compound:
1. TaskExecutor#requestThreadDump and Dispatcher#requestThreadDump execute
ThreadDumpInfo.dumpAndCreate
synchronously on the RPC main thread. While the dump is being built the
actor mailbox does not advance, so heartbeat
replies, task lifecycle messages, and checkpoint coordination all queue
up behind it. Other heavy handlers in
TaskExecutor (requestLogList, requestFileUploadByFilePath,
updatePartitions) are already offloaded to ioExecutor —
this one was not.
2. JvmUtils#createThreadDump unconditionally calls
ThreadMXBean.dumpAllThreads(true, true). Collecting locked
monitors and j.u.c. synchronizers requires walking every thread's lock
state inside a single JVM safepoint. On JVMs
with many threads (Netty + RocksDB + async I/O + user threads — 10k+ in
production is not uncommon) this takes
seconds to tens of seconds, during which every thread in the JVM
(including the heartbeat dispatcher) is paused. If
the safepoint plus mailbox queueing exceeds heartbeat.timeout (default 50
s), the JM triggers an unnecessary
failover.
Offloading alone helps short dumps but not long ones (the safepoint pauses
the heartbeat thread regardless of which
executor scheduled the dump). Restricting the safepoint work alone leaves
the mailbox stall in place. Both fixes are
needed.
## Brief change log
- Offload the dump to ioExecutor in both TaskExecutor#requestThreadDump
and Dispatcher#requestThreadDump via
CompletableFuture.supplyAsync, matching the pattern already used by other
heavy TE handlers.
- Introduce ThreadDumpMode {SAFE, FULL} and thread it through
RestfulGateway / TaskExecutorGateway /
ResourceManagerGateway / TaskExecutorGatewayDecoratorBase /
NonLeaderRetrievalRestfulGateway.
- SAFE → dumpAllThreads(false, false) — stack traces only, negligible
JVM pause.
- FULL → dumpAllThreads(true, true) — current behavior, retains
locked-monitor / synchronizer info for deadlock
analysis.
- Expose the mode as an optional REST query parameter ?mode=safe|full on
both /jobmanager/thread-dump and
/taskmanagers/{id}/thread-dump via a new ThreadDumpModeQueryParameter and
two new MessageParameters classes; unknown
values become HTTP 400.
- Add config option cluster.thread-dump.default-mode (default FULL) that
controls the fallback when no query
parameter is supplied. Default is deliberately FULL to preserve the
existing on-upgrade behavior; the description
flags SAFE as the recommended value for large clusters. A separate dev@
discussion will decide whether to flip the
default in a future release.
- Web UI: add a Safe / Full toggle to both thread-dump pages
(flink-runtime-web/web-dashboard/.../{job,task}-manager/thread-dump/).
Selecting a mode does not auto-fetch; the
user must press the refresh button (avoids surprising the operator when
they merely click the toggle). The download
link tracks the current selection so exported dumps match the on-screen
content.
## Verifying this change
This change added tests and can be verified as follows:
New unit tests
- ThreadDumpModeQueryParameterTest — 6 cases covering key name,
optionality, case-insensitive parsing, unknown value
→ IllegalArgumentException, and lower-case serialization.
- TaskExecutorThreadDumpTest — spins up a real TaskExecutor and verifies
that (a) requestThreadDump returns a
non-empty dump for null, SAFE, and FULL, and (b) when mode is omitted the
request honors
cluster.thread-dump.default-mode (the test overrides it to SAFE and
asserts the "Number of locked synchronizers"
section is absent, which only appears in FULL).
Existing tests updated
- TestingResourceManagerGateway, TestingTaskExecutorGateway,
TestingRestfulGateway — signature updates.
- rest_api_v1.snapshot — new mode query parameter recorded for both
endpoints; RuntimeRestAPIStabilityTest passes
against the updated snapshot.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: yes (Before this
change, requesting a thread dump on a busy TM could cause a spurious Heartbeat
of TaskManager timed out. After this change the
request no longer blocks the RPC mailbox and when the operator selects
SAFE, the
safepoint pause is short enough not to trip heartbeat.timeout.)
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? yes — the new
ThreadDumpMode REST parameter and `cluster.thread-dump.default-mode` config
option.
- If yes, how is the feature documented? docs
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Claude Opus 4.7)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
