Attempt to tolerate missing keys a bit more gracefully: reset the current node to restart from scratch, rather than continuing on an almost certainly doomed path, where we'll crash later.
This is explicitly abusing the action rewinding framework to mitigate a different bug.
PiperOrigin-RevId: 243119106
diff --git a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
index 79c0cdb..cc0e038 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
@@ -40,7 +40,7 @@
import com.google.devtools.build.skyframe.ParallelEvaluatorContext.EnqueueParentBehavior;
import com.google.devtools.build.skyframe.QueryableGraph.Reason;
import com.google.devtools.build.skyframe.SkyFunction.Restart;
-import com.google.devtools.build.skyframe.SkyFunctionEnvironment.UndonePreviouslyRequestedDep;
+import com.google.devtools.build.skyframe.SkyFunctionEnvironment.UndonePreviouslyRequestedDeps;
import com.google.devtools.build.skyframe.SkyFunctionException.ReifiedSkyFunctionException;
import com.google.devtools.build.skyframe.ThinNodeEntry.DirtyType;
import java.math.BigInteger;
@@ -425,7 +425,7 @@
env =
new SkyFunctionEnvironment(
skyKey, state.getTemporaryDirectDeps(), oldDeps, evaluatorContext);
- } catch (UndonePreviouslyRequestedDep undonePreviouslyRequestedDep) {
+ } catch (UndonePreviouslyRequestedDeps undonePreviouslyRequestedDeps) {
// If a previously requested dep is no longer done, restart this node from scratch.
restart(skyKey, state);
// Top priority since this node has already been evaluating, so get it off our plate.
diff --git a/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java b/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java
index fecf359..732fc4c 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SimpleCycleDetector.java
@@ -26,7 +26,7 @@
import com.google.devtools.build.lib.util.GroupedList;
import com.google.devtools.build.skyframe.ParallelEvaluatorContext.EnqueueParentBehavior;
import com.google.devtools.build.skyframe.QueryableGraph.Reason;
-import com.google.devtools.build.skyframe.SkyFunctionEnvironment.UndonePreviouslyRequestedDep;
+import com.google.devtools.build.skyframe.SkyFunctionEnvironment.UndonePreviouslyRequestedDeps;
import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Deque;
@@ -156,12 +156,12 @@
directDeps,
Sets.difference(entry.getAllRemainingDirtyDirectDeps(), removedDeps),
evaluatorContext);
- } catch (UndonePreviouslyRequestedDep undoneDep) {
+ } catch (UndonePreviouslyRequestedDeps undoneDep) {
// All children were finished according to the CHILDREN_FINISHED sentinel, and cycle
// detection does not do normal SkyFunction evaluation, so no restarting nor child
// dirtying was possible.
throw new IllegalStateException(
- "Previously requested dep not done: " + undoneDep.getDepKey(), undoneDep);
+ "Previously requested dep not done: " + undoneDep.getDepKeys(), undoneDep);
}
env.setError(entry, ErrorInfo.fromChildErrors(key, errorDeps));
env.commit(entry, EnqueueParentBehavior.SIGNAL);
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
index e0514d2..5a80ac4 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -162,7 +162,7 @@
GroupedList<SkyKey> directDeps,
Set<SkyKey> oldDeps,
ParallelEvaluatorContext evaluatorContext)
- throws InterruptedException, UndonePreviouslyRequestedDep {
+ throws InterruptedException, UndonePreviouslyRequestedDeps {
super(directDeps);
this.skyKey = skyKey;
this.oldDeps = oldDeps;
@@ -193,10 +193,10 @@
try {
this.previouslyRequestedDepsValues =
batchPrefetch(skyKey, directDeps, oldDeps, /*assertDone=*/ false);
- } catch (UndonePreviouslyRequestedDep undonePreviouslyRequestedDep) {
+ } catch (UndonePreviouslyRequestedDeps undonePreviouslyRequestedDeps) {
throw new IllegalStateException(
- "batchPrefetch can't throw UndonePreviouslyRequestedDep unless assertDone is true",
- undonePreviouslyRequestedDep);
+ "batchPrefetch can't throw UndonePreviouslyRequestedDeps unless assertDone is true",
+ undonePreviouslyRequestedDeps);
}
Preconditions.checkState(
!this.previouslyRequestedDepsValues.containsKey(ErrorTransienceValue.KEY),
@@ -206,7 +206,7 @@
private Map<SkyKey, SkyValue> batchPrefetch(
SkyKey requestor, GroupedList<SkyKey> depKeys, Set<SkyKey> oldDeps, boolean assertDone)
- throws InterruptedException, UndonePreviouslyRequestedDep {
+ throws InterruptedException, UndonePreviouslyRequestedDeps {
QueryableGraph.PrefetchDepsRequest request = null;
if (PREFETCH_OLD_DEPS) {
request = new QueryableGraph.PrefetchDepsRequest(requestor, oldDeps, depKeys);
@@ -235,6 +235,7 @@
.getGraphInconsistencyReceiver()
.noteInconsistencyAndMaybeThrow(
requestor, difference, Inconsistency.ALREADY_DECLARED_CHILD_MISSING);
+ throw new UndonePreviouslyRequestedDeps(ImmutableList.copyOf(difference));
}
ImmutableMap.Builder<SkyKey, SkyValue> depValuesBuilder =
ImmutableMap.builderWithExpectedSize(batchMap.size());
@@ -251,7 +252,7 @@
skyKey,
ImmutableList.of(entry.getKey()),
Inconsistency.BUILDING_PARENT_FOUND_UNDONE_CHILD);
- throw new UndonePreviouslyRequestedDep(entry.getKey());
+ throw new UndonePreviouslyRequestedDeps(ImmutableList.of(entry.getKey()));
}
depValuesBuilder.put(entry.getKey(), !depDone ? NULL_MARKER : valueMaybeWithMetadata);
if (depDone) {
@@ -902,16 +903,16 @@
.toString();
}
- /** Thrown during environment construction if a previously requested dep is no longer done. */
- static class UndonePreviouslyRequestedDep extends Exception {
- private final SkyKey depKey;
+ /** Thrown during environment construction if previously requested deps are no longer done. */
+ static class UndonePreviouslyRequestedDeps extends Exception {
+ private final ImmutableList<SkyKey> depKeys;
- UndonePreviouslyRequestedDep(SkyKey depKey) {
- this.depKey = depKey;
+ UndonePreviouslyRequestedDeps(ImmutableList<SkyKey> depKeys) {
+ this.depKeys = depKeys;
}
- SkyKey getDepKey() {
- return depKey;
+ ImmutableList<SkyKey> getDepKeys() {
+ return depKeys;
}
}
}
