dongjoon-hyun commented on code in PR #58054:
URL: https://github.com/apache/spark/pull/58054#discussion_r3815696649
##########
core/src/main/scala/org/apache/spark/ui/jobs/JobsTab.scala:
##########
@@ -62,4 +66,85 @@ private[ui] class JobsTab(parent: SparkUI, store:
AppStatusStore)
}
}
}
+
+ // Serves the hold/resume requests off the Jetty serving thread: they talk
to the cluster
+ // manager and may block up to the RPC ask timeout. A single thread also
serializes
+ // concurrent requests. Created on first use and shut down by `stop()`, so
that the thread
+ // does not outlive the SparkContext.
+ private var holdRequestExecutor: Option[ExecutorService] = None
+ private var stopped = false
+
+ // None once stopped, so that a request served during teardown neither hits
a rejected
+ // execution on the shut-down pool nor recreates it and leaks the thread.
+ private def holdRequestExecutorPool: Option[ExecutorService] = synchronized {
+ if (stopped) {
+ None
+ } else {
+ Some(holdRequestExecutor.getOrElse {
+ val pool =
ThreadUtils.newDaemonSingleThreadExecutor("spark-ui-hold-resume")
+ holdRequestExecutor = Some(pool)
+ pool
+ })
+ }
+ }
+
+ def stop(): Unit = synchronized {
+ stopped = true
+ holdRequestExecutor.foreach(_.shutdownNow())
+ }
+
+ // Outcome of the last hold/resume request served by this tab, rendered by
AllJobsPage next
+ // to the Application line: Some(message) while a request is running or
after it was not
+ // acknowledged, None when idle or after a success.
+ @volatile private var holdRequestStatus: Option[String] = None
+
+ private[jobs] def lastHoldRequestStatus: Option[String] = holdRequestStatus
+
+ def handleHoldRequest(request: HttpServletRequest): Unit = {
+ if (holdEnabled &&
parent.securityManager.checkModifyPermissions(request.getRemoteUser)) {
+ sc.filter(_.executorHoldSupported).foreach { ctx =>
+ holdRequestExecutorPool.foreach { pool =>
+ holdRequestStatus = Some("hold requested")
Review Comment:
Fixed in a927b1e with the tagging you sketched: the status is now
`Option[(isHold, message)]` and `AllJobsPage` renders it only next to the
matching control, so a failed hold never labels the resume link. The wording is
softened to "did not take effect, see the driver logs", which is accurate for
all three false-returning paths. Reporting *why* a hold declined (e.g. "a
pipelined job is running") needs `holdExecutors()` to return more than a
Boolean, so I left that as a follow-up rather than widening the API in this PR.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
