Don't necessarily crash on a missing dep. Instead, pass it to the inconsistency receiver, for better stability while we debug. We should be able to recover from this situation reasonably gracefully.
PiperOrigin-RevId: 229944221
diff --git a/src/main/java/com/google/devtools/build/skyframe/BUILD b/src/main/java/com/google/devtools/build/skyframe/BUILD
index 7de5e27..b1ff51e 100644
--- a/src/main/java/com/google/devtools/build/skyframe/BUILD
+++ b/src/main/java/com/google/devtools/build/skyframe/BUILD
@@ -40,6 +40,7 @@
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
"//src/main/java/com/google/devtools/common/options",
+ "//third_party:flogger",
"//third_party:guava",
"//third_party:jsr305",
],
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 9f0d5cc..d43a163 100644
--- a/src/main/java/com/google/devtools/build/skyframe/GraphInconsistencyReceiver.java
+++ b/src/main/java/com/google/devtools/build/skyframe/GraphInconsistencyReceiver.java
@@ -32,7 +32,8 @@
RESET_REQUESTED,
CHILD_MISSING_FOR_DIRTY_NODE, // TODO(mschaller): put "parent" before "child" for consistency
PARENT_FORCE_REBUILD_OF_CHILD,
- BUILDING_PARENT_FOUND_UNDONE_CHILD
+ BUILDING_PARENT_FOUND_UNDONE_CHILD,
+ ALREADY_DECLARED_CHILD_MISSING
}
/** A {@link GraphInconsistencyReceiver} that crashes on any inconsistency. */
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 9c45ef6..a84ab55 100644
--- a/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
+++ b/src/main/java/com/google/devtools/build/skyframe/SkyFunctionEnvironment.java
@@ -20,6 +20,7 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
+import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
@@ -49,6 +50,8 @@
/** A {@link SkyFunction.Environment} implementation for {@link ParallelEvaluator}. */
class SkyFunctionEnvironment extends AbstractSkyFunctionEnvironment {
+ private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
+
private static final SkyValue NULL_MARKER = new SkyValue() {};
private static final boolean PREFETCH_OLD_DEPS =
Boolean.parseBoolean(
@@ -166,7 +169,7 @@
this.bubbleErrorInfo = null;
this.hermeticity = skyKey.functionName().getHermeticity();
this.previouslyRequestedDepsValues =
- batchPrefetch(skyKey, directDeps, oldDeps, /*assertDone=*/ true, skyKey);
+ batchPrefetch(skyKey, directDeps, oldDeps, /*assertDone=*/ true);
Preconditions.checkState(
!this.previouslyRequestedDepsValues.containsKey(ErrorTransienceValue.KEY),
"%s cannot have a dep on ErrorTransienceValue during building",
@@ -188,7 +191,7 @@
this.hermeticity = skyKey.functionName().getHermeticity();
try {
this.previouslyRequestedDepsValues =
- batchPrefetch(skyKey, directDeps, oldDeps, /*assertDone=*/ false, skyKey);
+ batchPrefetch(skyKey, directDeps, oldDeps, /*assertDone=*/ false);
} catch (UndonePreviouslyRequestedDep undonePreviouslyRequestedDep) {
throw new IllegalStateException(
"batchPrefetch can't throw UndonePreviouslyRequestedDep unless assertDone is true",
@@ -201,11 +204,7 @@
}
private Map<SkyKey, SkyValue> batchPrefetch(
- SkyKey requestor,
- GroupedList<SkyKey> depKeys,
- Set<SkyKey> oldDeps,
- boolean assertDone,
- SkyKey keyForDebugging)
+ SkyKey requestor, GroupedList<SkyKey> depKeys, Set<SkyKey> oldDeps, boolean assertDone)
throws InterruptedException, UndonePreviouslyRequestedDep {
QueryableGraph.PrefetchDepsRequest request = null;
if (PREFETCH_OLD_DEPS) {
@@ -224,16 +223,19 @@
try {
inFlightEntry = evaluatorContext.getGraph().get(null, Reason.OTHER, requestor);
} catch (InterruptedException e) {
+ logger.atWarning().withCause(e).log(
+ "Interrupted while getting parent entry for %s for crash", requestor);
// We're crashing, don't mask it.
Thread.currentThread().interrupt();
}
- throw new IllegalStateException(
- "Missing keys for "
- + keyForDebugging
- + ": "
- + Sets.difference(depKeys.toSet(), batchMap.keySet())
- + "\n\n"
- + inFlightEntry);
+ Set<SkyKey> difference = Sets.difference(depKeys.toSet(), batchMap.keySet());
+ logger.atSevere().log("Missing keys for %s: %s\n\n%s", requestor, difference, inFlightEntry);
+ for (SkyKey missingDep : difference) {
+ evaluatorContext
+ .getGraphInconsistencyReceiver()
+ .noteInconsistencyAndMaybeThrow(
+ requestor, missingDep, Inconsistency.ALREADY_DECLARED_CHILD_MISSING);
+ }
}
ImmutableMap.Builder<SkyKey, SkyValue> depValuesBuilder =
ImmutableMap.builderWithExpectedSize(batchMap.size());
