agrawaldevesh commented on a change in pull request #29367:
URL: https://github.com/apache/spark/pull/29367#discussion_r466699724
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationClient.scala
##########
@@ -81,6 +82,43 @@ private[spark] trait ExecutorAllocationClient {
countFailures: Boolean,
force: Boolean = false): Seq[String]
+ /**
+ * Request that the cluster manager decommission the specified executors.
+ * Default implementation delegates to kill, scheduler must override
+ * if it supports graceful decommissioning.
+ *
+ * @param executors identifiers of executors & decom info.
+ * @param adjustTargetNumExecutors whether the target number of executors
will be adjusted down
+ * after these executors have been
decommissioned.
+ * @return the ids of the executors acknowledged by the cluster manager to
be removed.
+ */
+ def decommissionExecutors(
+ executors: Seq[(String, ExecutorDecommissionInfo)],
+ adjustTargetNumExecutors: Boolean): Seq[String] = {
+ killExecutors(executors.map(_._1),
+ adjustTargetNumExecutors,
+ countFailures = false)
+ }
+
+
+ /**
+ * Request that the cluster manager decommission the specified executor.
+ * Default implementation delegates to decommissionExecutors, scheduler can
override
+ * if it supports graceful decommissioning.
+ *
+ * @param executorId identifiers of executor to decommission
+ * @param decommissionInfo information about the decommission (reason, host
loss)
+ * @return whether the request is acknowledged by the cluster manager.
+ */
+ def decommissionExecutor(executorId: String,
Review comment:
Simply because I want it to be clear about which method to override for
implementing decommissioning: should the subclass of ExecutorAllocationClient
override decommissionExecutor, or decommissionExecutors or both ? Marking it
final will make clear: decommissionExecutors is the one they should override to
decom a batch of executors, while decommissionExecutor is just a helper method
to decom a single executor using the decommissionExecutors workhorse.
The reason I am advocating for adding adjustTargetNumExecutors is so that it
is explicit. adjustTargetNumExecutors is a dangerous operation and I want the
caller to be explicit in asking for that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
