Skip to content

[SPARK-44895][CORE][UI] Add 'daemon', 'priority' for ThreadStackTrace #43095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
yaooqinn wants to merge 1 commit into from
Closed

[SPARK-44895][CORE][UI] Add 'daemon', 'priority' for ThreadStackTrace #43095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions core/src/main/scala/org/apache/spark/status/api/v1/api.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,19 +540,21 @@ case class ThreadStackTrace(
lockName: Option[String],
lockOwnerName: Option[String],
suspended: Boolean,
inNative: Boolean) {
inNative: Boolean,
isDaemon: Boolean,
priority: Int) {
Copy link
Member

@srowen srowen Sep 26, 2023

Choose a reason for hiding this comment

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

Has priority ever meant anything for Java threads? I thought it had no effect. Is this useful info?

Copy link
Member Author

@yaooqinn yaooqinn Sep 26, 2023

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@mridulm mridulm Sep 27, 2023

Choose a reason for hiding this comment

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

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.


/**
* Returns a string representation of this thread stack trace
* w.r.t java.lang.management.ThreadInfo(JDK 8)'s toString.
*
* TODO(SPARK-44895): Considering 'daemon', 'priority' from higher JDKs
*
* TODO(SPARK-44896): Also considering adding information os_prio, cpu, elapsed, tid, nid, etc.,
* from the jstack tool
*/
override def toString: String = {
val sb = new StringBuilder(s""""$threadName" Id=$threadId $threadState""")
val daemon = if (isDaemon) " daemon" else ""
val sb = new StringBuilder(
s""""$threadName"$daemon prio=$priority Id=$threadId $threadState""")
lockName.foreach(lock => sb.append(s" on $lock"))
lockOwnerName.foreach {
owner => sb.append(s"""owned by "$owner"""")
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/scala/org/apache/spark/util/Utils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2198,7 +2198,9 @@ private[spark] object Utils
Option(threadInfo.getLockName),
Option(threadInfo.getLockOwnerName),
threadInfo.isSuspended,
threadInfo.isInNative)
threadInfo.isInNative,
threadInfo.isDaemon,
threadInfo.getPriority)
}

/**
Expand Down
14 changes: 14 additions & 0 deletions core/src/test/scala/org/apache/spark/ui/UISeleniumSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,20 @@ class UISeleniumSuite extends SparkFunSuite with WebBrowser with Matchers {
}
}

test("SPARK-44895: Add 'daemon', 'priority' for ThreadStackTrace") {
withSpark(newSparkContext()) { sc =>
val uiThreads = getJson(sc.ui.get, "executors/driver/threads")
.children
.filter(v => (v \ "threadName").extract[String].matches("SparkUI-\\d+"))
val priority = Thread.currentThread().getPriority

uiThreads.foreach { v =>
assert((v \ "isDaemon").extract[Boolean])
assert((v \ "priority").extract[Int] === priority)
}
}
}

def goToUi(sc: SparkContext, path: String): Unit = {
goToUi(sc.ui.get, path)
}
Expand Down