Skip to content

Commit

Permalink
Attempt to tolerate missing keys a bit more gracefully: reset the cur…
Browse files Browse the repository at this point in the history
…rent 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
  • Loading branch information
janakdr authored and copybara-github committed Apr 11, 2019
1 parent e40141a commit 889f5fd
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -425,7 +425,7 @@ public void run() {
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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -156,12 +156,12 @@ private static ErrorInfo checkForCycles(SkyKey root, ParallelEvaluatorContext ev
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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public void post(ExtendedEventHandler.Postable e) {
GroupedList<SkyKey> directDeps,
Set<SkyKey> oldDeps,
ParallelEvaluatorContext evaluatorContext)
throws InterruptedException, UndonePreviouslyRequestedDep {
throws InterruptedException, UndonePreviouslyRequestedDeps {
super(directDeps);
this.skyKey = skyKey;
this.oldDeps = oldDeps;
Expand Down Expand Up @@ -193,10 +193,10 @@ public void post(ExtendedEventHandler.Postable e) {
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),
Expand All @@ -206,7 +206,7 @@ public void post(ExtendedEventHandler.Postable e) {

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);
Expand Down Expand Up @@ -235,6 +235,7 @@ private Map<SkyKey, SkyValue> batchPrefetch(
.getGraphInconsistencyReceiver()
.noteInconsistencyAndMaybeThrow(
requestor, difference, Inconsistency.ALREADY_DECLARED_CHILD_MISSING);
throw new UndonePreviouslyRequestedDeps(ImmutableList.copyOf(difference));
}
ImmutableMap.Builder<SkyKey, SkyValue> depValuesBuilder =
ImmutableMap.builderWithExpectedSize(batchMap.size());
Expand All @@ -251,7 +252,7 @@ private Map<SkyKey, SkyValue> batchPrefetch(
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) {
Expand Down Expand Up @@ -902,16 +903,16 @@ public String toString() {
.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;
}
}
}

0 comments on commit 889f5fd

Please sign in to comment.