Skip to content
This repository was archived by the owner on Jan 9, 2020. It is now read-only.

Fix MB units conversion(spark->k8s)&Set the executor ENV_EXECUTOR_MEMORY to executorMemoryWithOverhead #471

Closed
wants to merge 1 commit into from
Closed

Fix MB units conversion(spark->k8s)&Set the executor ENV_EXECUTOR_MEMORY to executorMemoryWithOverhead #471

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...a/org/apache/spark/deploy/kubernetes/submit/submitsteps/BaseDriverConfigurationStep.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ private[spark] class BaseDriverConfigurationStep(
.withAmount(driverCpuCores)
.build()
val driverMemoryQuantity = new QuantityBuilder(false)
.withAmount(s"${driverMemoryMb}M")
.withAmount(s"${driverMemoryMb}Mi")
.build()
val driverMemoryLimitQuantity = new QuantityBuilder(false)
.withAmount(s"${driverContainerMemoryWithOverhead}M")
.withAmount(s"${driverContainerMemoryWithOverhead}Mi")
.build()
val maybeCpuLimitQuantity = driverLimitCores.map { limitCores =>
("cpu", new QuantityBuilder(false).withAmount(limitCores).build())
Expand Down
9 changes: 3 additions & 6 deletions ...ala/org/apache/spark/scheduler/cluster/kubernetes/KubernetesClusterSchedulerBackend.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ private[spark] class KubernetesClusterSchedulerBackend(
private val executorPodNamePrefix = conf.get(KUBERNETES_EXECUTOR_POD_NAME_PREFIX)

private val executorMemoryMb = conf.get(org.apache.spark.internal.config.EXECUTOR_MEMORY)
private val executorMemoryString = conf.get(
org.apache.spark.internal.config.EXECUTOR_MEMORY.key,
org.apache.spark.internal.config.EXECUTOR_MEMORY.defaultValueString)

private val memoryOverheadMb = conf
.get(KUBERNETES_EXECUTOR_MEMORY_OVERHEAD)
Expand Down Expand Up @@ -441,10 +438,10 @@ private[spark] class KubernetesClusterSchedulerBackend(
SPARK_ROLE_LABEL -> SPARK_POD_EXECUTOR_ROLE) ++
executorLabels
val executorMemoryQuantity = new QuantityBuilder(false)
.withAmount(s"${executorMemoryMb}M")
.withAmount(s"${executorMemoryMb}Mi")
.build()
val executorMemoryLimitQuantity = new QuantityBuilder(false)
.withAmount(s"${executorMemoryWithOverhead}M")
.withAmount(s"${executorMemoryWithOverhead}Mi")
.build()
val executorCpuQuantity = new QuantityBuilder(false)
.withAmount(executorCores.toString)
Expand All @@ -469,7 +466,7 @@ private[spark] class KubernetesClusterSchedulerBackend(
(ENV_DRIVER_URL, driverUrl),
// Executor backend expects integral value for executor cores, so round it up to an int.
(ENV_EXECUTOR_CORES, math.ceil(executorCores).toInt.toString),
(ENV_EXECUTOR_MEMORY, executorMemoryString),
(ENV_EXECUTOR_MEMORY, executorMemoryWithOverhead + "m"),
Copy link

@mccheah mccheah Aug 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to do this. The point of the memory with overhead is to make it higher than what we give the JVM. The JVM should only have the exact amount of heap given by spark.executor.memory, but the overhead accounts for the fact that the JVM might also allocate memory off-heap, and in Python's case it also covers for the storage of Python's data.

Hence I think #470 does cover everything we need here.

Copy link
Author

@duyanghao duyanghao Aug 31, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ash211 @mccheah as i said in #467, Right now, the ENV_EXECUTOR_MEMORY is memory but ENV_DRIVER_MEMORY is memory+overhead

If the ENV_EXECUTOR_MEMORY should only be memory, as you said above, the ENV_DRIVER_MEMORY should also only be memory rather than memory+overhead,is that right?

So i suggest that ENV_DRIVER_MEMORY should also be just memory instead of memory+overheads.

I have created a PR for to correct this problem.

(ENV_APPLICATION_ID, applicationId()),
(ENV_EXECUTOR_ID, executorId),
(ENV_MOUNTED_CLASSPATH, s"$executorJarsDownloadDir/*")) ++ sc.executorEnvs.toSeq)
Expand Down
4 changes: 2 additions & 2 deletions .../apache/spark/deploy/kubernetes/submit/submitsteps/BaseDriverConfigurationStepSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ private[spark] class BaseDriverConfigurationStepSuite extends SparkFunSuite {
val resourceRequirements = preparedDriverSpec.driverContainer.getResources
val requests = resourceRequirements.getRequests.asScala
assert(requests("cpu").getAmount === "2")
assert(requests("memory").getAmount === "256M")
assert(requests("memory").getAmount === "256Mi")
val limits = resourceRequirements.getLimits.asScala
assert(limits("memory").getAmount === "456M")
assert(limits("memory").getAmount === "456Mi")
assert(limits("cpu").getAmount === "4")
val driverPodMetadata = preparedDriverSpec.driverPod.getMetadata
assert(driverPodMetadata.getName === "spark-driver-pod")
Expand Down