advancedxy commented on a change in pull request #32819:
URL: https://github.com/apache/spark/pull/32819#discussion_r704346156
##########
File path: core/src/main/scala/org/apache/spark/ExecutorAllocationManager.scala
##########
@@ -862,6 +854,15 @@ private[spark] class ExecutorAllocationManager(
}
}
+ override def onExecutorAllocatorRangeUpdate(
+ event: SparkListenerExecutorAllocatorRangeUpdate): Unit = {
+ val newLower = event.lower.getOrElse(_minNumExecutors)
+ val newUpper = event.upper.getOrElse(_maxNumExecutors)
+ validateSettings(newLower, newUpper)
Review comment:
I believe this should be like
```
try {
validateSettings(newLower, newUpper)
_minNumExecutors = newLower
_maxNumExecutors = newUpper
} catch {
....
}
```
Otherwise exception is thrown in this listener.
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala
##########
@@ -80,6 +82,22 @@ case class SetCommand(kv: Option[(String, Option[String])])
}
(keyValueOutput, runFunc)
+ case Some((DYN_ALLOCATION_MIN_EXECUTORS.key, Some(value))) =>
Review comment:
This is an administration operation. I am not sure it's a good idea to
control this by just run set command for long running service such as thrift
server as users can easily run this command, and misuse this feature.
How about expose this via the Rest API?
--
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]
