From e13958cd7545409cc444148707db3d104e82ef3f Mon Sep 17 00:00:00 2001 From: jhorvitz Date: Mon, 25 Apr 2022 08:55:43 -0700 Subject: [PATCH] Add a flag `--rewind_lost_inputs` to toggle action rewinding. The flag value is ignored unless other rewinding prerequisites are met. PiperOrigin-RevId: 444281575 --- .../lib/buildtool/BuildRequestOptions.java | 10 ++++++++++ .../lib/skyframe/SequencedSkyframeExecutor.java | 17 +++++++++++------ .../skyframe/SequencedSkyframeExecutorTest.java | 13 ++++++++----- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java index 1f71794edbdcb3..9d1d087b4eefee 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java @@ -420,6 +420,16 @@ public boolean useTopLevelTargetsForSymlinks() { help = "Whether to store output metadata in the action cache") public boolean actionCacheStoreOutputMetadata; + @Option( + name = "rewind_lost_inputs", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.EXECUTION}, + help = + "Whether to use action rewinding to recover from lost inputs. Ignored unless" + + " prerequisites for rewinding are met (no incrementality, no action cache).") + public boolean rewindLostInputs; + @Option( name = "discard_actions_after_execution", defaultValue = "true", 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 index 27a46b17cd48d6..e1740db5cbd7ea 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java @@ -273,11 +273,10 @@ public WorkspaceInfoFromDiff sync( OptionsProvider options) throws InterruptedException, AbruptExitException { if (evaluatorNeedsReset) { - // Rewinding is only supported with no incremental state and no action cache. inconsistencyReceiver = - trackIncrementalState || useActionCache(options) - ? GraphInconsistencyReceiver.THROWING - : new RewindableGraphInconsistencyReceiver(); + rewindingPermitted(options) + ? new RewindableGraphInconsistencyReceiver() + : GraphInconsistencyReceiver.THROWING; // Recreate MemoizingEvaluator so that graph is recreated with correct edge-clearing status, // or if the graph doesn't have edges, so that a fresh graph can be used. resetEvaluator(); @@ -300,9 +299,15 @@ public WorkspaceInfoFromDiff sync( return workspaceInfo; } - private static boolean useActionCache(OptionsProvider options) { + private boolean rewindingPermitted(OptionsProvider options) { + // Rewinding is only supported with no incremental state and no action cache. + if (trackIncrementalState) { + return false; + } BuildRequestOptions buildRequestOptions = options.getOptions(BuildRequestOptions.class); - return buildRequestOptions != null && buildRequestOptions.useActionCache; + return buildRequestOptions != null + && !buildRequestOptions.useActionCache + && buildRequestOptions.rewindLostInputs; } /** 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 index 611c13916ad79c..ff6567a0888e68 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java @@ -2478,20 +2478,23 @@ public NestedSet discoverInputs(ActionExecutionContext actionExecution @Test public void usesCorrectGraphInconsistencyReceiver( - @TestParameter boolean trackIncrementalState, @TestParameter boolean useActionCache) + @TestParameter boolean trackIncrementalState, + @TestParameter boolean useActionCache, + @TestParameter boolean rewindLostInputs) throws Exception { extraSkyFunctions.put( SkyFunctionName.FOR_TESTING, (key, env) -> { - if (trackIncrementalState || useActionCache) { - assertThat(env.restartPermitted()).isFalse(); - } else { + if (!trackIncrementalState && !useActionCache && rewindLostInputs) { assertThat(env.restartPermitted()).isTrue(); + } else { + assertThat(env.restartPermitted()).isFalse(); } return new SkyValue() {}; }); initializeSkyframeExecutor(); - options.parse("--use_action_cache=" + useActionCache); + options.parse( + "--use_action_cache=" + useActionCache, "--rewind_lost_inputs=" + rewindLostInputs); skyframeExecutor.setActive(false); skyframeExecutor.decideKeepIncrementalState(