You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/06/28 10:41:07 UTC
[GitHub] [spark] alexandrutofan opened a new pull request, #37016: Driver cores mult be a positive number fix
alexandrutofan opened a new pull request, #37016:
URL: https://github.com/apache/spark/pull/37016
### What changes were proposed in this pull request?
Hello, according to this [problem](https://issues.apache.org/jira/browse/SPARK-39617 ) raised also by me, and highlighted by two other persons on Stack Overflow ([this](https://stackoverflow.com/questions/71691292/driver-cores-must-be-a-positive-number) and [this](https://stackoverflow.com/questions/72473151/exception-in-thread-main-org-apache-spark-sparkexception-driver-cores-must-be)) and thanks to this [feature](https://issues.apache.org/jira/browse/SPARK-35013) we are currently unable to run Spark 3.2.x with Mesos in Cluster Mode.
From what I saw, it happends because of the **_MesosClusterDispatcher_** and its components which treat the driverCores variable as a Double and in the end it's passed to SparkSubmit as a double and this makes the check by **_SparkSubmitArguments_** to fail.
I'm proposing this quick fix because I think it's a quick & safe way to go with.
### Why are the changes needed?
To be able to run Spark 3.2.x in Cluster Mode with Mesos
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
It's a very small change, basically the Cast from String to Int is safe now.
Thank you!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mihaivinaga commented on pull request #37016: Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
mihaivinaga commented on PR #37016:
URL: https://github.com/apache/spark/pull/37016#issuecomment-1169622311
@srowen the issue is created and posted in the initial post: https://issues.apache.org/jira/browse/SPARK-39617
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #37016: Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #37016:
URL: https://github.com/apache/spark/pull/37016#issuecomment-1168984035
Please file a JIRA and update the PR per https://spark.apache.org/contributing.html
Can we not just fix the Mesos component itself? this is hacky but not terrible.
Mesos support is probably going away in Spark 4, note.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] alexandrutofan commented on pull request #37016: [SPARK-39617][CORE] Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
alexandrutofan commented on PR #37016:
URL: https://github.com/apache/spark/pull/37016#issuecomment-1171083717
@LuciferYang I had a look on the Mesos side but unfortunately I don't have the time to do that
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] closed pull request #37016: [SPARK-39617][CORE] Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #37016: [SPARK-39617][CORE] Driver cores mult be a positive number fix
URL: https://github.com/apache/spark/pull/37016
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] github-actions[bot] commented on pull request #37016: [SPARK-39617][CORE] Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #37016:
URL: https://github.com/apache/spark/pull/37016#issuecomment-1279859091
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #37016: [SPARK-39617][CORE] Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #37016:
URL: https://github.com/apache/spark/pull/37016#issuecomment-1170877861
@alexandrutofan can u re-trigger GitHub Actions, a UT failed
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] alexandrutofan commented on a diff in pull request #37016: [SPARK-39617][CORE] Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
alexandrutofan commented on code in PR #37016:
URL: https://github.com/apache/spark/pull/37016#discussion_r915684500
##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala:
##########
@@ -253,7 +253,7 @@ private[deploy] class SparkSubmitArguments(args: Seq[String], env: Map[String, S
&& Try(JavaUtils.byteStringAsBytes(executorMemory)).getOrElse(-1L) <= 0) {
error("Executor memory must be a positive number")
}
- if (driverCores != null && Try(driverCores.toInt).getOrElse(-1) <= 0) {
+ if (driverCores != null && Try(driverCores.toDouble.toInt).getOrElse(-1) <= 0) {
error("Driver cores must be a positive number")
Review Comment:
It's better to do that in general, but IMHO, in this case, the message is very easy to understand, and also I'd rather keep it consistent with the other checks..
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] alexandrutofan closed pull request #37016: Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
alexandrutofan closed pull request #37016: Driver cores mult be a positive number fix
URL: https://github.com/apache/spark/pull/37016
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #37016: Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #37016:
URL: https://github.com/apache/spark/pull/37016#issuecomment-1169011797
Can one of the admins verify this patch?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #37016: Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #37016:
URL: https://github.com/apache/spark/pull/37016#issuecomment-1169938900
This needs to be connected by putting [SPARK-39617] in the title along with component -- see the contributing guide
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on pull request #37016: Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #37016:
URL: https://github.com/apache/spark/pull/37016#issuecomment-1169939595
Also, you opened this vs 3.2, but needs to be vs master.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #37016: [SPARK-39617][CORE] Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
LuciferYang commented on PR #37016:
URL: https://github.com/apache/spark/pull/37016#issuecomment-1170880326
It is possible to add a new UT to mesos module?
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] martin-g commented on a diff in pull request #37016: Driver cores mult be a positive number fix
Posted by GitBox <gi...@apache.org>.
martin-g commented on code in PR #37016:
URL: https://github.com/apache/spark/pull/37016#discussion_r908444889
##########
core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala:
##########
@@ -253,7 +253,7 @@ private[deploy] class SparkSubmitArguments(args: Seq[String], env: Map[String, S
&& Try(JavaUtils.byteStringAsBytes(executorMemory)).getOrElse(-1L) <= 0) {
error("Executor memory must be a positive number")
}
- if (driverCores != null && Try(driverCores.toInt).getOrElse(-1) <= 0) {
+ if (driverCores != null && Try(driverCores.toDouble.toInt).getOrElse(-1) <= 0) {
error("Driver cores must be a positive number")
Review Comment:
IMO this should be improved properly by pattern matching on the `Try` and printing the actual `Failure`.
Not just for `driverCores` but for all input arguments.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org