Skip to content

[HOTFIX] [CORE] fix flaky BasicSchedulerIntegrationTest #13688

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.

Sign up for GitHub

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

Closed
squito wants to merge 1 commit into from
Closed

[HOTFIX] [CORE] fix flaky BasicSchedulerIntegrationTest #13688

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions core/src/test/scala/org/apache/spark/scheduler/SchedulerIntegrationSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,10 +518,11 @@ class BasicSchedulerIntegrationSuite extends SchedulerIntegrationSuite[SingleCor

// make sure the required map output is available
task.stageId match {
case 1 => assertMapOutputAvailable(b)
case 3 => assertMapOutputAvailable(c)
case 4 => assertMapOutputAvailable(d)
case _ => // no shuffle map input, nothing to check
case _ =>
// we can't check for the output for the two intermediate stages, unfortunately,
// b/c the stage numbering is non-deterministic, so stage number alone doesn't tell
// us what to check
Copy link
Contributor

@kayousterhout kayousterhout Jun 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to do an OR here -- check that one of the intermediate outputs is available? (fine if you want to do this later and merge the fix for now)

Copy link
Contributor Author

@squito squito Jun 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this, but decided it was kind of silly since you could be checking for something irrelevant, and its not a trivial change (you'd need to catch an exception from doing the wrong check). Anyway just figured I'd leave it out for the hotfix.

}

(task.stageId, task.stageAttemptId, task.partitionId) match {
Expand Down Expand Up @@ -557,11 +558,9 @@ class BasicSchedulerIntegrationSuite extends SchedulerIntegrationSuite[SingleCor
val (taskDescription, task) = backend.beginTask()
stageToAttempts.getOrElseUpdate(task.stageId, new HashSet()) += task.stageAttemptId

// make sure the required map output is available
task.stageId match {
case 1 => assertMapOutputAvailable(shuffledRdd)
case _ => // no shuffle map input, nothing to check
}
// We cannot check if shuffle output is available, because the failed fetch will clear the
// shuffle output. Then we'd have a race, between the already-started task from the first
// attempt, and when the failure clears out the map output status.

(task.stageId, task.stageAttemptId, task.partitionId) match {
case (0, _, _) =>
Expand Down