cloud-fan commented on code in PR #58054:
URL: https://github.com/apache/spark/pull/58054#discussion_r3810012398
##########
core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala:
##########
@@ -175,6 +177,28 @@ private[spark] class ExecutorAllocationManager(
// (2) an executor idle timeout has elapsed.
@volatile private var initializing: Boolean = true
+ // Whether allocation is suspended because the executors are held. While
this is true,
+ // `schedule()` is a no-op so that pending tasks do not bring up new
executors.
+ // See `SparkContext.holdExecutors()`.
+ private var suspended: Boolean = false
+
+ // Whether the current executor targets still have to be pushed to the
cluster manager. Set
+ // when a push from `suspend()`/`resume()` fails or is rejected (e.g. before
the YARN AM has
+ // registered), and by `reset()`, which may run inside a cluster manager RPC
handler where a
+ // synchronous request would self-deadlock (e.g. YARN's
RegisterClusterManager). The push is
+ // performed from the allocation thread in `schedule()` and retried until
acknowledged, with
+ // an exponential backoff: the conditions it retries (an AM restart, an
unreachable cluster
Review Comment:
**Nit:**
```suggestion
// an exponential backoff: the conditions under which it retries (an AM
restart, an unreachable
```
##########
core/src/main/scala/org/apache/spark/scheduler/cluster/CoarseGrainedSchedulerBackend.scala:
##########
@@ -739,6 +776,41 @@ class CoarseGrainedSchedulerBackend(scheduler:
TaskSchedulerImpl, val rpcEnv: Rp
executorDataMap.keySet.toSeq
}
+ /** See `SparkContext.holdExecutors()`. */
+ private[spark] def setExecutorsHeld(held: Boolean): Unit = {
+ executorsHeld = held
Review Comment:
**Blocking:**
Please enforce the hold at the executor-requirement mutation path, not only
at registration. While this flag is true, `requestExecutors` and
`requestTotalExecutors` still publish nonzero targets, so the cluster allocates
executors just to have this guard drain them; then `resumeExecutors()` restores
the pre-hold snapshot and loses those requests. Keep the latest desired target
in the backend, publish zero until resume, and cover both public request APIs
with hold/request/resume interleaving tests.
--
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]