tgravescs commented on a change in pull request #35901:
URL: https://github.com/apache/spark/pull/35901#discussion_r829372761
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStep.scala
##########
@@ -53,28 +53,31 @@ private[spark] class BasicDriverFeatureStep(conf:
KubernetesDriverConf)
// Memory settings
private val driverMemoryMiB = conf.get(DRIVER_MEMORY)
- private val memoryOverheadFactor = if
(conf.contains(DRIVER_MEMORY_OVERHEAD_FACTOR)) {
- conf.get(DRIVER_MEMORY_OVERHEAD_FACTOR)
- } else {
- conf.get(MEMORY_OVERHEAD_FACTOR)
- }
- // The memory overhead factor to use. If the user has not set it, then use a
different
- // value for non-JVM apps. This value is propagated to executors.
- private val overheadFactor =
+ // The default memory overhead factor to use. If the user has not set it,
then use a different
+ // value for non-JVM apps. This value is propagated to executors and used if
the executor
+ // overhead factor is not set explicitly.
+ private val defaultOverheadFactor =
Review comment:
yes this is exactly what I was thinking originally, but then I realized
that if the default values for driver vs executor were to ever change in the
new configs this wouldn't handle that. So that is why it might actually be
better to check in the BasicExecutorFeatureStep. I'm ok with doing that as a
followup though as this fixes the issue and both have the same default.
one thing we may want to add comment above saying this is either the setting
of old config or the default.
--
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]
