Make action rewinding compatible with --use_action_cache.

When preparing an action for rewinding, evict it from the action cache to ensure that it is executed. PiperOrigin-RevId: 574158458 Change-Id: I82075f9a1a79cdd601ac6e0d2b3c17969dae81ca
bazelbuild · Oct 17, 2023 · 0a2aac4 · 0a2aac4
1 parent 05bea52
commit 0a2aac4
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 17 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
@@ -149,7 +149,8 @@ private ActionCache.Entry getCacheEntry(Action action) {
     return ActionCacheUtils.getCacheEntry(actionCache, action);
   }
 
-  private void removeCacheEntry(Action action) {
+  public void removeCacheEntry(Action action) {
+    checkState(enabled(), "Action cache disabled");
     ActionCacheUtils.removeCacheEntry(actionCache, action);
   }
 

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -317,16 +317,6 @@ private boolean rewindingEnabled(OptionsProvider options) throws AbruptExitExcep
     if (buildRequestOptions == null || !buildRequestOptions.rewindLostInputs) {
       return false;
     }
-    if (buildRequestOptions.useActionCache) {
-      throw new AbruptExitException(
-          DetailedExitCode.of(
-              FailureDetail.newBuilder()
-                  .setMessage("--rewind_lost_inputs requires --nouse_action_cache")
-                  .setActionRewinding(
-                      ActionRewinding.newBuilder()
-                          .setCode(ActionRewinding.Code.REWIND_LOST_INPUTS_PREREQ_UNMET))
-                  .build()));
-    }
     if (isMergedSkyframeAnalysisExecution()) {
       throw new AbruptExitException(
           DetailedExitCode.of(

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
@@ -464,6 +464,10 @@ private void prepareDepForRewinding(ActionLookupData failedKey, Action dep) {
     if (!actionFileSystemType().inMemoryFileSystem()) {
       outputDirectoryHelper.invalidateTreeArtifactDirectoryCreation(dep.getOutputs());
     }
+    // Evict the rewinding action from the action cache to ensure that it is executed.
+    if (actionCacheChecker.enabled()) {
+      actionCacheChecker.removeCacheEntry(dep);
+    }
   }
 
   @Nullable

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java
@@ -2664,14 +2664,11 @@ public NestedSet<Artifact> discoverInputs(ActionExecutionContext actionExecution
 
   @Test
   public void rewindingPrerequisites(
-      @TestParameter boolean trackIncrementalState,
-      @TestParameter boolean useActionCache,
-      @TestParameter boolean skymeldEnabled)
+      @TestParameter boolean trackIncrementalState, @TestParameter boolean skymeldEnabled)
       throws Exception {
     initializeSkyframeExecutor();
     options.parse(
         "--rewind_lost_inputs",
-        "--use_action_cache=" + useActionCache,
         "--experimental_merged_skyframe_analysis_execution=" + skymeldEnabled);
     skyframeExecutor.setMergedSkyframeAnalysisExecutionSupplier(() -> skymeldEnabled);
 
@@ -2685,7 +2682,7 @@ public void rewindingPrerequisites(
         reporter);
     skyframeExecutor.setActive(true);
 
-    if (useActionCache || skymeldEnabled) {
+    if (skymeldEnabled) {
       AbruptExitException e = assertThrows(AbruptExitException.class, this::syncSkyframeExecutor);
       assertThat(e.getDetailedExitCode().getFailureDetail().getActionRewinding().getCode())
           .isEqualTo(ActionRewinding.Code.REWIND_LOST_INPUTS_PREREQ_UNMET);

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java
@@ -71,7 +71,6 @@ protected void setupOptions() throws Exception {
     super.setupOptions();
     addOptions(
         "--spawn_strategy=standalone",
-        "--nouse_action_cache",
         "--noexperimental_merged_skyframe_analysis_execution",
         "--rewind_lost_inputs",
         "--features=cc_include_scanning",