[SPARK-27192][Core] spark.task.cpus should be less or equal than spark.executor.cores #24261
Conversation
…k.task.cpus when use static executor allocation.
|
@srowen @dongjoon-hyun |
|
Test build #4672 has started for PR 24261 at commit |
|
Thank you, @liutang123 . BTW, the previous one fails due to the time-out in R unit test. I'll retrigger the test. |
|
Retest this please. |
|
Test build #104148 has finished for PR 24261 at commit
|
srowen
left a comment
srowen
left a comment
There was a problem hiding this comment.
@dongjoon-hyun as a double check this time does it look reasonable?
There was a problem hiding this comment.
I don't know if that comment is necessary, and I suppose you could write s"local[$taskCpus]" here. But seems OK with me overall.
|
Oh, sure. @srowen . I'll take a look today, too. Thanks. |
| throw new SparkException( | ||
| s"${EXECUTOR_CORES.key} must not be less than ${CPUS_PER_TASK.key}.") | ||
| } | ||
| } |
There was a problem hiding this comment.
What's the difference between this and checkCpusPerTask ?
There was a problem hiding this comment.
You can see the difference at #24131 's discuss.
| def checkCpusPerTask( | ||
| clusterMode: Boolean, | ||
| maxCoresPerExecutor: Option[Int]): Unit = { | ||
| val cpusPerTask = sc.conf.get(CPUS_PER_TASK) |
There was a problem hiding this comment.
@Ngone51 CPUS_PER_TASK has default value 1, so, contains check is not needed.
|
Retest this please. |
| if (clusterMode && sc.conf.contains(EXECUTOR_CORES)) { | ||
| if (sc.conf.get(EXECUTOR_CORES) < cpusPerTask) { | ||
| throw new SparkException(s"${CPUS_PER_TASK.key}" + | ||
| s" must be <= ${EXECUTOR_CORES.key} when run on $master.") |
There was a problem hiding this comment.
why is $master relevant here? I see we only check clusterMode
| } else if (maxCoresPerExecutor.isDefined) { | ||
| if (maxCoresPerExecutor.get < cpusPerTask) { | ||
| throw new SparkException(s"Only ${maxCoresPerExecutor.get} cores available per executor" + | ||
| s" when run on $master, and ${CPUS_PER_TASK.key} must be <= it.") |
There was a problem hiding this comment.
Max cores per executor is 3 when run on local[3] mode, and is 1 when run on local mode. I think we should info user that witch master he or she used.
|
Test build #104254 has finished for PR 24261 at commit
|
srowen
left a comment
srowen
left a comment
There was a problem hiding this comment.
@dongjoon-hyun @felixcheung I'll merge this tomorrow if you're OK
|
Merged to master |
6 participants
What changes were proposed in this pull request?
check spark.task.cpus before creating TaskScheduler in SparkContext
How was this patch tested?
UT
Please review http://spark.apache.org/contributing.html before opening a pull request.