diff options
3 files changed, 73 insertions, 27 deletions
diff --git a/src/main/java/com/google/devtools/build/skyframe/EvaluationResult.java b/src/main/java/com/google/devtools/build/skyframe/EvaluationResult.java index c398f7a9a7..64e1ff1475 100644 --- a/src/main/java/com/google/devtools/build/skyframe/EvaluationResult.java +++ b/src/main/java/com/google/devtools/build/skyframe/EvaluationResult.java @@ -40,6 +40,7 @@ import javax.annotation.Nullable; public class EvaluationResult<T extends SkyValue> { private final boolean hasError; + @Nullable private final Exception catastrophe; private final Map<SkyKey, T> resultMap; private final Map<SkyKey, ErrorInfo> errorMap; @@ -48,13 +49,18 @@ public class EvaluationResult<T extends SkyValue> { /** * Constructor for the "completed" case. Used only by {@link Builder}. */ - private EvaluationResult(Map<SkyKey, T> result, Map<SkyKey, ErrorInfo> errorMap, - boolean hasError, @Nullable WalkableGraph walkableGraph) { + private EvaluationResult( + Map<SkyKey, T> result, + Map<SkyKey, ErrorInfo> errorMap, + boolean hasError, + @Nullable Exception catastrophe, + @Nullable WalkableGraph walkableGraph) { Preconditions.checkState(errorMap.isEmpty() || hasError, "result=%s, errorMap=%s", result, errorMap); this.resultMap = Preconditions.checkNotNull(result); this.errorMap = Preconditions.checkNotNull(errorMap); this.hasError = hasError; + this.catastrophe = catastrophe; this.walkableGraph = walkableGraph; } @@ -75,6 +81,12 @@ public class EvaluationResult<T extends SkyValue> { return hasError; } + /** @return catastrophic error encountered during evaluation, if any */ + @Nullable + public Exception getCatastrophe() { + return catastrophe; + } + /** * @return All successfully evaluated {@link SkyValue}s. */ @@ -152,6 +164,7 @@ public class EvaluationResult<T extends SkyValue> { private final Map<SkyKey, T> result = new HashMap<>(); private final Map<SkyKey, ErrorInfo> errors = new HashMap<>(); private boolean hasError = false; + @Nullable private Exception catastrophe = null; private WalkableGraph walkableGraph = null; /** Adds a value to the result. An error for this key must not already be present. */ @@ -184,11 +197,15 @@ public class EvaluationResult<T extends SkyValue> { } public EvaluationResult<T> build() { - return new EvaluationResult<>(result, errors, hasError, walkableGraph); + return new EvaluationResult<>(result, errors, hasError, catastrophe, walkableGraph); } public void setHasError(boolean hasError) { this.hasError = hasError; } + + public void setCatastrophe(Exception catastrophe) { + this.catastrophe = catastrophe; + } } } 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 e2a6608210..54b3cfd18d 100644 --- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java +++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluator.java @@ -1312,10 +1312,14 @@ public final class ParallelEvaluator implements Evaluator { // ErrorInfo could only be null if SchedulerException wrapped an InterruptedException, but // that should have been propagated. ErrorInfo errorInfo = Preconditions.checkNotNull(e.getErrorInfo(), errorKey); - catastrophe = errorInfo.isCatastrophic(); - if (!catastrophe || !keepGoing) { + if (!keepGoing) { bubbleErrorInfo = bubbleErrorUp(errorInfo, errorKey, skyKeys, visitor); } else { + Preconditions.checkState( + errorInfo.isCatastrophic(), + "Scheduler exception only thrown for catastrophe in keep_going evaluation: %s", + e); + catastrophe = true; // Bubbling the error up requires that graph edges are present for done nodes. This is not // always the case in a keepGoing evaluation, since it is assumed that done nodes do not // need to be traversed. In this case, we hope the caller is tolerant of a possibly empty @@ -1507,10 +1511,17 @@ public final class ParallelEvaluator implements Evaluator { * {@code skyKeys} are known to be in the DONE state ({@code entry.isDone()} returns true). */ private <T extends SkyValue> EvaluationResult<T> constructResult( - @Nullable ValueVisitor visitor, Iterable<SkyKey> skyKeys, - Map<SkyKey, ValueWithMetadata> bubbleErrorInfo, boolean catastrophe) { - Preconditions.checkState(!keepGoing || catastrophe || bubbleErrorInfo == null, - "", skyKeys, bubbleErrorInfo); + @Nullable ValueVisitor visitor, + Iterable<SkyKey> skyKeys, + @Nullable Map<SkyKey, ValueWithMetadata> bubbleErrorInfo, + boolean catastrophe) { + Preconditions.checkState( + catastrophe == (keepGoing && bubbleErrorInfo != null), + "Catastrophe not consistent with keepGoing mode and bubbleErrorInfo: %s %s %s %s", + skyKeys, + catastrophe, + keepGoing, + bubbleErrorInfo); EvaluationResult.Builder<T> result = EvaluationResult.builder(); List<SkyKey> cycleRoots = new ArrayList<>(); boolean hasError = false; @@ -1556,6 +1567,20 @@ public final class ParallelEvaluator implements Evaluator { Preconditions.checkState(bubbleErrorInfo == null || hasError, "If an error bubbled up, some top-level node must be in error", bubbleErrorInfo, skyKeys); result.setHasError(hasError); + if (catastrophe) { + // We may not have a top-level node completed. Inform the caller of the catastrophic exception + // that shut down the evaluation so that it has some context. + ErrorInfo errorInfo = + Preconditions.checkNotNull( + Iterables.getOnlyElement(bubbleErrorInfo.values()).getErrorInfo(), + "bubbleErrorInfo should have contained element with errorInfo: %s", + bubbleErrorInfo); + Preconditions.checkState( + errorInfo.isCatastrophic(), + "bubbleErrorInfo should have contained element with catastrophe: %s", + bubbleErrorInfo); + result.setCatastrophe(errorInfo.getException()); + } return result.build(); } diff --git a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java index 55b3bb982f..95f9e4057f 100644 --- a/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java +++ b/src/test/java/com/google/devtools/build/skyframe/ParallelEvaluatorTest.java @@ -556,25 +556,28 @@ public class ParallelEvaluatorTest { SkyKey catastropheKey = GraphTester.toSkyKey("catastrophe"); SkyKey otherKey = GraphTester.toSkyKey("someKey"); - tester.getOrCreate(catastropheKey).setBuilder(new SkyFunction() { - @Nullable - @Override - public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { - throw new SkyFunctionException(new SomeErrorException("bad"), - Transience.PERSISTENT) { - @Override - public boolean isCatastrophic() { - return true; - } - }; - } + final Exception catastrophe = new SomeErrorException("bad"); + tester + .getOrCreate(catastropheKey) + .setBuilder( + new SkyFunction() { + @Nullable + @Override + public SkyValue compute(SkyKey skyKey, Environment env) throws SkyFunctionException { + throw new SkyFunctionException(catastrophe, Transience.PERSISTENT) { + @Override + public boolean isCatastrophic() { + return true; + } + }; + } - @Nullable - @Override - public String extractTag(SkyKey skyKey) { - return null; - } - }); + @Nullable + @Override + public String extractTag(SkyKey skyKey) { + return null; + } + }); tester.getOrCreate(otherKey).setBuilder(new SkyFunction() { @Nullable @@ -600,6 +603,7 @@ public class ParallelEvaluatorTest { } else { assertTrue(result.hasError()); assertThat(result.errorMap()).isEmpty(); + assertThat(result.getCatastrophe()).isSameAs(catastrophe); } } |