diff options
author | Janak Ramakrishnan <janakr@google.com> | 2016-02-22 22:44:45 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2016-02-23 13:08:32 +0000 |
commit | 3f8fe0ded2a05b8e971c68c18cf820faba01fa1f (patch) | |
tree | 93dc8658634206046c8ed3a02ed59dabd7512515 /src/main/java/com | |
parent | 915933bc241837e98ba869cb54798e33830c4232 (diff) |
Prevent new evaluations from starting if a done child's error is discovered. Also delete some code that's been dead for a while, now that we eagerly shut down evaluation when we come across a child in error during a fail-fast evaluation.
--
MOS_MIGRATED_REVID=115272603
Diffstat (limited to 'src/main/java/com')
-rw-r--r-- | src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java | 54 |
1 files changed, 29 insertions, 25 deletions
diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java index ff885d8dbe..ce92df94e4 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -1037,28 +1037,26 @@ public final class ParallelEvaluator implements Evaluator { SkyKey childErrorKey = env.getDepErrorKey(); NodeEntry childErrorEntry = Preconditions.checkNotNull(graph.get(childErrorKey), "skyKey: %s, state: %s childErrorKey: %s", skyKey, state, childErrorKey); - if (!state.getTemporaryDirectDeps().contains(childErrorKey)) { - // This means the cached error was freshly requested (e.g. the parent has never been - // built before). - Preconditions.checkState(newDirectDeps.contains(childErrorKey), "%s %s %s", state, - childErrorKey, newDirectDeps); - state.addTemporaryDirectDeps(GroupedListHelper.create(ImmutableList.of(childErrorKey))); - DependencyState childErrorState = childErrorEntry.addReverseDepAndCheckIfDone(skyKey); - Preconditions.checkState(childErrorState == DependencyState.DONE, - "skyKey: %s, state: %s childErrorKey: %s", skyKey, state, childErrorKey, - childErrorEntry); - } else { - // This means the cached error was previously requested, and was then subsequently (after - // a restart) requested along with another sibling dep. This can happen on an incremental - // eval call when the parent is dirty and the child error is in a separate dependency - // group from the sibling dep. - Preconditions.checkState(!newDirectDeps.contains(childErrorKey), "%s %s %s", state, - childErrorKey, newDirectDeps); - Preconditions.checkState(childErrorEntry.isDone(), - "skyKey: %s, state: %s childErrorKey: %s", skyKey, state, childErrorKey, - childErrorEntry); - } + Preconditions.checkState( + !state.getTemporaryDirectDeps().contains(childErrorKey), + "Done error was already know: %s %s %s %s", + skyKey, + state, + childErrorKey, + childErrorEntry); + Preconditions.checkState( + newDirectDeps.contains(childErrorKey), "%s %s %s", state, childErrorKey, newDirectDeps); + state.addTemporaryDirectDeps(GroupedListHelper.create(ImmutableList.of(childErrorKey))); + DependencyState childErrorState = childErrorEntry.addReverseDepAndCheckIfDone(skyKey); + Preconditions.checkState( + childErrorState == DependencyState.DONE, + "skyKey: %s, state: %s childErrorKey: %s", + skyKey, + state, + childErrorKey, + childErrorEntry); ErrorInfo childErrorInfo = Preconditions.checkNotNull(childErrorEntry.getErrorInfo()); + visitor.preventNewEvaluations(); throw SchedulerException.ofError(childErrorInfo, childErrorKey); } @@ -1078,10 +1076,16 @@ public final class ParallelEvaluator implements Evaluator { // TODO(bazel-team): This means a bug in the SkyFunction. What to do? Preconditions.checkState(!env.childErrorInfos.isEmpty(), "Evaluation of SkyKey failed and no dependencies were requested: %s %s", skyKey, state); - env.commit(/*enqueueParents=*/keepGoing); - if (!keepGoing) { - throw SchedulerException.ofError(state.getErrorInfo(), skyKey); - } + Preconditions.checkState( + keepGoing, + "nokeep_going evaluation should have failed on first child error: %s %s %s", + skyKey, + state, + env.childErrorInfos); + // If the child error was catastrophic, committing this parent to the graph is not + // necessary, but since we don't do error bubbling in catastrophes, it doesn't violate any + // invariants either. + env.commit(/*enqueueParents=*/ true); return; } |