[SPARK-59055][CORE] Report the application hold status to Spark Master

[dongjoon-hyun]

What changes were proposed in this pull request?

This PR lets a driver report its hold status to the standalone Master, so that the Master UI and
/json/ endpoint can show it. Display only -- the (hold) / (resume) controls stay on the
driver web UI.

• SparkContext calls a new no-op CoarseGrainedSchedulerBackend.reportExecutorHoldStatus hook
  after initialization and on every hold/resume transition. StandaloneSchedulerBackend overrides
  it and forwards to StandaloneAppClient, which sends the new ApplicationHoldUpdated message to
  the Master. The status is cached and re-sent on registration and failover.
• The Master mirrors it onto ApplicationInfo.holdSupported / held (@transient; a new Master
  learns it again from the driver's re-report). The draining count is not pushed: the Master
  derives it from the executors it already tracks.
• The Master UI annotates the state column (e.g. RUNNING (held, draining 2 executors)), and
  /json/ gains holdsupported, held, and draining fields per application.

spark.ui.holdEnabled is honored (an opted-out application is not reported as holdable), and
finished applications are never annotated. No new configuration and no new public API.

Why are the changes needed?

SPARK-58828 and SPARK-59010 made the hold status visible only per application on the driver. An
operator on the Master page cannot tell which applications are held or still draining. This also
lays the groundwork for offering the controls on the Master UI in a follow-up PR.

Does this PR introduce any user-facing change?

Yes, additive: the Master UI annotates held applications in the state column, and each application
in /json/ gains holdsupported, held, and draining fields.

How was this patch tested?

Pass the CIs.

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

Generated-by: Claude Fable 5

[dongjoon-hyun]

Could you review this PR too, @peter-toth ?

[peter-toth]

Thanks for the PR, @dongjoon-hyun!

The driver-side push reads right to me: a cached (supported, held) pair on the StandaloneAppClient endpoint, re-sent from RegisteredApplication/MasterChanged, with the draining count derived on the Master from the executors it already tracks instead of carried in the message. @transient on the two ApplicationInfo fields is the right call, and init() resetting them means a recovered app really does relearn them from the driver. All my findings are on the display half. MasterPage annotates on held alone and never reads holdSupported, so the spark.ui.holdEnabled gate that the docs and the StandaloneSchedulerBackend scaladoc both promise does not exist (1), and /json/ and the page disagree once an application finishes (2). The driver-to-Master leg of the path is also untested, which a ~200 ms test in StandaloneDynamicAllocationSuite closes (3).

Blocking

• 1. spark.ui.holdEnabled does not gate the annotation: appStateText keys off app.held only, so an application that opted out is still annotated RUNNING (held, draining N executors) — contradicting docs/spark-standalone.md:722-724 and the reportExecutorHoldStatus scaladoc. Observed on the rendered page. [inline: core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala:320]
• 2. /json/ keeps a stale hold after the application finishes: writeApplicationInfo has no isFinished guard and also runs over completedApps, so a finished application reports held: true and a non-zero draining — exactly what appStateText suppresses, while the doc says both surfaces report the same. [inline: core/src/main/scala/org/apache/spark/deploy/JsonProtocol.scala:119]
• 3. The driver-to-Master leg is untested: neither SparkContext.reportExecutorHoldStatus nor this override, with its UI_HOLD_ENABLED conjunct, is exercised; AppClientSuite starts one hop later. [inline: core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala:260]

Non-blocking

• 4. The report reads _executorsHeld outside the lock: a concurrent hold and resume can deliver the two reports inverted, leaving the Master showing (held) for a running application until the next transition. One line on the helper fixes both callers. [inline: core/src/main/scala/org/apache/spark/SparkContext.scala:2327]
• 5. The Master's per-application page is not annotated: ApplicationPage.scala:92 still renders the bare state, so clicking through from an annotated row loses the hold. [inline: core/src/main/scala/org/apache/spark/deploy/master/ui/MasterPage.scala:369]

Minor

• 6. reportHoldStatus drops the report silently: both siblings in the class log a warning on the same null-endpoint condition. [inline: core/src/main/scala/org/apache/spark/deploy/client/StandaloneAppClient.scala:355]
• 7. Doc section placement: the new section sits ~60 lines above the existing # Monitoring and Logging section it belongs under. [inline: docs/spark-standalone.md:714]

[peter-toth]

Finding 1. appStateText never reads holdSupported, so the spark.ui.holdEnabled gate that StandaloneSchedulerBackend.reportExecutorHoldStatus builds has no effect on the annotation. Two claims go with it that don't hold:

• docs/spark-standalone.md:722-724 — "Only applications whose driver reports that it can be held are annotated, which requires spark.ui.holdEnabled to be true on that application".
• the scaladoc on the override — "reports itself as not holdable, so that the Master does not show a hold status that the driver UI does not offer to change".

holdExecutors() is a @DeveloperApi that the config does not gate; only the driver-UI link at AllJobsPage.scala:358 and the POST handlers at JobsTab.scala:109,133 read it. So an application started with spark.ui.holdEnabled=false that calls sc.holdExecutors() reports supported=false, held=true, and the Master page annotates it regardless. Observed rather than argued — a probe in ReadOnlyMasterWebUISuite with holdSupported = false, held = true and two executors renders WAITING (held, draining 2 executors) on GET /, and /json/ returns "holdsupported" : false, "held" : true, "draining" : 2 (127 ms). holdSupported has no reader outside JsonProtocol today, so the field this PR added to make the decision is unused at the place the decision is made.

Together with finding 2 this collapses into one accessor on ApplicationInfo, which also removes the chance of the page and the endpoint drifting apart:

/** Whether the application is held right now, per the last report from its running driver. */
private[deploy] def isHeld: Boolean = held && holdSupported && !isFinished

private[deploy] def numDrainingExecutors: Int = if (isHeld) executors.size else 0

appStateText then opens with if (!app.isHeld) and JsonProtocol writes ("held" -> obj.isHeld). If you'd rather keep the annotation driven by held alone, the doc sentence and the scaladoc clause both need to drop the claim instead.

Worth pinning either way, and cheap: ReadOnlyMasterWebUISuite already builds ApplicationInfos by hand and asserts against the rendered HTML, so the test is roughly

app1.held = true
app1.holdSupported = true
app1.addExecutor(createWorkerInfo(), 1, 1024, Map.empty, DEFAULT_RESOURCE_PROFILE_ID)
app1.addExecutor(createWorkerInfo(), 1, 1024, Map.empty, DEFAULT_RESOURCE_PROFILE_ID)
// GET / ...
assert(result.contains("WAITING (held, draining 2 executors)"))

plus the same with holdSupported = false asserting the row is not annotated.

[dongjoon-hyun]

Good catch, and thank you for the probe. Fixed as you suggested: added ApplicationInfo.isHeld (held && holdSupported && !isFinished) and made both numDrainingExecutors and the JSON held field read it, so the page and the endpoint can no longer drift apart. Also added the rendered-page test to ReadOnlyMasterWebUISuite, including the holdSupported = false case asserting the row is not annotated.

[peter-toth]

Finding 2. held and draining are written unguarded, and writeMasterState runs writeApplicationInfo over completedApps too (JsonProtocol.scala:244), so a finished application keeps reporting its last hold. appStateText deliberately suppresses exactly this case — "its driver is gone, so the last reported hold is stale" — and docs/spark-standalone.md:719-720 says /json/ "reports the same in the holdsupported, held, and draining fields". So the two new surfaces disagree, and the doc describes neither.

draining is the worse half. Master.scala:563 keeps a finished application's executors on purpose (if (!appInfo.isFinished) appInfo.removeExecutor(exec)), so a held application that is then killed from the Master page reports its executor count as draining indefinitely. Measured with a MasterSuite probe — held = true, one executor, markFinished(FINISHED) — gives "held" : true and "draining" : 1.

An operator scripting /json/ to find held applications gets completed ones back. Same fix as finding 1: fold isFinished into a single accessor on ApplicationInfo and have both the page and the endpoint read it, so neither can be updated without the other.

[dongjoon-hyun]

Fixed via the same isHeld accessor: held now writes obj.isHeld and draining derives from it, so a finished application reports held: false, draining: 0 on both surfaces. Pinned in MasterSuite with a markFinished case (while the Master still keeps the executors) -- ApplicationState is private[master], so the finished case is asserted there rather than in JsonProtocolSuite.

[peter-toth]

Finding 3. Nothing exercises this override or SparkContext.reportExecutorHoldStatus. AppClientSuite calls client.reportHoldStatus directly, so it starts one hop after both new pieces of driver-side logic: the schedulerBackend match dispatch in SparkContext with its three call sites, and the supported && conf.get(config.UI.UI_HOLD_ENABLED) conjunct here. The conjunct is the one the docs make a promise about, so it is the one most worth pinning.

StandaloneDynamicAllocationSuite is a good home — it already holds a real Master reference and a getApplications() helper, and its workers fake executor launches, so no shuffle service has to run: executorHoldSupported only reads the two confs. I ran this there against the PR head, 187 ms:

test("SPARK-59055: report the hold status of the application to the Master") {
  sc = new SparkContext(appConf
    .set(config.SHUFFLE_SERVICE_ENABLED, true)
    .set(config.DECOMMISSION_ENABLED, true))
  // The report from the end of the SparkContext constructor.
  eventually(timeout(10.seconds), interval(10.millis)) {
    assert(getApplications().length === 1)
    assert(getApplications().head.holdSupported)
  }
  assert(!getApplications().head.held)

  assert(sc.holdExecutors())
  eventually(timeout(10.seconds), interval(10.millis)) {
    assert(getApplications().head.held)
  }
  // The faked executors never exit, so all of them are still counted as draining.
  assert(getApplications().head.numDrainingExecutors === 2)

  assert(sc.resumeExecutors())
  eventually(timeout(10.seconds), interval(10.millis)) {
    assert(!getApplications().head.held)
  }
}

A second one adding .set(config.UI.UI_HOLD_ENABLED, false) and asserting !getApplications().head.holdSupported covers the conjunct (40 ms) — that is the test that surfaced finding 1.

The failover re-send from MasterChanged stays uncovered after this; that one is genuinely harder and fine as a follow-up.

[dongjoon-hyun]

Thank you for the ready-made tests -- added both to StandaloneDynamicAllocationSuite. I extended the spark.ui.holdEnabled=false one to call sc.holdExecutors() first and wait for held = true to arrive at the Master before asserting !holdSupported, so it cannot pass vacuously before the report lands. Agreed on leaving the MasterChanged re-send to a follow-up.

[peter-toth]

Finding 4. The two callers invoke this after leaving the synchronized block that flipped _executorsHeld, so the read here and the send it feeds are not ordered against a concurrent transition. holdExecutors() and resumeExecutors() are both reachable from the driver UI POST handlers (JobsTab.scala:109,133), which run on separate Jetty threads, so: the hold sets the flag and reads true; the resume then clears it, reads false and sends first; the hold's true arrives last. The Master is then left showing (held) for a running application, and send is fire-and-forget, so nothing corrects it until the next transition.

Reading and sending under the same monitor is enough — whichever call serializes last reads and reports the current value. send only enqueues, so holding the monitor across it does not block:

  private def reportExecutorHoldStatus(): Unit = synchronized {
    schedulerBackend match {
      case cg: CoarseGrainedSchedulerBackend =>
        cg.reportExecutorHoldStatus(executorHoldSupported, _executorsHeld)
      case _ =>
    }
  }

That covers both call sites and the one at SparkContext.scala:720, which is uncontended anyway.

[dongjoon-hyun]

Fixed -- reportExecutorHoldStatus() is now synchronized, covering both transition call sites and the constructor one.

[peter-toth]

Finding 5. The Master's per-application page is not annotated — ApplicationPage.scala:92 still renders <li><strong>State:</strong> {app.state}</li>. So an operator who sees RUNNING (held, draining 2 executors) here and clicks the app id lands on the page that lists those executors and finds a bare RUNNING. That page is arguably the more natural place for the count, since draining is derived from the very executor table it shows.

Calling the same helper from ApplicationPage would need it somewhere both pages can reach (UIUtils, or a small method on ApplicationInfo next to numDrainingExecutors). Fine as a follow-up if you'd rather keep this PR to the listing page, but the two Master pages disagreeing about the same application is worth closing.

[dongjoon-hyun]

Done in this PR: moved the annotation into ApplicationInfo.stateText next to numDrainingExecutors, and both MasterPage and ApplicationPage render it now.

[peter-toth]

Finding 6. This returns silently when endpoint.get is null, while both siblings in the class log on the same condition — requestTotalExecutors at :344 and killExecutors at :368 ("Attempted to ... before driver fully initialized"). A dropped hold report leaves the Master's status wrong with nothing in the log to explain it, and unlike the siblings there is no return value for the caller to notice. An else logWarning(...) in the neighbours' shape would do.

[dongjoon-hyun]

Fixed -- added the warning in the neighbours' shape.

[peter-toth]

Finding 7. This lands between ## REST API and # Resource Scheduling, about 60 lines above the existing # Monitoring and Logging section (:773) — which is where the file already describes what the Master web UI shows, and where a reader looks for this. Moving it there, or making it a ## under it, also keeps the launch → resource-scheduling flow unbroken.

[dongjoon-hyun]

Moved under # Monitoring and Logging as a ## Held Applications subsection.

[dongjoon-hyun]

Thank you for the thorough review, @peter-toth! All 7 findings are addressed in a66d704 -- the blocking ones via a single ApplicationInfo.isHeld accessor that both the page and /json/ now read, plus your driver-to-Master tests in StandaloneDynamicAllocationSuite. Details in the inline replies.

[peter-toth]

Re-checked through a66d704 — findings 1-7 all resolved, nothing regressed. The ApplicationInfo.isHeld / stateText pair closes 1, 2 and 5 in one place; the two StandaloneDynamicAllocationSuite tests close 3; reportExecutorHoldStatus() is now synchronized for 4; 6 and 7 are done. I re-derived 4 rather than taking it as fixed, and it holds: every report reads _executorsHeld under the monitor that guards the flips, and the send sits inside that critical section, so enqueue order matches lock order and the last report carries the current state. All 6 new tests pass here.

My finding 1 offered two resolutions and this took the accessor one. Having measured it, the doc-side one was right: folding holdSupported into isHeld means spark.ui.holdEnabled=false now hides a genuinely held, genuinely draining application from the Master completely (8).

Blocking

• 8. spark.ui.holdEnabled=false hides a real hold (new): an application held through sc.holdExecutors() with the config off renders a bare RUNNING and reports held: false, draining: 0 while two of its executors are actually draining on the Master — measured both ways. Dropping the UI_HOLD_ENABLED conjunct is a one-line fix; keeping the gate needs the config's own doc to say it blanks the Master's view. [inline: core/src/main/scala/org/apache/spark/scheduler/cluster/StandaloneSchedulerBackend.scala:262]

Minor

• 9. The comment credits the allocation manager (late catch): executorHoldSupported never reads it — only supportsExecutorHold, the two confs and shuffleDriverComponents. [inline: core/src/main/scala/org/apache/spark/SparkContext.scala:719]
• 10. stateText's zero-draining branch is unpinned (late catch): the new MasterSuite test reaches "held, no executors left" but asserts only numDrainingExecutors === 0, which a not-held application returns too. [inline: core/src/test/scala/org/apache/spark/deploy/master/MasterSuite.scala:272]

[peter-toth]

Finding 8. This conjunct now decides whether the Master shows anything at all, because isHeld ANDs holdSupported in. But spark.ui.holdEnabled gates a driver-UI button — holdExecutors() is a @DeveloperApi the config does not touch, and only AllJobsPage.scala:358 and JobsTab.scala:109,133 read it. So an application that turns the button off and drives holds programmatically gets nothing on the Master, which is the gap this PR opens with ("An operator on the Master page cannot tell which applications are held or still draining").

Measured in StandaloneDynamicAllocationSuite — holdEnabled=false, sc.holdExecutors(), waiting for held to reach the Master, 180 ms:

executors=2 state=RUNNING held=true holdSupported=false isHeld=false draining=0 stateText='RUNNING'
{..."state":"RUNNING","holdsupported":false,"held":false,"draining":0,...}

Two executors are sitting on the Master, still draining, and /json/ says held: false, draining: 0 — while JsonProtocol.scala:99 documents held as "whether the application is currently held; always false once it finishes". The control run (config left at its default) on the same setup gives stateText='RUNNING (held, draining 2 executors)' and "held":true,"draining":2.

This is my finding 1 landing on the wrong side of the two fixes it offered. Annotating always costs one doc sentence; gating costs real data. One line:

Suggested change

	_.reportHoldStatus(supported && conf.get(config.UI.UI_HOLD_ENABLED), held))
	_.reportHoldStatus(supported, held))

holdSupported then means "this deployment can hold this application", which is what /json/'s holdsupported description already claims, and isHeld's holdSupported conjunct survives as a cheap guard — holdExecutors() requires the preconditions, so held without supported is unreachable once the config is out of it. I applied exactly this and re-ran: holdEnabled=false then gives holdSupported=true isHeld=true draining=2 stateText='RUNNING (held, draining 2 executors)', 45 ms. (It fits on one line after the edit; config stays used elsewhere in the file.)

Two edits go with it: the scaladoc above loses its spark.ui.holdEnabled paragraph, and docs/spark-standalone.md:777-778 loses "which requires spark.ui.holdEnabled to be true on that application". SPARK-59055: spark.ui.holdEnabled=false is reported as not holdable inverts — it fails with the change, and is worth keeping with its assertions flipped, as the test that pins the config not suppressing the status.

If you'd rather keep the gate, it needs documenting on the config itself (UI.scala:96-102 and configuration.md:1609) — nothing today tells an operator that turning the driver button off also blanks the Master's view — and JsonProtocol.scala:99 needs the qualifier on held.

[dongjoon-hyun]

You are right -- the split changed what the config should gate: in a display-only PR it was hiding data, not a button. Took your one-line fix: the conjunct is gone, the raw supported is reported, and the scaladoc paragraph and the doc sentence went with it. The holdEnabled=false test is kept with its assertions flipped, as SPARK-59055: spark.ui.holdEnabled=false does not suppress the hold status. The control-side gate stays in the follow-up PR, which rejects hold requests at the driver when the config is off.

[peter-toth]

Finding 9. executorHoldSupported (:2095-2102) reads cg.supportsExecutorHold, SHUFFLE_SERVICE_ENABLED / shuffleDriverComponents.supportsReliableStorage() and DECOMMISSION_ENABLED. The allocation manager is not in it, and _executorsHeld is still false at this point, so nothing here depends on _executorAllocationManager.foreach(_.start()) at :712 — only on _shuffleDriverComponents at :659.

Suggested change

	// Advertise whether this application can be held, now that the shuffle driver components and
	// the allocation manager, which decide it, are up.
	// Advertise whether this application can be held, now that the shuffle driver components,
	// which decide it, are up.

[dongjoon-hyun]

Fixed -- took the suggestion.

[peter-toth]

Finding 10. Here the application is held with no executors left — stateText renders WAITING (held), the "hold complete" signal the docs point operators at — but the assertion is numDrainingExecutors === 0, which is also what a not-held application returns (:260, :264) and what a finished one returns (:279). So nothing in the suite tells "hold complete" apart from "not held", and stateText's draining == 0 branch (ApplicationInfo.scala:230-231) has no coverage at all:

Suggested change

	assert(appInfo.numDrainingExecutors === 0)
	assert(appInfo.numDrainingExecutors === 0)
	assert(appInfo.isHeld)
	assert(appInfo.stateText === "WAITING (held)")

The singular executor form (:233) is unpinned too — ReadOnlyMasterWebUISuite covers only the plural. One addExecutor after the block above gives WAITING (held, draining 1 executor). I ran both additions against this head, 39 ms.

[dongjoon-hyun]

Fixed -- added the isHeld / stateText === "WAITING (held)" assertions and the singular draining 1 executor case (which also let the finished block drop its own addExecutor).

[dongjoon-hyun]

Thank you for the re-review and for measuring both resolutions of finding 1, @peter-toth. Findings 8-10 are addressed in ea68292 -- the UI_HOLD_ENABLED conjunct is dropped so the Master always shows the true hold state, and the comment and test-coverage nits are fixed as suggested.

[dongjoon-hyun]

Could you review this PR when you have some time, @viirya ?