LucaCanali commented on a change in pull request #29477:
URL: https://github.com/apache/spark/pull/29477#discussion_r475425134



##########
File path: 
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##########
@@ -323,4 +324,25 @@ private[spark] object KubernetesUtils extends Logging {
         .build()
     }
   }
+
+  /**
+   * Convert MEMORY_OFFHEAP_SIZE to MB Unit, return 0 if 
MEMORY_OFFHEAP_ENABLED is false.
+   */
+  def executorOffHeapMemorySizeAsMb(kubernetesConf: KubernetesExecutorConf): 
Int = {
+    val sizeInMB = 
Utils.memoryStringToMb(kubernetesConf.get(MEMORY_OFFHEAP_SIZE).toString)
+    checkOffHeapEnabled(kubernetesConf, sizeInMB).toInt
+  }
+
+  /**
+   * return 0 if MEMORY_OFFHEAP_ENABLED is false.
+   */
+  def checkOffHeapEnabled(kubernetesConf: KubernetesExecutorConf, offHeapSize: 
Long): Long = {
+    if (kubernetesConf.get(MEMORY_OFFHEAP_ENABLED)) {
+      require(offHeapSize > 0,
+        s"${MEMORY_OFFHEAP_SIZE.key} must be > 0 when 
${MEMORY_OFFHEAP_ENABLED.key} == true")

Review comment:
       Good point, I'll refactor this. A couple additional notes:
   - the code originally comes from YarnSparkHadoopUtil, so we may want to 
refactor there too and possibly merge into some common point, so that YARN and 
K8S can use it. Any thoughts?
   - The memory manager does already an additional check and requires 
"spark.memory.offHeap.size must be > 0 when spark.memory.offHeap.enabled == 
true"




----------------------------------------------------------------
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]

Reply via email to