mridulm commented on a change in pull request #29090:
URL: https://github.com/apache/spark/pull/29090#discussion_r516543107
##########
File path:
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala
##########
@@ -249,7 +249,7 @@ private[deploy] class SparkSubmitArguments(args:
Seq[String], env: Map[String, S
error("Driver memory must be a positive number")
}
if (executorMemory != null
- && Try(JavaUtils.byteStringAsBytes(executorMemory)).getOrElse(-1L) <=
0) {
+ && Try(JavaUtils.byteStringAsMb(executorMemory)).getOrElse(-1L) <= 0) {
Review comment:
In theory, the only difference would be if user set the memory to < 1 mb.
This is ridiculous enough to ignore as a valid usecase :-)
##########
File path:
launcher/src/main/java/org/apache/spark/launcher/CommandBuilderUtils.java
##########
@@ -328,4 +328,17 @@ static String findJarsDir(String sparkHome, String
scalaVersion, boolean failIfN
return libdir.getAbsolutePath();
}
+ /**
+ * Add "m" as the default suffix unit when no explicit unit is given.
+ */
+ static String addDefaultMSuffixIfNeeded(String memoryString) {
+ if (memoryString.chars().allMatch(Character::isDigit)) {
+ System.err.println("Memory setting without explicit unit (" +
+ memoryString + ") is taken to be in MB by default! For details check
SPARK-32293.");
Review comment:
Given that we are documenting 'm' is the suffix we use if not specified,
do we need this message to stderr ?
----------------------------------------------------------------
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
