[SPARK-32920][SHUFFLE] Finalization of Shuffle push/merge with Push based shuffle and preparation step for the reduce stage #30691

venkata91 · 2020-12-09T20:27:48Z

What changes were proposed in this pull request?

Summary of the changes made as part of this PR:

DAGScheduler changes to finalize a ShuffleMapStage which involves talking to all the shuffle mergers (ExternalShuffleService) and getting all the completed merge statuses.
Once the ShuffleMapStage finalization is complete, mark the ShuffleMapStage to be finalized which marks the stage as complete and subsequently letting the child stage start.
Also added the relevant tests to DAGSchedulerSuite for changes made as part of SPARK-32919

Lead-authored-by: Min Shen [email protected]
Co-authored-by: Venkata krishnan Sowrirajan [email protected]
Co-authored-by: Chandni Singh [email protected]

Why are the changes needed?

Refer to SPARK-30602

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added unit tests to DAGSchedulerSuite

venkata91 · 2020-12-09T20:49:18Z

cc @jiangxb1987 @Victsm @mridulm @Ngone51 @tgravescs Please review

core/src/main/scala/org/apache/spark/MapOutputTracker.scala

core/src/main/scala/org/apache/spark/internal/config/package.scala

core/src/main/scala/org/apache/spark/Dependency.scala

github-actions · 2021-03-22T00:49:32Z

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!

mridulm · 2021-04-28T05:35:37Z

+CC @venkata91 please resolve conflicts

1. Handling of MergeResults from the executors in MapOutputTracker 2. Shuffle merge finalization in DagScheduler This also includes the following changes: - LIHADOOP-52972 Tests for changes in MapOutputTracker and DagScheduler related to pushbased shuffle. Author: Chandni Singh <[email protected]> - LIHADOOP-52202 Utility to create a directory with 770 permission. Author: Chandni Singh <[email protected]> - LIHADOOP-52972 Moved isPushBasedShuffleEnabled to Utils and added a unit test for it. Author: Ye Zhou <[email protected]> LIHADOOP-53321 Magnet: Merge client shuffle block fetcher related changes LIHADOOP-54115 Unregister map and merge outputs on the host when DAG scheduler encounters a shuffle chunk failure RB=2151376 BUG=LIHADOOP-54115 G=spark-reviewers R=yezhou,mshen A=mshen LIHADOOP-52494 Magnet fallback to origin shuffle blocks when fetch of a shuffle chunk fails Prepare for PR SPARK-32920: Push based shuffle finalization of shuffle push/merge as part of ShuffleMapStage and preparing for the reduce stage Address otterc review comments

venkata91 · 2021-04-28T17:20:29Z

Fixed the PR rebasing the latest master changes and also marked it as open for review. cc @mridulm @Victsm @otterc @Ngone51

venkata91 · 2021-05-04T17:33:29Z

Gentle ping @Ngone51 @tgravescs @mridulm @Victsm

core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala

core/src/test/scala/org/apache/spark/scheduler/DAGSchedulerSuite.scala

mridulm

Thanks for working on this @venkata91 !
Took a pass through it

core/src/main/scala/org/apache/spark/Dependency.scala

core/src/main/scala/org/apache/spark/internal/config/package.scala

core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala

mridulm · 2021-05-05T15:43:23Z

core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala

+    }
+  }
+
+  private def processShuffleMapStageCompletion(shuffleStage: ShuffleMapStage): Unit = {


review note: no changes here. Method extracted from handleTaskCompletion

fetch failure cases

.idea/vcs.xml

core/src/main/scala/org/apache/spark/Dependency.scala

core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala

Victsm · 2021-06-09T00:14:35Z

Changes lgtm.

mridulm · 2021-06-10T06:53:01Z

jenkins, test this please

SparkQA · 2021-06-10T07:41:22Z

Test build #139624 has finished for PR 30691 at commit e570818.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-06-10T08:31:53Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44151/

SparkQA · 2021-06-10T09:06:48Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44151/

mridulm · 2021-06-10T14:50:54Z

jenkins, test this please

SparkQA · 2021-06-10T17:30:17Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44178/

SparkQA · 2021-06-10T17:52:09Z

Test build #139650 has finished for PR 30691 at commit e570818.

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

mridulm · 2021-06-10T18:00:32Z

The test failures are unrelated to this pr.
Merging to master.

SparkQA · 2021-06-10T18:03:26Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44178/

mridulm · 2021-06-10T18:07:52Z

Thanks for working on this @venkata91
Thanks for all the reviews and comments @Ngone51, @otterc, @Victsm !

…ased shuffle and preparation step for the reduce stage ### What changes were proposed in this pull request? Summary of the changes made as part of this PR: 1. `DAGScheduler` changes to finalize a ShuffleMapStage which involves talking to all the shuffle mergers (`ExternalShuffleService`) and getting all the completed merge statuses. 2. Once the `ShuffleMapStage` finalization is complete, mark the `ShuffleMapStage` to be finalized which marks the stage as complete and subsequently letting the child stage start. 3. Also added the relevant tests to `DAGSchedulerSuite` for changes made as part of [SPARK-32919](https://issues.apache.org/jira/browse/SPARK-32919) Lead-authored-by: Min Shen mshenlinkedin.com Co-authored-by: Venkata krishnan Sowrirajan vsowrirajanlinkedin.com Co-authored-by: Chandni Singh chsinghlinkedin.com ### Why are the changes needed? Refer to [SPARK-30602](https://issues.apache.org/jira/browse/SPARK-30602) ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? Added unit tests to DAGSchedulerSuite Closes apache#30691 from venkata91/SPARK-32920. Lead-authored-by: Venkata krishnan Sowrirajan <[email protected]> Co-authored-by: Min Shen <[email protected]> Co-authored-by: Chandni Singh <[email protected]> Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>

…ased shuffle and preparation step for the reduce stage Summary of the changes made as part of this PR: 1. `DAGScheduler` changes to finalize a ShuffleMapStage which involves talking to all the shuffle mergers (`ExternalShuffleService`) and getting all the completed merge statuses. 2. Once the `ShuffleMapStage` finalization is complete, mark the `ShuffleMapStage` to be finalized which marks the stage as complete and subsequently letting the child stage start. 3. Also added the relevant tests to `DAGSchedulerSuite` for changes made as part of [SPARK-32919](https://issues.apache.org/jira/browse/SPARK-32919) Lead-authored-by: Min Shen mshenlinkedin.com Co-authored-by: Venkata krishnan Sowrirajan vsowrirajanlinkedin.com Co-authored-by: Chandni Singh chsinghlinkedin.com Refer to [SPARK-30602](https://issues.apache.org/jira/browse/SPARK-30602) No Added unit tests to DAGSchedulerSuite Closes #30691 from venkata91/SPARK-32920. Lead-authored-by: Venkata krishnan Sowrirajan <[email protected]> Co-authored-by: Min Shen <[email protected]> Co-authored-by: Chandni Singh <[email protected]> Signed-off-by: Mridul Muralidharan <mridul<at>gmail.com>

github-actions bot added the CORE label Dec 9, 2020

otterc reviewed Dec 9, 2020

View reviewed changes

Ngone51 reviewed Dec 11, 2020

View reviewed changes

core/src/main/scala/org/apache/spark/Dependency.scala Outdated Show resolved Hide resolved

HyukjinKwon marked this pull request as draft December 11, 2020 03:39

github-actions bot added the Stale label Mar 22, 2021

github-actions bot closed this Mar 23, 2021

mridulm removed the Stale label Apr 28, 2021

mridulm reopened this Apr 28, 2021

venkata91 changed the title ~~[WIP][SPARK-32920][SHUFFLE] Finalization of Shuffle push/merge with Push based shuffle and preparation step for the reduce stage~~ [SPARK-32920][SHUFFLE] Finalization of Shuffle push/merge with Push based shuffle and preparation step for the reduce stage Apr 28, 2021

venkata91 marked this pull request as ready for review April 28, 2021 17:14

venkata91 force-pushed the SPARK-32920 branch from 94c440c to bd8edc5 Compare April 28, 2021 17:17

Address ngone51 review comments

d8fe417

otterc reviewed May 5, 2021

View reviewed changes

mridulm reviewed May 5, 2021

View reviewed changes

Address otterc and mridulm review comments and additional changes for

755148c

fetch failure cases

venkata91 force-pushed the SPARK-32920 branch 2 times, most recently from aa889f1 to 7046206 Compare May 7, 2021 17:47

mridulm reviewed May 11, 2021

View reviewed changes

.idea/vcs.xml Outdated Show resolved Hide resolved

mridulm reviewed May 11, 2021

View reviewed changes

core/src/main/scala/org/apache/spark/Dependency.scala Outdated Show resolved Hide resolved

mridulm reviewed May 11, 2021

View reviewed changes

core/src/main/scala/org/apache/spark/Dependency.scala Outdated Show resolved Hide resolved

mridulm reviewed May 11, 2021

View reviewed changes

core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala Outdated Show resolved Hide resolved

mridulm reviewed May 11, 2021

View reviewed changes

core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala Outdated Show resolved Hide resolved

mridulm reviewed May 11, 2021

View reviewed changes

core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala Outdated Show resolved Hide resolved