[SPARK-44895][CORE][UI] Add 'daemon', 'priority' for ThreadStackTrace #43095
Conversation
|
cc @dongjoon-hyun @srowen @HyukjinKwon thanks |
| inNative: Boolean) { | ||
| inNative: Boolean, | ||
| isDaemon: Boolean, | ||
| priority: Int) { |
There was a problem hiding this comment.
Has priority ever meant anything for Java threads? I thought it had no effect. Is this useful info?
There was a problem hiding this comment.
The JVM supports a priority-based scheduling algorithm for thread scheduling. For example, Spark UI threads get a priority of 5, while its acceptors get 3. However, given the limited scope of my knowledge, I can't tell whether it actually works or not.
There was a problem hiding this comment.
Yeah, practically it does not convey much given how jvm and linux use thread priority in case of java.
Wondering if it might be distracting for users - they cant really do much about it, while it ends up being a red herring while analyzing performance.
I would be fine with dropping it, given the marginal value (if any) they bring.
mridulm
left a comment
mridulm
left a comment
There was a problem hiding this comment.
Looks good to me, just had one comment.
|
Merged to master |
### What changes were proposed in this pull request? Improve `ThreadStackTrace` with `synchronizers`, `monitors`, `lockName`, `lockOwnerName`, `suspended`, `inNative` for thread dump. ### Why are the changes needed? ThreadStackTrace does not support stack trace including `synchronizers`, `monitors`, `lockName`, `lockOwnerName`, `suspended`, `inNative` at present. It's recommend to improve `ThreadStackTrace` of thread dump for more details of thread stack trace. ### Does this PR introduce _any_ user-facing change? The response of `ThreadStack` in `/api/v1/thread_dump` adds `synchronizers`, `monitors`, `lockName`, `lockOwnerName`, `suspended`, `inNative` fields. Cherry pick: - apache/spark#42575 - apache/spark#43095 ### How was this patch tested? `ApiV1BaseResourceSuite#thread_dump` Closes #2888 from SteNicholas/CELEBORN-1697. Authored-by: SteNicholas <programgeek@163.com> Signed-off-by: mingji <fengmingxiao.fmx@alibaba-inc.com>
4 participants
What changes were proposed in this pull request?
Since version 9, Java has supported the 'daemon' and 'priority' fields in ThreadInfo. In this PR, we extract them from ThreadInfo to ThreadStackTrace
Why are the changes needed?
more information for thread pages in UI and rest APIs
Does this PR introduce any user-facing change?
yes, ThreadStackTrace changes
How was this patch tested?
new tests
Was this patch authored or co-authored using generative AI tooling?
no