[3.0][SPARK-32518][CORE] CoarseGrainedSchedulerBackend.maxNumConcurrentTasks should consider all kinds of resources by Ngone51 · Pull Request #29395 · apache/spark

Ngone51 · 2020-08-10T06:42:29Z

What changes were proposed in this pull request?

Make CoarseGrainedSchedulerBackend.maxNumConcurrentTasks() considers all kinds of resources when calculating the max concurrent tasks
Refactor calculateAvailableSlots() to make it be able to be used for both CoarseGrainedSchedulerBackend and TaskSchedulerImpl

Why are the changes needed?

Currently, CoarseGrainedSchedulerBackend.maxNumConcurrentTasks() only considers the CPU for the max concurrent tasks. This can cause the application to hang when a barrier stage requires extra custom resources but the cluster doesn't have enough corresponding resources. Because, without the checking for other custom resources in maxNumConcurrentTasks, the barrier stage can be submitted to the TaskSchedulerImpl. But the TaskSchedulerImpl won't launch tasks for the barrier stage due to the insufficient task slots calculated by TaskSchedulerImpl.calculateAvailableSlots (which does check all kinds of resources).

If the barrier stage doesn't launch all the tasks in one true, the application will fail and suggest user to disable delay scheduling. However, this actually a misleading suggestion since the real root cause is not enough resources.

Does this PR introduce any user-facing change?

Yes. In case of a barrier stage requires more custom resources than the cluster has, previously, the application will fail with misleading suggestion of disabling delay scheduling. After this PR, the application will fail with the error message saying not enough resources.

How was this patch tested?

Added a unit test.

Ngone51 · 2020-08-10T06:42:44Z

ping @cloud-fan @tgravescs

Ngone51 · 2020-08-10T06:45:06Z

+ ConfigBuilder("spark.testing.skipValidateCores")
+ .version("3.1.0")
+ .booleanConf
+ .createWithDefault(false)


These 2 configs are backported from Master branch.

ditto. This should be 3.0.1 when it comes to branch-3.0, @Ngone51 .
Also, after merging this, please update master branch consistently.

Thank you @dongjoon-hyun for letting me know. I was wondering about it previously.

SparkQA · 2020-08-10T07:05:02Z

Test build #127268 has finished for PR 29395 at commit c980996.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

tgravescs · 2020-08-10T14:17:37Z

+ .collect()
+ }
+ assert(exception.getMessage.contains("[SPARK-24819]: Barrier execution " +
+ "mode does not allow run a barrier stage that requires more slots"))


I'm not sure if its worth it but it would be nice to perhaps print what the limiting resource is. If its to much change or work to track we may just skip it.

This actually a good idea. But as you mentioned, I'm afraid this needs much more changes. So, I'd like to skip it here.

ok, we can revisit if it becomes an issue later.

SparkQA · 2020-08-11T16:11:42Z

Test build #127324 has finished for PR 29395 at commit f635c18.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

Ngone51 · 2020-08-12T12:06:38Z

@tgravescs @clockfly Is it looks OK now?

tgravescs

lgtm

dongjoon-hyun · 2020-08-14T03:53:52Z

 .createWithDefault(2)
+
+ val RESOURCES_WARNING_TESTING = ConfigBuilder("spark.resources.warnings.testing")
+ .version("3.1.0")


This should be 3.0.1 when it comes to branch-3.0, @Ngone51 .

dongjoon-hyun

Please config version properly.

SparkQA · 2020-08-14T07:05:02Z

Test build #127436 has finished for PR 29395 at commit 9c18479.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

Ngone51 · 2020-08-14T07:05:52Z

retest this please.

SparkQA · 2020-08-14T09:00:49Z

Test build #127442 has finished for PR 29395 at commit 9c18479.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-08-14T09:12:51Z

retest this please

SparkQA · 2020-08-14T11:35:50Z

Test build #127450 has finished for PR 29395 at commit 9c18479.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

tgravescs · 2020-08-14T13:13:32Z

test this please

SparkQA · 2020-08-14T15:44:35Z

Test build #127458 has finished for PR 29395 at commit 9c18479.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Ngone51 · 2020-08-15T12:33:33Z

retest this please

SparkQA · 2020-08-15T14:36:33Z

Test build #127473 has finished for PR 29395 at commit 9c18479.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Ngone51 · 2020-08-17T03:25:13Z

retest this please.

SparkQA · 2020-08-17T05:28:22Z

Test build #127496 has finished for PR 29395 at commit 9c18479.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

Ngone51 · 2020-08-17T05:31:31Z

org.apache.spark.sql.DataFrameSuite.SPARK-28224: Aggregate sum big decimal overflow is quite flaky. But I'm sure it can pass on my laptop.

Ngone51 · 2020-08-17T05:31:43Z

retest this please.

Ngone51 · 2020-08-17T06:32:20Z

Seems like we need to wait for this fix: #29448

SparkQA · 2020-08-17T07:05:02Z

Test build #127498 has finished for PR 29395 at commit 9c18479.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2020-08-18T04:28:31Z

Test build #127519 has finished for PR 29395 at commit daa205d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

cloud-fan · 2020-08-18T06:49:54Z

thanks, merging to 3.0!

…ntTasks should consider all kinds of resources ### What changes were proposed in this pull request? 1. Make `CoarseGrainedSchedulerBackend.maxNumConcurrentTasks()` considers all kinds of resources when calculating the max concurrent tasks 2. Refactor `calculateAvailableSlots()` to make it be able to be used for both `CoarseGrainedSchedulerBackend` and `TaskSchedulerImpl` ### Why are the changes needed? Currently, `CoarseGrainedSchedulerBackend.maxNumConcurrentTasks()` only considers the CPU for the max concurrent tasks. This can cause the application to hang when a barrier stage requires extra custom resources but the cluster doesn't have enough corresponding resources. Because, without the checking for other custom resources in `maxNumConcurrentTasks`, the barrier stage can be submitted to the `TaskSchedulerImpl`. But the `TaskSchedulerImpl` won't launch tasks for the barrier stage due to the insufficient task slots calculated by `TaskSchedulerImpl.calculateAvailableSlots` (which does check all kinds of resources). If the barrier stage doesn't launch all the tasks in one true, the application will fail and suggest user to disable delay scheduling. However, this actually a misleading suggestion since the real root cause is not enough resources. ### Does this PR introduce _any_ user-facing change? Yes. In case of a barrier stage requires more custom resources than the cluster has, previously, the application will fail with misleading suggestion of disabling delay scheduling. After this PR, the application will fail with the error message saying not enough resources. ### How was this patch tested? Added a unit test. Closes#29395 from Ngone51/backport-spark-32518. Authored-by: yi.wu <yi.wu@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>

probot-autolabelerBot added the CORE label Aug 10, 2020

Ngone51 mentioned this pull request Aug 10, 2020
[SPARK-32518][CORE] CoarseGrainedSchedulerBackend.maxNumConcurrentTasks should consider all kinds of resources #29332
Closed

Ngone51 commented Aug 10, 2020
View reviewed changes

tgravescs reviewed Aug 10, 2020
View reviewed changes

tgravescs approved these changes Aug 12, 2020
View reviewed changes

dongjoon-hyun reviewed Aug 14, 2020
View reviewed changes

dongjoon-hyun requested changes Aug 14, 2020
View reviewed changes

Ngone51 added 4 commits August 18, 2020 09:46

fix
c79d3bb

update
aa265a0

address comments
7883a3e

update config version
daa205d

Ngone51 force-pushed the backport-spark-32518 branch from 9c18479 to daa205dCompare August 18, 2020 01:49

cloud-fan closed this Aug 18, 2020

Uh oh!

Conversation

Ngone51 commented Aug 10, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Ngone51 commented Aug 10, 2020

Uh oh!

Ngone51Aug 10, 2020

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyunAug 14, 2020

Choose a reason for hiding this comment

Uh oh!

Ngone51Aug 14, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 10, 2020

Uh oh!

Uh oh!

Uh oh!

tgravescsAug 10, 2020

Choose a reason for hiding this comment

Uh oh!

Ngone51Aug 11, 2020

Choose a reason for hiding this comment

Uh oh!

tgravescsAug 11, 2020

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 11, 2020

Uh oh!

Ngone51 commented Aug 12, 2020

Uh oh!

tgravescs left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyunAug 14, 2020

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 14, 2020

Uh oh!

Ngone51 commented Aug 14, 2020

Uh oh!

SparkQA commented Aug 14, 2020

Uh oh!

cloud-fan commented Aug 14, 2020

Uh oh!

SparkQA commented Aug 14, 2020

Uh oh!

tgravescs commented Aug 14, 2020

Uh oh!

SparkQA commented Aug 14, 2020

Uh oh!

Ngone51 commented Aug 15, 2020

Uh oh!

SparkQA commented Aug 15, 2020

Uh oh!

Ngone51 commented Aug 17, 2020

Uh oh!

SparkQA commented Aug 17, 2020

Uh oh!

Ngone51 commented Aug 17, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ngone51 commented Aug 17, 2020

Uh oh!

Ngone51 commented Aug 17, 2020

Uh oh!

SparkQA commented Aug 17, 2020

Uh oh!

SparkQA commented Aug 18, 2020

Uh oh!

cloud-fan commented Aug 18, 2020• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ngone51 commented Aug 17, 2020•
edited
Loading

cloud-fan commented Aug 18, 2020•
edited
Loading