tgravescs commented on a change in pull request #28972:
URL: https://github.com/apache/spark/pull/28972#discussion_r450850223
##########
File path:
core/src/main/scala/org/apache/spark/resource/ExecutorResourceRequests.scala
##########
@@ -54,6 +54,19 @@ class ExecutorResourceRequests() extends Serializable {
this
}
+ /**
+ * Specify off heap memory. The value specified will be converted to MiB.
Review comment:
Ah good point, the only way to access SparkConf is for user to pass it
in, which we don't want to do here.
So I think we should treat this like the config does now. First document
this function that says you can set this but it requires MEMORY_OFFHEAP_ENABLED
for it to be used, then allow user to set it even if MEMORY_OFFHEAP_ENABLED is
false. Then in Yarn allocator before we set offHeapMem = execReq.amount we
check MEMORY_OFFHEAP_ENABLED similar to what executorOffHeapMemorySizeAsMb does
now.
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
