-
Notifications
You must be signed in to change notification settings - Fork 28.5k
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
[SPARK-3224] FetchFailed reduce stages should only show up once in failed stages (in UI) #2127
Changes from 2 commits
1dd3eb5
3d3d356
796d282
3f01847
49282b3
effb1ce
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 |
---|---|---|
|
@@ -1045,31 +1045,38 @@ class DAGScheduler( | |
stage.pendingTasks += task | ||
|
||
case FetchFailed(bmAddress, shuffleId, mapId, reduceId) => | ||
// Mark the stage that the reducer was in as unrunnable | ||
val failedStage = stageIdToStage(task.stageId) | ||
markStageAsFinished(failedStage, Some("Fetch failure")) | ||
runningStages -= failedStage | ||
// TODO: Cancel running tasks in the stage | ||
logInfo("Marking " + failedStage + " (" + failedStage.name + | ||
") for resubmision due to a fetch failure") | ||
// Mark the map whose fetch failed as broken in the map stage | ||
val mapStage = shuffleToMapStage(shuffleId) | ||
// It is likely that we receive multiple FetchFailed for a single stage (because we have | ||
// multiple tasks running concurrently on different executors). In that case, it is possible | ||
// the fetch failure has already been handled by the executor. | ||
if (runningStages.contains(failedStage)) { | ||
markStageAsFinished(failedStage, Some("Fetch failure")) | ||
runningStages -= failedStage | ||
// TODO: Cancel running tasks in the stage | ||
logInfo("Marking " + failedStage + " (" + failedStage.name + | ||
") for resubmision due to a fetch failure") | ||
|
||
logInfo("The failed fetch was from " + mapStage + " (" + mapStage.name + | ||
"); marking it for resubmission") | ||
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 combine this with the above now-very-redundant log msg? |
||
if (failedStages.isEmpty && eventProcessActor != null) { | ||
// Don't schedule an event to resubmit failed stages if failed isn't empty, because | ||
// in that case the event will already have been scheduled. eventProcessActor may be | ||
// null during unit tests. | ||
import env.actorSystem.dispatcher | ||
env.actorSystem.scheduler.scheduleOnce( | ||
RESUBMIT_TIMEOUT, eventProcessActor, ResubmitFailedStages) | ||
} | ||
failedStages += failedStage | ||
failedStages += mapStage | ||
} | ||
|
||
// Mark the map whose fetch failed as broken in the map stage | ||
if (mapId != -1) { | ||
mapStage.removeOutputLoc(mapId, bmAddress) | ||
mapOutputTracker.unregisterMapOutput(shuffleId, mapId, bmAddress) | ||
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. I'm still not sure this is right. If you receive a new failure event for a new map output, don't you still need to schedule a ResubmitFailedStages event? Since otherwise the new map stage might not include all of the failed map tasks. 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. Is that a problem? I think the reduce stage retry will fail, leading to resubmission anyway? |
||
} | ||
logInfo("The failed fetch was from " + mapStage + " (" + mapStage.name + | ||
"); marking it for resubmission") | ||
if (failedStages.isEmpty && eventProcessActor != null) { | ||
// Don't schedule an event to resubmit failed stages if failed isn't empty, because | ||
// in that case the event will already have been scheduled. eventProcessActor may be | ||
// null during unit tests. | ||
import env.actorSystem.dispatcher | ||
env.actorSystem.scheduler.scheduleOnce( | ||
RESUBMIT_TIMEOUT, eventProcessActor, ResubmitFailedStages) | ||
} | ||
failedStages += failedStage | ||
failedStages += mapStage | ||
|
||
// TODO: mark the executor as failed only if there were lots of fetch failures on it | ||
if (bmAddress != null) { | ||
handleExecutorLost(bmAddress.executorId, Some(task.epoch)) | ||
|
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.
see the comment here explaining the problem