Skip to content

Conversation

@yaooqinn
Copy link
Member

@yaooqinn yaooqinn commented Aug 20, 2023

What changes were proposed in this pull request?

This PR added a button to download thread dump as a txt w.r.t. jstack formatting

Why are the changes needed?

The formatting of raw jstack can be relatively easy to read and analyze using various thread tools.

Does this PR introduce any user-facing change?

image

How was this patch tested?

Raw Dump File

driver.txt

Reporting by external tools

https://fastthread.io/my-thread-report.jsp?p=c2hhcmVkLzIwMjMvMDgvMjAvZHJpdmVyLnR4dC0tMTMtNTMtMjI=&

Was this patch authored or co-authored using generative AI tooling?

no

@zhengruifeng
Copy link
Contributor

cc @gengliangwang @gatorsmile

@HyukjinKwon
Copy link
Member

also @sarutak and @jasonli-db

<div style="display: flex; align-items: center;">
<a class="expandbutton" onClick="expandAllThreadStackTrace(true)">Expand All</a>
<a class="expandbutton d-none" onClick="collapseAllThreadStackTrace(true)">Collapse All</a>
<a class="downloadbutton" href={"data:text/plain;charset=utf-8," + threadDump.map(_.toString).mkString} download={"threaddump_" + executorId + ".txt"}>Download</a>
Copy link
Member

Choose a reason for hiding this comment

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

how about adding "application id" as part of the file name?

Copy link
Member Author

Choose a reason for hiding this comment

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

When it comes to distinguishing files, there are several options such as using app id, attempt id, timestamps, executor details, and more to name the files. However, I prefer to keep it simple to allow for easy renaming when users click the button.

This reverts commit 89f0af5.
This reverts commit 3411da8.
This reverts commit 59813b0.
@github-actions github-actions bot added the BUILD label Aug 21, 2023
@yaooqinn yaooqinn changed the title [WIP][SPARK-44863][UI] Add a button to download thread dump as a txt in Spark UI [SPARK-44863][UI] Add a button to download thread dump as a txt in Spark UI Aug 21, 2023
*/
override def toString: String = {
val sb = new StringBuilder()
val basic = "\"" + threadName + "\" Id=" + threadId + " " + threadState
Copy link
Contributor

Choose a reason for hiding this comment

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

val basic = s""""$threadName" Id=$threadId $threadState"""
sb.append(basic)

or

sb.append("\"").append(threadName).append("\" Id=").append(threadId).append(" ").append(threadState)
// sb.append("\"", threadName, "\" Id=", threadId, " ", threadState) multiple parameters api needs to wait until only Scala 2.13+ is supported.

The former has better readability, while the latter can avoid creating the intermediate string basic.

Copy link
Member Author

Choose a reason for hiding this comment

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

I choose another simple way val sb = new StringBuilder(s""""$threadName" Id=$threadId $threadState""")

@zhengruifeng
Copy link
Contributor

cc @jasonli-db and @gengliangwang would you mind take a look?

Copy link
Contributor

@LuciferYang LuciferYang left a comment

Choose a reason for hiding this comment

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

+1, LGTM

Copy link
Contributor

@jasonli-db jasonli-db left a comment

Choose a reason for hiding this comment

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

LGTM!

@yaooqinn yaooqinn closed this in f7a0b3a Aug 24, 2023
@yaooqinn
Copy link
Member Author

thank you all, merged to master
#42654 is for branch 3.5

@yaooqinn yaooqinn deleted the SPARK-44863 branch August 24, 2023 09:49
@zhengruifeng
Copy link
Contributor

@yaooqinn thank you so much for working on this!

Mrhs121 pushed a commit to Mrhs121/spark that referenced this pull request Jan 2, 2024
…ark UI

### What changes were proposed in this pull request?

This PR added a button to download thread dump as a txt w.r.t. jstack formatting

### Why are the changes needed?

The formatting of raw jstack can be relatively easy to read and analyze using various thread tools.

### Does this PR introduce _any_ user-facing change?

![image](https://github.com/apache/spark/assets/8326978/86c8e87d-970d-4ddb-967e-20c1d3534d42)

### How was this patch tested?

#### Raw Dump File
[driver.txt](https://github.com/apache/spark/files/12388302/driver.txt)

#### Reporting by external tools

https://fastthread.io/my-thread-report.jsp?p=c2hhcmVkLzIwMjMvMDgvMjAvZHJpdmVyLnR4dC0tMTMtNTMtMjI=&

### Was this patch authored or co-authored using generative AI tooling?

no

Closes apache#42575 from yaooqinn/SPARK-44863.

Authored-by: Kent Yao <[email protected]>
Signed-off-by: Kent Yao <[email protected]>

(cherry picked from commit f7a0b3a)
FMX pushed a commit to apache/celeborn that referenced this pull request Nov 7, 2024
### 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 <[email protected]>
Signed-off-by: mingji <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants