jpcorreia99 commented on code in PR #45240:
URL: https://github.com/apache/spark/pull/45240#discussion_r1505736480
##########
core/src/main/scala/org/apache/spark/resource/ResourceProfile.scala:
##########
@@ -489,10 +489,13 @@ object ResourceProfile extends Logging {
private[spark] def calculateOverHeadMemory(
overHeadMemFromConf: Option[Long],
+ minimumOverHeadMemoryFromConf: Option[Long],
executorMemoryMiB: Long,
overheadFactor: Double): Long = {
+ val minMemoryOverhead =
+
minimumOverHeadMemoryFromConf.getOrElse(ResourceProfile.MEMORY_OVERHEAD_MIN_MIB);
overHeadMemFromConf.getOrElse(math.max((overheadFactor *
executorMemoryMiB).toInt,
- ResourceProfile.MEMORY_OVERHEAD_MIN_MIB))
+ minMemoryOverhead))
Review Comment:
I order to minimize the changes to existing logic, I chose to make this flag
not have a default and instead write the logic to check if it is not set and
use the existing constant.
However, from a code cleaniness and maintenance standpoint, I think the
constant can be deleted and the flag value defaulted to 384Mib.
@the reviewer, what's your view here?
##########
core/src/main/scala/org/apache/spark/resource/ResourceProfile.scala:
##########
@@ -489,10 +489,13 @@ object ResourceProfile extends Logging {
private[spark] def calculateOverHeadMemory(
overHeadMemFromConf: Option[Long],
+ minimumOverHeadMemoryFromConf: Option[Long],
executorMemoryMiB: Long,
overheadFactor: Double): Long = {
+ val minMemoryOverhead =
+
minimumOverHeadMemoryFromConf.getOrElse(ResourceProfile.MEMORY_OVERHEAD_MIN_MIB);
overHeadMemFromConf.getOrElse(math.max((overheadFactor *
executorMemoryMiB).toInt,
- ResourceProfile.MEMORY_OVERHEAD_MIN_MIB))
+ minMemoryOverhead))
Review Comment:
I order to minimize the changes to existing logic, I chose to make this flag
not have a default and instead write the logic to check if it is not set and
use the existing constant.
However, from a code cleaniness and maintenance standpoint, I think the
constant can be deleted and the flag value defaulted to 384Mib.
@reviewer, what's your view here?
--
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]
