Have ActionRewindStrategy throw a more general `ActionRewindExcepti…

…on` when an action loses inputs too many times. `ActionRewindException` is thrown instead of `ActionExecutionException`, which is specific to `ActionExecutionFunction`. When rewinding top-level outputs, we also need to handle the possibility of ineffective rewinding, but an `ActionExecutionException` cannot be constructed since there is no failed `Action` associated with the lost digest. Instead, have the caller catch `ActionRewindException` and construct a relevant exception type. PiperOrigin-RevId: 604668674 Change-Id: I61a1bfb2f383cebe7852f33b08f0117b3e0d1dd3
iancha1992 · Feb 6, 2024 · 1a46a6f · 1a46a6f
1 parent 6932153
commit 1a46a6f
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 29 deletions.
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
@@ -87,6 +87,7 @@
 import com.google.devtools.build.lib.skyframe.ArtifactFunction.SourceArtifactException;
 import com.google.devtools.build.lib.skyframe.ArtifactNestedSetFunction.ArtifactNestedSetEvalException;
 import com.google.devtools.build.lib.skyframe.SkyframeActionExecutor.ActionPostprocessing;
+import com.google.devtools.build.lib.skyframe.rewinding.ActionRewindException;
 import com.google.devtools.build.lib.skyframe.rewinding.ActionRewindStrategy;
 import com.google.devtools.build.lib.skyframe.rewinding.ActionRewindStrategy.RewindPlan;
 import com.google.devtools.build.lib.skyframe.rewinding.ActionRewoundEvent;
@@ -531,7 +532,7 @@ private SkyFunction.Reset handleLostInputs(
       rewindPlan =
           actionRewindStrategy.getRewindPlan(
               actionLookupData, action, failedActionDeps, e, inputDepOwners, env);
-    } catch (ActionExecutionException rewindingFailedException) {
+    } catch (ActionRewindException rewindingFailedException) {
       // This ensures coalesced shared actions aren't orphaned.
       skyframeActionExecutor.prepareForRewinding(
           actionLookupData, action, /* depsToRewind= */ ImmutableList.of());
@@ -541,7 +542,11 @@ private SkyFunction.Reset handleLostInputs(
                   env.getListener(),
                   e.getPrimaryOutputPath(),
                   action,
-                  rewindingFailedException,
+                  new ActionExecutionException(
+                      e,
+                      action,
+                      /* catastrophe= */ false,
+                      rewindingFailedException.getDetailedExitCode()),
                   e.getFileOutErr(),
                   ActionExecutedEvent.ErrorTiming.AFTER_EXECUTION)));
     } catch (UndoneInputsException undoneInputsException) {

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/rewinding/ActionRewindException.java b/src/main/java/com/google/devtools/build/lib/skyframe/rewinding/ActionRewindException.java
@@ -0,0 +1,40 @@
+// Copyright 2024 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.rewinding;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.devtools.build.lib.server.FailureDetails.ActionRewinding;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.skyframe.DetailedException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+
+/** Exception thrown by {@link ActionRewindStrategy} when it cannot compute a rewind plan. */
+public final class ActionRewindException extends Exception implements DetailedException {
+  private final ActionRewinding.Code code;
+
+  ActionRewindException(String message, Exception cause, ActionRewinding.Code code) {
+    super(message, cause);
+    this.code = checkNotNull(code);
+  }
+
+  @Override
+  public DetailedExitCode getDetailedExitCode() {
+    return DetailedExitCode.of(
+        FailureDetail.newBuilder()
+            .setMessage(getMessage())
+            .setActionRewinding(ActionRewinding.newBuilder().setCode(code))
+            .build());
+  }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/rewinding/ActionRewindStrategy.java b/src/main/java/com/google/devtools/build/lib/skyframe/rewinding/ActionRewindStrategy.java
@@ -38,7 +38,6 @@
 import com.google.common.graph.MutableGraph;
 import com.google.common.graph.Traverser;
 import com.google.devtools.build.lib.actions.Action;
-import com.google.devtools.build.lib.actions.ActionExecutionException;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputDepOwners;
 import com.google.devtools.build.lib.actions.ActionLookupData;
@@ -49,13 +48,10 @@
 import com.google.devtools.build.lib.collect.nestedset.ArtifactNestedSetKey;
 import com.google.devtools.build.lib.events.ExtendedEventHandler;
 import com.google.devtools.build.lib.server.FailureDetails.ActionRewinding;
-import com.google.devtools.build.lib.server.FailureDetails.ActionRewinding.Code;
-import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
 import com.google.devtools.build.lib.skyframe.ActionUtils;
 import com.google.devtools.build.lib.skyframe.ArtifactFunction.ArtifactDependencies;
 import com.google.devtools.build.lib.skyframe.SkyframeAwareAction;
 import com.google.devtools.build.lib.skyframe.proto.ActionRewind.ActionRewindEvent;
-import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.build.skyframe.SkyFunction.Environment;
 import com.google.devtools.build.skyframe.SkyFunction.Reset;
 import com.google.devtools.build.skyframe.SkyKey;
@@ -108,8 +104,8 @@ public ActionRewindStrategy(BugReporter bugReporter) {
    * nodes corresponding to the actions which create the lost inputs, inclusive, must be included in
    * reevaluate the nodes that will recreate the lost inputs.
    *
-   * @throws ActionExecutionException if any lost inputs have been seen by this action as lost
-   *     before, or if any lost inputs are not the outputs of previously executed actions
+   * @throws ActionRewindException if any lost inputs have been seen by this action as lost before
+   *     too many times
    */
   public RewindPlan getRewindPlan(
       ActionLookupData failedKey,
@@ -118,7 +114,7 @@ public RewindPlan getRewindPlan(
       LostInputsActionExecutionException lostInputsException,
       ActionInputDepOwners inputDepOwners,
       Environment env)
-      throws ActionExecutionException, InterruptedException {
+      throws ActionRewindException, InterruptedException {
     ImmutableList<LostInputRecord> lostInputRecordsThisAction =
         checkIfActionLostInputTooManyTimes(failedKey, failedAction, lostInputsException);
 
@@ -243,7 +239,7 @@ private ImmutableList<LostInputRecord> checkIfActionLostInputTooManyTimes(
       ActionLookupData failedKey,
       Action failedAction,
       LostInputsActionExecutionException lostInputsException)
-      throws ActionExecutionException {
+      throws ActionRewindException {
     ImmutableMap<String, ActionInput> lostInputsByDigest = lostInputsException.getLostInputs();
     ImmutableList.Builder<LostInputRecord> lostInputRecordsThisAction = ImmutableList.builder();
     for (Map.Entry<String, ActionInput> entry : lostInputsByDigest.entrySet()) {
@@ -271,9 +267,11 @@ private ImmutableList<LostInputRecord> checkIfActionLostInputTooManyTimes(
                 "lost input too many times (#%s) for the same action. lostInput: %s, "
                     + "lostInput digest: %s, failedAction: %.10000s",
                 priorLosses + 1, lostInputsByDigest.get(digest), digest, failedAction);
-        bugReporter.sendBugReport(new IllegalStateException(message));
-        throw createActionExecutionException(
-            lostInputsException, failedAction, message, Code.LOST_INPUT_TOO_MANY_TIMES);
+        ActionRewindException e =
+            new ActionRewindException(
+                message, lostInputsException, ActionRewinding.Code.LOST_INPUT_TOO_MANY_TIMES);
+        bugReporter.sendBugReport(e);
+        throw e;
       } else if (0 < priorLosses) {
         logger.atInfo().log(
             "lost input again (#%s) for the same action. lostInput: %s, "
@@ -611,22 +609,6 @@ private static void assertSkyframeAwareRewindingGraph(
     }
   }
 
-  private static ActionExecutionException createActionExecutionException(
-      LostInputsActionExecutionException lostInputsException,
-      Action failedAction,
-      String message,
-      Code detailedCode) {
-    return new ActionExecutionException(
-        lostInputsException,
-        failedAction,
-        /*catastrophe=*/ false,
-        DetailedExitCode.of(
-            FailureDetail.newBuilder()
-                .setMessage(message)
-                .setActionRewinding(ActionRewinding.newBuilder().setCode(detailedCode))
-                .build()));
-  }
-
   /**
    * Wraps a {@link Reset} and a list of actions that need to be reported to {@link
    * com.google.devtools.build.lib.skyframe.SkyframeActionExecutor} because they will be rewound.

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/rewinding/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/rewinding/BUILD
@@ -23,6 +23,7 @@ java_library(
 java_library(
     name = "rewinding",
     srcs = [
+        "ActionRewindException.java",
         "ActionRewindStrategy.java",
         "ActionRewindingStats.java",
     ],
@@ -35,6 +36,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/skyframe:action_utils",
         "//src/main/java/com/google/devtools/build/lib/skyframe:artifact_function",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
         "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_aware_action",
         "//src/main/java/com/google/devtools/build/lib/skyframe/proto:action_rewind_event_java_proto",
         "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",