File report if rewinding fails and an action observes a lost input twice
RELNOTES: None.
PiperOrigin-RevId: 233454364
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index f0c00f4..c95360b 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -646,6 +646,7 @@
":transitive-info-provider",
],
deps = [
+ ":bug-report",
":build-request-options",
":command-utils",
":events",
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
index fa1ec3a..bcca7c7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionRewindStrategy.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.LostInputsExecException;
import com.google.devtools.build.lib.actions.LostInputsExecException.LostInputsActionExecutionException;
+import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyFunction.Restart;
import com.google.devtools.build.skyframe.SkyKey;
@@ -137,24 +138,23 @@
throws ActionExecutionException {
ImmutableMap<String, ActionInput> lostInputsByDigest = lostInputsException.getLostInputs();
for (String digest : lostInputsByDigest.keySet()) {
+ // The same action losing the same input twice is unexpected. The action should have waited
+ // until the depended-on action which generates the lost input is (re)run before trying
+ // again.
+ //
+ // Note that we could enforce a stronger check: if action A, which depends on an input N
+ // previously detected as lost (by any action, not just A), discovers that N is still lost,
+ // and action A started after the re-evaluation of N's generating action, then something has
+ // gone wrong. Administering that check would be more complex (e.g., the start/completion
+ // times of actions would need tracking), so we punt on it for now.
if (!lostInputRecords.add(LostInputRecord.create(actionLookupData, digest))) {
- // The same action losing the same input twice is unexpected. The action should have waited
- // until the depended-on action which generates the lost input is (re)run before trying
- // again. We tolerate such a loss--by failing the build instead of crashing--in case some
- // kind of infrastructure failure results in an input being lost twice.
- //
- // Note that we could enforce a stronger check: if action A, which depends on an input I
- // previously detected as lost (by any action, not just A), discovers that I is still lost,
- // and action A started after the re-evaluation of I's generating action, then something has
- // gone wrong. Administering that check would be more complex (e.g., the start/completion
- // times of actions would need tracking), so we punt on it for now.
- logger.severe(
- String.format(
- "lost input twice for the same action.\nlostInput: %s\nlostInput digest: %s\n"
- + "failedAction: %s",
- lostInputsByDigest.get(digest), digest, failedAction));
- // Launder the LostInputs exception as a plain ActionExecutionException so that it may be
- // processed by SkyframeActionExecutor without short-circuiting.
+ BugReport.sendBugReport(
+ new IllegalStateException(
+ String.format(
+ "lost input twice for the same action. lostInput: %s, lostInput digest: %s, "
+ + "failedAction: %s",
+ lostInputsByDigest.get(digest), digest, failedAction)),
+ ImmutableList.of());
throw new ActionExecutionException(
lostInputsException, failedAction, /*catastrophe=*/ false);
}
@@ -173,7 +173,7 @@
// failure results in their apparent loss.
logger.info(
String.format(
- "lostArtifact unexpectedly source.\nlostArtifact: %s\nlostInputs for artifact: %s\n"
+ "lostArtifact unexpectedly source. lostArtifact: %s, lostInputs for artifact: %s, "
+ "failedAction: %s",
lostArtifact, associatedLostInputs, failedAction));
// Launder the LostInputs exception as a plain ActionExecutionException so that it may be
@@ -210,8 +210,8 @@
if (failedActionDeps.contains(lostInput)) {
Preconditions.checkState(
lostInput instanceof Artifact,
- "unexpected non-artifact lostInput which is a dep of the current action.\n"
- + "lostInput: %s\nfailedAction: %s",
+ "unexpected non-artifact lostInput which is a dep of the current action. "
+ + "lostInput: %s, failedAction: %s",
lostInput,
failedActionForLogging);
lostInputsByDepOwners.put((Artifact) lostInput, lostInput);
@@ -251,12 +251,13 @@
// case, reevaluating the failed action (and no other deps) may help, because doing so may
// rerun the generating spawn.
//
- // It may also happen because of a bug, so we log that this has occurred.
+ // In other cases, such as with bugs, the second time the action fails will cause a crash in
+ // checkIfActionLostInputTwice. We log that this has occurred.
logger.info(
String.format(
- "lostInput not a dep of the failed action, and can't be associated with such a dep.\n"
- + "lostInput: %s\nowner: %s\nrunfilesDepOwner: %s\nrunfilesDepTransitiveOwner: %s"
- + "\nfailedAction: %s",
+ "lostInput not a dep of the failed action, and can't be associated with such a dep. "
+ + "lostInput: %s, owner: %s, runfilesDepOwner: %s, runfilesDepTransitiveOwner: %s"
+ + ", failedAction: %s",
lostInput,
owner,
runfilesDepOwner,
