-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-23948] Trigger mapstage's job listener in submitMissingTasks #21019
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2146,6 +2146,57 @@ class DAGSchedulerSuite extends SparkFunSuite with LocalSparkContext with TimeLi | |
| assertDataStructuresEmpty() | ||
| } | ||
|
|
||
| test("Trigger mapstage's job listener in submitMissingTasks") { | ||
| val rdd1 = new MyRDD(sc, 2, Nil) | ||
| val dep1 = new ShuffleDependency(rdd1, new HashPartitioner(2)) | ||
| val rdd2 = new MyRDD(sc, 2, List(dep1), tracker = mapOutputTracker) | ||
| val dep2 = new ShuffleDependency(rdd2, new HashPartitioner(2)) | ||
|
|
||
| val listener1 = new SimpleListener | ||
| val listener2 = new SimpleListener | ||
|
|
||
| submitMapStage(dep1, listener1) | ||
| submitMapStage(dep2, listener2) | ||
|
|
||
| // Complete the stage0. | ||
| assert(taskSets(0).stageId === 0) | ||
| complete(taskSets(0), Seq( | ||
| (Success, makeMapStatus("hostA", rdd1.partitions.length)), | ||
| (Success, makeMapStatus("hostB", rdd1.partitions.length)))) | ||
| assert(mapOutputTracker.getMapSizesByExecutorId(dep1.shuffleId, 0).map(_._1).toSet === | ||
| HashSet(makeBlockManagerId("hostA"), makeBlockManagerId("hostB"))) | ||
| assert(listener1.results.size === 1) | ||
|
|
||
| // When attempting stage1, trigger a fetch failure. | ||
| assert(taskSets(1).stageId === 1) | ||
| complete(taskSets(1), Seq( | ||
| (Success, makeMapStatus("hostC", rdd2.partitions.length)), | ||
| (FetchFailed(makeBlockManagerId("hostA"), dep1.shuffleId, 0, 0, "ignored"), null))) | ||
| scheduler.resubmitFailedStages() | ||
| // Stage1 listener should not have a result yet | ||
| assert(listener2.results.size === 0) | ||
|
|
||
| // Speculative task succeeded in stage1. | ||
| runEvent(makeCompletionEvent( | ||
| taskSets(1).tasks(1), | ||
| Success, | ||
| makeMapStatus("hostD", rdd2.partitions.length))) | ||
| // stage1 listener still should not have a result, though there's no missing partitions | ||
| // in it. Because stage1 is not inside runningStages at this moment. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||
| assert(listener2.results.size === 0) | ||
|
|
||
| // Stage0 should now be running as task set 2; make its task succeed | ||
| assert(taskSets(2).stageId === 0) | ||
| complete(taskSets(2), Seq( | ||
| (Success, makeMapStatus("hostC", rdd2.partitions.length)))) | ||
| assert(mapOutputTracker.getMapSizesByExecutorId(dep1.shuffleId, 0).map(_._1).toSet === | ||
| HashSet(makeBlockManagerId("hostC"), makeBlockManagerId("hostB"))) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can just use |
||
|
|
||
| // After stage0 is finished, stage1 will be submitted and found there is no missing | ||
| // partitions in it. Then listener got triggered. | ||
| assert(listener2.results.size === 1) | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you also add |
||
|
|
||
| /** | ||
| * In this test, we run a map stage where one of the executors fails but we still receive a | ||
| * "zombie" complete message from that executor. We want to make sure the stage is not reported | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to double check that
shuffleStage.isAvailablehere?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't seem this is necessary, as its already handled at the callsites ... but IMO its seems safer to include it, in case this gets invoked elsewhere in the future.