Basic action rewinding test and implementation
Given two genrules, where the 1st produces an output file which is
consumed as an input by the 2nd, if the intermediate file goes missing
before the 2nd genrule has the chance to execute, we'd like to be able
to "rewind" the 1st genrule's action, and make it regenerate its output.
This CL demonstrates that ActionExecutionFunction/SkyframeActionExecutor
support the simple case of rewinding actions for a chain of two genrules.
The action execution node corresponding to the failed action discovers
the set of Skyframe nodes to restart by evaluating, inline, the
dependencies of artifacts associated with its lost inputs. The edges
it adds while doing this are deleted when it restarts.
This is intended as a foundation for future development. Note that
LostInputsExecException is only constructed in tests. Support for
throwing it must still be added to execution strategies.
Future changes will add support for other kinds of actions and artifacts,
and put a limit on how often rewinding is tolerated before failing the
build.
Action rewinding currently depends on Skyframe node restarting, which is
supported only for evaluations which don't track reverse dependencies
between Skyframe nodes.
RELNOTES: None.
PiperOrigin-RevId: 215984081
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecException.java b/src/main/java/com/google/devtools/build/lib/actions/ExecException.java
index a217ac2..a44e925 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ExecException.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ExecException.java
@@ -15,30 +15,33 @@
package com.google.devtools.build.lib.actions;
/**
- * An exception indication that the execution of an action has failed OR could
- * not be attempted OR could not be finished OR had something else wrong.
+ * An exception indication that the execution of an action has failed OR could not be attempted OR
+ * could not be finished OR had something else wrong.
*
- * <p>The four main kinds of failure are broadly defined as follows:
+ * <p>The five main kinds of failure are broadly defined as follows:
*
- * <p>USER_INPUT which means it had something to do with what the user told us
- * to do. This failure should satisfy the invariant that it would happen
- * identically again if all other things are equal.
+ * <p>USER_INPUT which means it had something to do with what the user told us to do. This failure
+ * should satisfy the invariant that it would happen identically again if all other things are
+ * equal.
*
- * <p>ENVIRONMENT which is loosely defined as anything which is generally out of
- * scope for a blaze evaluation. As a rule of thumb, these are any errors would
- * not necessarily happen again given constant input.
+ * <p>ENVIRONMENT which is loosely defined as anything which is generally out of scope for a blaze
+ * evaluation. As a rule of thumb, these are any errors would not necessarily happen again given
+ * constant input.
*
- * <p>INTERRUPTION conditions arise from being unable to complete an evaluation
- * for whatever reason.
+ * <p>INTERRUPTION conditions arise from being unable to complete an evaluation for whatever reason.
*
- * <p>INTERNAL_ERROR would happen because of anything which arises from within
- * blaze itself but is generally unexpected to ever occur for any user input.
+ * <p>INTERNAL_ERROR would happen because of anything which arises from within blaze itself but is
+ * generally unexpected to ever occur for any user input.
*
- * <p>The class is a catch-all for both failures of actions and failures to
- * evaluate actions properly.
+ * <p>LOST_INPUT which means the failure occurred because the action expected to consume some input
+ * that went missing. Although this seems similar to ENVIRONMENT, Blaze may know how to fix this
+ * problem.
*
- * <p>Invariably, all low level ExecExceptions are caught by various specific
- * ConfigurationAction classes and re-raised as ActionExecutionExceptions.
+ * <p>The class is a catch-all for both failures of actions and failures to evaluate actions
+ * properly.
+ *
+ * <p>Invariably, all low level ExecExceptions are caught by various specific ConfigurationAction
+ * classes and re-raised as ActionExecutionExceptions.
*/
public abstract class ExecException extends Exception {
@@ -52,7 +55,7 @@
public ExecException(String message) {
this(message, false);
}
-
+
public ExecException(Throwable cause) {
super(cause);
this.catastrophe = false;
@@ -67,21 +70,20 @@
this(message, cause, false);
}
- /**
- * Catastrophic exceptions should stop the build, even if --keep_going.
- */
+ /** Catastrophic exceptions should stop the build, even if --keep_going. */
public boolean isCatastrophic() {
return catastrophe;
}
/**
* Returns a new ActionExecutionException without a message prefix.
+ *
* @param action failed action
* @return ActionExecutionException object describing the action failure
*/
public ActionExecutionException toActionExecutionException(Action action) {
// In all ExecException implementations verboseFailures argument used only to determine should
- // we pass ExecException as cause of ActionExecutionException. So use this method only
+ // we pass ExecException as cause of ActionExecutionException. So use this method only
// if you need this information inside of ActionExecutionexception.
return toActionExecutionException("", true, action);
}
@@ -96,6 +98,6 @@
* @param action failed action
* @return ActionExecutionException object describing the action failure
*/
- public abstract ActionExecutionException toActionExecutionException(String messagePrefix,
- boolean verboseFailures, Action action);
+ public abstract ActionExecutionException toActionExecutionException(
+ String messagePrefix, boolean verboseFailures, Action action);
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java
new file mode 100644
index 0000000..3581598
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java
@@ -0,0 +1,55 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.actions;
+
+import com.google.common.collect.ImmutableList;
+
+/**
+ * An {@link ExecException} thrown when an action fails to execute because one or more of its inputs
+ * was lost. In some cases, Bazel may know how to fix this on its own.
+ */
+public class LostInputsExecException extends ExecException {
+
+ private final ImmutableList<ActionInput> lostInputs;
+
+ public LostInputsExecException(ImmutableList<ActionInput> lostInputs) {
+ super("");
+ this.lostInputs = lostInputs;
+ }
+
+ private ImmutableList<ActionInput> getLostInputs() {
+ return lostInputs;
+ }
+
+ @Override
+ public ActionExecutionException toActionExecutionException(
+ String messagePrefix, boolean verboseFailures, Action action) {
+ String message = messagePrefix + " failed";
+ return new LostInputsActionExecutionException(message, this, action);
+ }
+
+ /** An {@link ActionExecutionException} wrapping a {@link LostInputsExecException}. */
+ public static class LostInputsActionExecutionException extends ActionExecutionException {
+
+ private LostInputsActionExecutionException(
+ String message, LostInputsExecException cause, Action action) {
+ super(message, cause, action, /*catastrophe=*/ false);
+ }
+
+ public ImmutableList<ActionInput> getLostInputs() {
+ return ((LostInputsExecException) getCause()).getLostInputs();
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
index 00595b5..ca0921e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java
@@ -34,6 +34,7 @@
import com.google.devtools.build.lib.actions.ArtifactSkyKey;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
+import com.google.devtools.build.lib.actions.LostInputsExecException.LostInputsActionExecutionException;
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.MissingDepException;
import com.google.devtools.build.lib.actions.MissingInputFileException;
@@ -48,6 +49,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.rules.cpp.IncludeScannable;
+import com.google.devtools.build.lib.skyframe.ActionRewindStrategy.RewindPlan;
import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -85,6 +87,7 @@
* </ol>
*/
public class ActionExecutionFunction implements SkyFunction, CompletionReceiver {
+ private final ActionRewindStrategy actionRewindStrategy = new ActionRewindStrategy();
private final SkyframeActionExecutor skyframeActionExecutor;
private final BlazeDirectories directories;
private final AtomicReference<TimestampGranularityMonitor> tsgm;
@@ -104,10 +107,7 @@
public SkyValue compute(SkyKey skyKey, Environment env)
throws ActionExecutionFunctionException, InterruptedException {
ActionLookupData actionLookupData = (ActionLookupData) skyKey.argument();
- ActionLookupValue actionLookupValue =
- (ActionLookupValue) env.getValue(actionLookupData.getActionLookupKey());
- int actionIndex = actionLookupData.getActionIndex();
- Action action = actionLookupValue.getAction(actionIndex);
+ Action action = getActionForLookupData(env, actionLookupData);
skyframeActionExecutor.noteActionEvaluationStarted(actionLookupData, action);
if ((action.isVolatile() && !(action instanceof SkyframeAwareAction))
|| action instanceof NotifyOnActionCacheHit) {
@@ -157,18 +157,16 @@
Preconditions.checkState(!env.valuesMissing(), "%s %s", action, state);
}
CheckInputResults checkedInputs = null;
+ Iterable<SkyKey> inputDepKeys =
+ toKeys(
+ state.allInputs.getAllInputs(),
+ action.discoversInputs() ? action.getMandatoryInputs() : null);
+ // Declare deps on known inputs to action. We do this unconditionally to maintain our
+ // invariant of asking for the same deps each build.
+ Map<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>> inputDeps =
+ env.getValuesOrThrow(
+ inputDepKeys, MissingInputFileException.class, ActionExecutionException.class);
try {
- // Declare deps on known inputs to action. We do this unconditionally to maintain our
- // invariant of asking for the same deps each build.
- Map<SkyKey, ValueOrException2<MissingInputFileException, ActionExecutionException>>
- inputDeps =
- env.getValuesOrThrow(
- toKeys(
- state.allInputs.getAllInputs(),
- action.discoversInputs() ? action.getMandatoryInputs() : null),
- MissingInputFileException.class,
- ActionExecutionException.class);
-
if (!sharedActionAlreadyRan && !state.hasArtifactData()) {
// Do we actually need to find our metadata?
checkedInputs = checkInputs(env, action, inputDeps);
@@ -216,6 +214,14 @@
checkCacheAndExecuteIfNeeded(
action, state, env, clientEnv, actionLookupData, sharedActionAlreadyRan,
skyframeDepsResult);
+ } catch (LostInputsActionExecutionException e) {
+ // Remove action from state map in case it's there (won't be unless it discovers inputs).
+ stateMap.remove(action);
+ RewindPlan rewindPlan = actionRewindStrategy.getRewindPlan(action, inputDepKeys, e, env);
+ for (Action actionToReset : rewindPlan.getActionsToReset()) {
+ skyframeActionExecutor.resetActionExecution(actionToReset);
+ }
+ return rewindPlan.getNodesToRestart();
} catch (ActionExecutionException e) {
// Remove action from state map in case it's there (won't be unless it discovers inputs).
stateMap.remove(action);
@@ -239,6 +245,18 @@
return result;
}
+ static Action getActionForLookupData(Environment env, ActionLookupData actionLookupData)
+ throws InterruptedException {
+ // Because of the phase boundary separating analysis and execution, all needed
+ // ActionLookupValues must have already been evaluated.
+ ActionLookupValue actionLookupValue =
+ Preconditions.checkNotNull(
+ (ActionLookupValue) env.getValue(actionLookupData.getActionLookupKey()),
+ "ActionLookupValue missing: %s",
+ actionLookupData);
+ return actionLookupValue.getAction(actionLookupData.getActionIndex());
+ }
+
/**
* An action's inputs needed for execution. May not just be the result of Action#getInputs(). If
* the action cache's view of this action contains additional inputs, it will request metadata for
@@ -877,11 +895,11 @@
* Used to declare all the exception types that can be wrapped in the exception thrown by {@link
* ActionExecutionFunction#compute}.
*/
- private static final class ActionExecutionFunctionException extends SkyFunctionException {
+ static final class ActionExecutionFunctionException extends SkyFunctionException {
private final ActionExecutionException actionException;
- public ActionExecutionFunctionException(ActionExecutionException e) {
+ ActionExecutionFunctionException(ActionExecutionException e) {
// We conservatively assume that the error is transient. We don't have enough information to
// distinguish non-transient errors (e.g. compilation error from a deterministic compiler)
// from transient ones (e.g. IO error).
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
new file mode 100644
index 0000000..9292565
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
@@ -0,0 +1,145 @@
+// Copyright 2018 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.skyframe;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
+import com.google.devtools.build.lib.actions.Action;
+import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionLookupData;
+import com.google.devtools.build.lib.actions.AlreadyReportedActionExecutionException;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.LostInputsExecException.LostInputsActionExecutionException;
+import com.google.devtools.build.lib.skyframe.ActionExecutionFunction.ActionExecutionFunctionException;
+import com.google.devtools.build.skyframe.SkyFunction.Environment;
+import com.google.devtools.build.skyframe.SkyFunction.Restart;
+import com.google.devtools.build.skyframe.SkyKey;
+import java.util.HashSet;
+import java.util.Set;
+import javax.annotation.Nullable;
+
+/**
+ * Given an action that failed to execute because of missing inputs which were generated by other
+ * actions, this finds the Skyframe nodes corresponding to those inputs and the actions which
+ * generated them.
+ */
+class ActionRewindStrategy {
+
+ /**
+ * Returns a {@link RewindPlan} specifying:
+ *
+ * <ol>
+ * <li>the Skyframe nodes to restart to recreate the lost inputs specified by {@code
+ * lostInputsException}
+ * <li>the actions whose execution state (in {@link SkyframeActionExecutor}) must be reset
+ * </ol>
+ *
+ * @throws ActionExecutionFunctionException if any lost inputs are not the outputs of previously
+ * executed actions
+ */
+ // TODO(mschaller): support special/tree artifact types
+ RewindPlan getRewindPlan(
+ Action failedAction,
+ Iterable<SkyKey> inputDepKeys,
+ LostInputsActionExecutionException lostInputsException,
+ Environment env)
+ throws ActionExecutionFunctionException, InterruptedException {
+ ImmutableList<ActionInput> lostInputs = lostInputsException.getLostInputs();
+ for (ActionInput actionInput : lostInputs) {
+ if (!(actionInput instanceof Artifact) || ((Artifact) actionInput).isSourceArtifact()) {
+ throw new ActionExecutionFunctionException(
+ new AlreadyReportedActionExecutionException(lostInputsException));
+ }
+ }
+
+ // Find the action execution nodes that lost inputs depend on.
+ HashSet<SkyKey> depsToRestart = Sets.newHashSet();
+ ImmutableSet<SkyKey> inputDepKeysSet = ImmutableSet.copyOf(inputDepKeys);
+ ImmutableList.Builder<Action> actionsToReset = ImmutableList.builder();
+ for (ActionInput lostInput : lostInputs) {
+ Preconditions.checkState(
+ inputDepKeysSet.contains(lostInput),
+ "Lost input not a dep of action.\nLost input: %s\nDeps: %s\nAction: %s",
+ lostInput,
+ inputDepKeysSet,
+ failedAction);
+ depsToRestart.add((Artifact) lostInput);
+
+ Set<SkyKey> depsOfArtifact = getDepsOfArtifact((Artifact) lostInput, env);
+ if (depsOfArtifact == null) {
+ // Some deps of the artifact are not done. Another rewind must be in-flight, and there is no
+ // need to restart the shared deps twice.
+ continue;
+ }
+ // Restart the execution-phase dependencies of the artifact.
+ depsToRestart.addAll(depsOfArtifact);
+ }
+
+ // SkyframeActionExecutor must re-execute the actions being restarted, so we must tell it to
+ // evict its cached action results.
+ actionsToReset.add(failedAction);
+ for (SkyKey depToRestart : depsToRestart) {
+ if (!(depToRestart instanceof ActionLookupData)) {
+ continue;
+ }
+ actionsToReset.add(
+ ActionExecutionFunction.getActionForLookupData(env, (ActionLookupData) depToRestart));
+ }
+
+ return new RewindPlan(
+ Restart.selfAnd(ImmutableList.copyOf(depsToRestart)), actionsToReset.build());
+ }
+
+ /**
+ * Returns the set of {@code lostInput}'s execution-phase dependencies, or {@code null} if any of
+ * those dependencies are not done.
+ */
+ @Nullable
+ private Set<SkyKey> getDepsOfArtifact(Artifact lostInput, Environment env)
+ throws InterruptedException {
+ ArtifactFunction.ArtifactDependencies artifactDependencies =
+ ArtifactFunction.ArtifactDependencies.discoverDependencies(lostInput, env);
+ if (artifactDependencies == null) {
+ return null;
+ }
+ Preconditions.checkState(
+ !artifactDependencies.isTemplateActionForTreeArtifact(),
+ "Rewinding template actions not yet supported: %s",
+ artifactDependencies);
+ // TODO(mschaller): extend ArtifactDependencies to support template actions (and other special
+ // cases)
+ return ImmutableSet.of(artifactDependencies.getNontemplateActionExecutionKey());
+ }
+
+ static class RewindPlan {
+ private final Restart nodesToRestart;
+ private final ImmutableList<Action> actionsToReset;
+
+ RewindPlan(Restart nodesToRestart, ImmutableList<Action> actionsToReset) {
+ this.nodesToRestart = nodesToRestart;
+ this.actionsToReset = actionsToReset;
+ }
+
+ Restart getNodesToRestart() {
+ return nodesToRestart;
+ }
+
+ ImmutableList<Action> getActionsToReset() {
+ return actionsToReset;
+ }
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
index a03a8be..0171573 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactFunction.java
@@ -366,7 +366,7 @@
return value;
}
- private static final class ArtifactFunctionException extends SkyFunctionException {
+ static final class ArtifactFunctionException extends SkyFunctionException {
ArtifactFunctionException(MissingInputFileException e, Transience transience) {
super(e, transience);
}
@@ -407,7 +407,7 @@
/**
* Constructs an {@link ArtifactDependencies} for the provided {@code derivedArtifact}, which
- * must not be a source artifact.
+ * must not be a source artifact. Returns {@code null} if any dependencies are not yet ready.
*/
@Nullable
static ArtifactDependencies discoverDependencies(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
index ec79422..378ddc3 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -58,6 +58,7 @@
import com.google.devtools.build.lib.actions.Executor;
import com.google.devtools.build.lib.actions.FileArtifactValue;
import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
+import com.google.devtools.build.lib.actions.LostInputsExecException.LostInputsActionExecutionException;
import com.google.devtools.build.lib.actions.MapBasedActionGraph;
import com.google.devtools.build.lib.actions.MetadataConsumer;
import com.google.devtools.build.lib.actions.MetadataProvider;
@@ -427,6 +428,10 @@
return buildActionMap.containsKey(new OwnerlessArtifactWrapper(action.getPrimaryOutput()));
}
+ void resetActionExecution(Action action) {
+ buildActionMap.remove(new OwnerlessArtifactWrapper(action.getPrimaryOutput()));
+ }
+
private boolean actionReallyExecuted(Action action, ActionLookupData actionLookupData) {
Pair<ActionLookupData, ?> cachedRun =
Preconditions.checkNotNull(
@@ -1007,6 +1012,10 @@
return true;
}
// Defer reporting action success until outputs are checked
+ } catch (LostInputsActionExecutionException e) {
+ // If inputs are lost, then avoid publishing ActionExecutedEvents. A higher-level handler may
+ // try to fix things.
+ throw e;
} catch (ActionExecutionException e) {
throw processAndThrow(
eventHandler,
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 d168281..6fea594 100644
--- a/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
+++ b/src/main/java/com/google/devtools/build/skyframe/AbstractParallelEvaluator.java
@@ -573,7 +573,7 @@
evaluatorContext
.getGraphInconsistencyReceiver()
.noteInconsistencyAndMaybeThrow(
- skyKey, childErrorKey, Inconsistency.CHILD_UNDONE_FOR_BUILDING_NODE);
+ skyKey, childErrorKey, Inconsistency.BUILDING_PARENT_FOUND_UNDONE_CHILD);
}
}
SkyValue childErrorInfoMaybe =
@@ -710,9 +710,7 @@
evaluatorContext
.getGraphInconsistencyReceiver()
.noteInconsistencyAndMaybeThrow(
- restartEntry.getKey(),
- /*otherKey=*/ key,
- Inconsistency.CHILD_FORCED_REEVALUATION_BY_PARENT);
+ key, restartEntry.getKey(), Inconsistency.PARENT_FORCE_REBUILD_OF_CHILD);
// Nodes are marked "force-rebuild" to ensure that they run, and to allow them to evaluate to
// a different value than before, even if their versions remain the same.
restartEntry.getValue().markDirty(DirtyType.FORCE_REBUILD);
@@ -848,7 +846,7 @@
evaluatorContext
.getGraphInconsistencyReceiver()
.noteInconsistencyAndMaybeThrow(
- skyKey, depKey, Inconsistency.CHILD_UNDONE_FOR_BUILDING_NODE);
+ skyKey, depKey, Inconsistency.BUILDING_PARENT_FOUND_UNDONE_CHILD);
if (triState == DependencyState.NEEDS_SCHEDULING) {
// Top priority since this depKey was already evaluated before, and we want to finish it off
// again, reducing the chance that another node may observe this dep to be undone.
diff --git a/src/main/java/com/google/devtools/build/skyframe/GraphInconsistencyReceiver.java b/src/main/java/com/google/devtools/build/skyframe/GraphInconsistencyReceiver.java
index c668b26..9f0d5cc 100644
--- a/src/main/java/com/google/devtools/build/skyframe/GraphInconsistencyReceiver.java
+++ b/src/main/java/com/google/devtools/build/skyframe/GraphInconsistencyReceiver.java
@@ -30,9 +30,9 @@
/** The type of inconsistency detected. */
enum Inconsistency {
RESET_REQUESTED,
- CHILD_MISSING_FOR_DIRTY_NODE,
- CHILD_FORCED_REEVALUATION_BY_PARENT,
- CHILD_UNDONE_FOR_BUILDING_NODE
+ CHILD_MISSING_FOR_DIRTY_NODE, // TODO(mschaller): put "parent" before "child" for consistency
+ PARENT_FORCE_REBUILD_OF_CHILD,
+ BUILDING_PARENT_FOUND_UNDONE_CHILD
}
/** A {@link GraphInconsistencyReceiver} that crashes on any inconsistency. */
diff --git a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java
index 5a5748d..cf66d32 100644
--- a/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java
+++ b/src/main/java/com/google/devtools/build/skyframe/ParallelEvaluatorContext.java
@@ -105,7 +105,8 @@
}
Map<SkyKey, ? extends NodeEntry> getBatchValues(
- @Nullable SkyKey parent, Reason reason, Iterable<SkyKey> keys) throws InterruptedException {
+ @Nullable SkyKey parent, Reason reason, Iterable<? extends SkyKey> keys)
+ throws InterruptedException {
return graph.getBatch(parent, reason, keys);
}
diff --git a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java
index 1ef2971..86f1781 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunction.java
@@ -94,11 +94,11 @@
return selfAnd(ImmutableList.copyOf(additionalKeysToRestart));
}
- static Restart selfAnd(ImmutableList<SkyKey> additionalKeysToRestart) {
+ static Restart selfAnd(ImmutableList<? extends SkyKey> additionalKeysToRestart) {
return () -> additionalKeysToRestart;
}
- ImmutableList<SkyKey> getAdditionalKeysToRestart();
+ ImmutableList<? extends SkyKey> getAdditionalKeysToRestart();
}
/**
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 2d8083f..d773d20 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -245,7 +245,7 @@
evaluatorContext
.getGraphInconsistencyReceiver()
.noteInconsistencyAndMaybeThrow(
- skyKey, entry.getKey(), Inconsistency.CHILD_UNDONE_FOR_BUILDING_NODE);
+ skyKey, entry.getKey(), Inconsistency.BUILDING_PARENT_FOUND_UNDONE_CHILD);
throw new UndonePreviouslyRequestedDep(entry.getKey());
}
depValuesBuilder.put(entry.getKey(), !depDone ? NULL_MARKER : valueMaybeWithMetadata);
