Allow minExecutors=0 in dynamic mode #89
Conversation
sudiptob2
force-pushed
the
feat/157/min-zero-executor
branch
from
753bb02 to
8910d9b
Compare
| val modeHelper = DeploymentModeHelper(conf) | ||
| val gangCardinality = modeHelper.getGangCardinality | ||
| configGenerator.getAnnotations ++ templateAnnotations ++ nodeUniformityLabel | ||
| .filter(_ => gangCardinality > 0) // Only add gang annotations if cardinality > 0 |
There was a problem hiding this comment.
- Armada does not accept gangCardinality = 0; gives
gang cardinality 0 is non-positiveerror
There was a problem hiding this comment.
Cardinality is driver + num(executors), so even if you submit driver only, cardinality will be 1 in that case.
There was a problem hiding this comment.
Is it true for client mode as well? I guess for client mode, ExecutorCount is the gangCardinality
There was a problem hiding this comment.
in client mode, the cardinality is just the num of executors:
https://github.com/armadaproject/armada-spark/blob/master/src/main/scala/org/apache/spark/deploy/armada/DeploymentModeHelper.scala#L215-L228
GeorgeJahad
left a comment
GeorgeJahad
left a comment
There was a problem hiding this comment.
lgtm, thanks @sudiptob2 !
| if (executorCount <= 0) { | ||
| val modeHelper = DeploymentModeHelper(conf) | ||
| val executorCount = modeHelper.getExecutorCount | ||
| val isDynamic = conf.getBoolean("spark.dynamicAllocation.enabled", false) |
There was a problem hiding this comment.
Is there a constant for this already defined somewhere in Spark? I'd ideally avoid magic strings
There was a problem hiding this comment.
Hmm..I tried to look into this but it seems there is no predefined constant to avoid using magic strings for configuration keys. Also, this pattern of accessing configuration is already used in other places in the codebase.
@GeorgeJahad, any leads to improve this?
There was a problem hiding this comment.
Looks like there is one here:
sudiptob2
force-pushed
the
feat/157/min-zero-executor
branch
from
8910d9b to
e8245c9
Compare
GeorgeJahad
left a comment
GeorgeJahad
left a comment
There was a problem hiding this comment.
lgtm, thanks @sudiptob2 !
Signed-off-by: Sudipto Baral <[email protected]> # Conflicts: # src/main/scala/org/apache/spark/deploy/armada/submit/ArmadaClientApplication.scala
Signed-off-by: Sudipto Baral <[email protected]>
Signed-off-by: Sudipto Baral <[email protected]>
Signed-off-by: Sudipto Baral <[email protected]>
Signed-off-by: Sudipto Baral <[email protected]>
sudiptob2
force-pushed
the
feat/157/min-zero-executor
branch
from
e8245c9 to
3b9fc3e
Compare
3 participants
Closes G-Research/spark#157