Skip to content

[SPARK-20288] Avoid generating the MapStatus by stageId in BasicSchedulerIntegrationSuite#17603

Closed
jinxing64 wants to merge 1 commit intoapache:masterfrom
jinxing64:SPARK-20288
Closed

[SPARK-20288] Avoid generating the MapStatus by stageId in BasicSchedulerIntegrationSuite#17603
jinxing64 wants to merge 1 commit intoapache:masterfrom
jinxing64:SPARK-20288

Conversation

@jinxing64
Copy link
Copy Markdown

@jinxing64 jinxing64 commented Apr 11, 2017

What changes were proposed in this pull request?

ShuffleId is determined before job submitted. But it's hard to predict stageId by shuffleId.
Stage is created in DAGScheduler(
https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L381), but the order is n
ot determined in HashSet.
I added a log(println(s"Creating ShufflMapStage-$id on shuffle-${shuffleDep.shuffleId}")) after (https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L331), when testing BasicSchedulerIntegrationSuite:"multi-stage job". It will print:
Creating ShufflMapStage-0 on shuffle-0
Creating ShufflMapStage-1 on shuffle-2
Creating ShufflMapStage-2 on shuffle-1
Creating ShufflMapStage-3 on shuffle-3
or
Creating ShufflMapStage-0 on shuffle-1
Creating ShufflMapStage-1 on shuffle-3
Creating ShufflMapStage-2 on shuffle-0
Creating ShufflMapStage-3 on shuffle-2
It might be better to avoid generating the MapStatus by stageId.

@SparkQA
Copy link
Copy Markdown

SparkQA commented Apr 11, 2017

Test build #75693 has started for PR 17603 at commit ea883f5.

@jinxing64
Copy link
Copy Markdown
Author

I found this when test #17533. It failed now and then when try to get size of reduce from MapStatus.

I'm not sure how to make it better:
Modify the test as this pr
or
Change https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala#L380

@SparkQA
Copy link
Copy Markdown

SparkQA commented Apr 11, 2017

Test build #75696 has finished for PR 17603 at commit 5a93693.

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

@jinxing64
Copy link
Copy Markdown
Author

@squito
Could you help comment on this ? :)

@squito
Copy link
Copy Markdown
Contributor

squito commented May 30, 2017

lgtm

rerunning tests just to be safe since its been so long

@SparkQA
Copy link
Copy Markdown

SparkQA commented May 30, 2017

Test build #3768 has finished for PR 17603 at commit 5a93693.

  • This patch passes all tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@jinxing64
Copy link
Copy Markdown
Author

@squito
Thank you so much :-) :-)

@jinxing64
Copy link
Copy Markdown
Author

Jenkins, retest this please.

@SparkQA
Copy link
Copy Markdown

SparkQA commented May 31, 2017

Test build #77569 has finished for PR 17603 at commit 831de22.

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

@squito
Copy link
Copy Markdown
Contributor

squito commented May 31, 2017

merged to master

@asfgit asfgit closed this in ac7fc30 May 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants