From 7b455eb32190705dd15c09371bde87c7d0d8a727 Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Thu, 30 Mar 2023 05:23:38 -0700 Subject: [PATCH 1/7] Automatically retry the build if encountered remote cache eviction error With https://github.com/bazelbuild/bazel/pull/17358, Bazel will exit with code 39 if remote cache evicts blobs during the build. With https://github.com/bazelbuild/bazel/pull/17462 and #17747, Bazel is able to continue the build without bazel clean or bazel shutdown. However, even with https://github.com/bazelbuild/bazel/pull/17639 and following changes to extend the lease, remote cache can still evict blobs in some rare cases. Based on above changes, this PR makes bazel retry the invocation if it encountered the remote cache eviction error during previous invocation if `--experimental_remote_cache_eviction_retries` is set, or **build rewinding**. ``` $ bazel build --experimental_remote_cache_eviction_retries=5 ... INFO: Invocation ID: b7348bfa-9446-4c72-a888-0a0ad012f225 Loading: Loading: Loading: 0 packages loaded Analyzing: target //a:bar (0 packages loaded, 0 targets configured) INFO: Analyzed target //a:bar (0 packages loaded, 0 targets configured). INFO: Found 1 target... [0 / 2] [Prepa] BazelWorkspaceStatusAction stable-status.txt ERROR: .../workspace/a/BUILD:8:8: Executing genrule //a:bar failed: Failed to fetch blobs because they do not exist remotely: Missing digest: b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c/4 Target //a:bar failed to build Use --verbose_failures to see the command lines of failed build steps. INFO: Elapsed time: 0.447s, Critical Path: 0.05s INFO: 2 processes: 2 internal. ERROR: Build did NOT complete successfully Found remote cache eviction error, retrying the build... INFO: Invocation ID: 983f60dc-8bb9-4b82-aa33-a378469ce140 Loading: Loading: Loading: 0 packages loaded Analyzing: target //a:bar (0 packages loaded, 0 targets configured) INFO: Analyzed target //a:bar (0 packages loaded, 0 targets configured). INFO: Found 1 target... [0 / 2] [Prepa] BazelWorkspaceStatusAction stable-status.txt Target //a:bar up-to-date: bazel-bin/a/bar.out INFO: Elapsed time: 0.866s, Critical Path: 0.35s INFO: 3 processes: 1 internal, 1 processwrapper-sandbox, 1 remote. INFO: Build completed successfully, 3 total actions $ ``` Part of https://github.com/bazelbuild/bazel/pull/16660. Closes #17711. PiperOrigin-RevId: 520610524 Change-Id: I20d43d1968767a03250b9c8f8a6dda4e056d4f52 --- .../build/lib/exec/AbstractSpawnStrategy.java | 33 +++++++++-- .../build/lib/exec/ExecutionOptions.java | 22 +++++++ .../build/lib/remote/RemoteSpawnRunner.java | 3 +- .../lib/runtime/BlazeCommandDispatcher.java | 56 +++++++++++++----- .../BuildWithoutTheBytesIntegrationTest.java | 4 +- .../remote/build_without_the_bytes_test.sh | 59 +++++++++++++++++++ 6 files changed, 153 insertions(+), 24 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index 01ea75faba166f..3298d65c5edc8b 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -243,12 +243,33 @@ public int getId() { public ListenableFuture prefetchInputs() throws IOException, ForbiddenActionInputException { if (Spawns.shouldPrefetchInputsForLocalExecution(spawn)) { - return actionExecutionContext - .getActionInputPrefetcher() - .prefetchFiles( - getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true) - .values(), - getMetadataProvider()); + return Futures.catchingAsync( + actionExecutionContext + .getActionInputPrefetcher() + .prefetchFiles( + getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true) + .values(), + getMetadataProvider(), + Priority.MEDIUM), + BulkTransferException.class, + (BulkTransferException e) -> { + if (BulkTransferException.allCausedByCacheNotFoundException(e)) { + var code = + (executionOptions.useNewExitCodeForLostInputs + || executionOptions.remoteRetryOnCacheEviction > 0) + ? Code.REMOTE_CACHE_EVICTED + : Code.REMOTE_CACHE_FAILED; + throw new EnvironmentalExecException( + e, + FailureDetail.newBuilder() + .setMessage("Failed to fetch blobs because they do not exist remotely.") + .setSpawn(FailureDetails.Spawn.newBuilder().setCode(code)) + .build()); + } else { + throw e; + } + }, + directExecutor()); } return immediateVoidFuture(); diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java index f964d876a7e0c6..8811254af822e1 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java @@ -494,6 +494,28 @@ public boolean usingLocalTestJobs() { + "test log. Otherwise, Bazel generates a test.xml as part of the test action.") public boolean splitXmlGeneration; + @Option( + name = "incompatible_remote_use_new_exit_code_for_lost_inputs", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = + "If set to true, Bazel will use new exit code 39 instead of 34 if remote cache evicts" + + " blobs during the build.") + public boolean useNewExitCodeForLostInputs; + + @Option( + name = "experimental_remote_cache_eviction_retries", + defaultValue = "0", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.EXECUTION}, + help = + "The maximum number of attempts to retry if the build encountered remote cache eviction" + + " error. A non-zero value will implicitly set" + + " --incompatible_remote_use_new_exit_code_for_lost_inputs to true.") + public int remoteRetryOnCacheEviction; + /** An enum for specifying different formats of test output. */ public enum TestOutputFormat { SUMMARY, // Provide summary output only. diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 0f7bb077577efa..1b0baa3808c395 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -557,7 +557,8 @@ private SpawnResult handleError( catastrophe = true; } else if (remoteCacheFailed) { status = Status.REMOTE_CACHE_FAILED; - if (remoteOptions.useNewExitCodeForLostInputs) { + if (executionOptions.useNewExitCodeForLostInputs + || executionOptions.remoteRetryOnCacheEviction > 0) { detailedCode = FailureDetails.Spawn.Code.REMOTE_CACHE_EVICTED; } else { detailedCode = FailureDetails.Spawn.Code.REMOTE_CACHE_FAILED; diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java index 0fbd5a2d3f0327..b91d69e7384ac3 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java @@ -43,6 +43,7 @@ import com.google.devtools.build.lib.events.PrintingEventHandler; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.profiler.MemoryProfiler; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; @@ -54,6 +55,7 @@ import com.google.devtools.build.lib.util.AnsiStrippingOutputStream; import com.google.devtools.build.lib.util.DebugLoggerConfigurator; import com.google.devtools.build.lib.util.DetailedExitCode; +import com.google.devtools.build.lib.util.ExitCode; import com.google.devtools.build.lib.util.InterruptedFailureDetails; import com.google.devtools.build.lib.util.LoggingUtil; import com.google.devtools.build.lib.util.Pair; @@ -230,18 +232,29 @@ public BlazeCommandResult exec( return createDetailedCommandResult( retrievedShutdownReason, FailureDetails.Command.Code.PREVIOUSLY_SHUTDOWN); } - BlazeCommandResult result = - execExclusively( - originalCommandLine, - invocationPolicy, - args, - outErr, - firstContactTimeMillis, - commandName, - command, - waitTimeInMs, - startupOptionsTaggedWithBazelRc, - commandExtensions); + BlazeCommandResult result; + int attempt = 0; + while (true) { + try { + result = + execExclusively( + originalCommandLine, + invocationPolicy, + args, + outErr, + firstContactTimeMillis, + commandName, + command, + waitTimeInMs, + startupOptionsTaggedWithBazelRc, + commandExtensions, + attempt); + break; + } catch (RemoteCacheEvictedException e) { + outErr.printErrLn("Found remote cache eviction error, retrying the build..."); + attempt += 1; + } + } if (result.shutdown()) { setShutdownReason( "Server shut down " @@ -289,7 +302,9 @@ private BlazeCommandResult execExclusively( BlazeCommand command, long waitTimeInMs, Optional>> startupOptionsTaggedWithBazelRc, - List commandExtensions) { + List commandExtensions, + int attempt) + throws RemoteCacheEvictedException { // Record the start time for the profiler. Do not put anything before this! long execStartTimeNanos = runtime.getClock().nanoTime(); @@ -631,7 +646,18 @@ private BlazeCommandResult execExclusively( } needToCallAfterCommand = false; - return runtime.afterCommand(env, result); + var newResult = runtime.afterCommand(env, result); + if (newResult.getExitCode().equals(ExitCode.REMOTE_CACHE_EVICTED)) { + var executionOptions = + Preconditions.checkNotNull(options.getOptions(ExecutionOptions.class)); + if (attempt < executionOptions.remoteRetryOnCacheEviction) { + throw new RemoteCacheEvictedException(); + } + } + + return newResult; + } catch (RemoteCacheEvictedException e) { + throw e; } catch (Throwable e) { logger.atSevere().withCause(e).log("Shutting down due to exception"); Crash crash = Crash.from(e); @@ -665,6 +691,8 @@ private BlazeCommandResult execExclusively( } } + private static class RemoteCacheEvictedException extends IOException {} + private static void replayEarlyExitEvents( OutErr outErr, BlazeOptionHandler optionHandler, diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java index 25b5c3cf33c0bb..f45f8086cb0434 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java @@ -466,9 +466,7 @@ public void remoteCacheEvictBlobs_whenPrefetchingInput_exitWithCode39() throws E // Assert: Exit code is 39 assertThat(error) .hasMessageThat() - .contains( - "Build without the Bytes does not work if your remote cache evicts blobs" - + " during builds"); + .contains("Failed to fetch blobs because they do not exist remotely"); assertThat(error).hasMessageThat().contains(String.format("%s/%s", hashCode, bytes.length)); assertThat(error.getDetailedExitCode().getExitCode().getNumericExitCode()).isEqualTo(39); } diff --git a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh index aa599ff9af679a..0415433c3c4d03 100755 --- a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh +++ b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh @@ -1627,4 +1627,63 @@ end_of_record" expect_log "$expected_result" } +function test_remote_cache_eviction_retries() { + mkdir -p a + + cat > a/BUILD <<'EOF' +genrule( + name = 'foo', + srcs = ['foo.in'], + outs = ['foo.out'], + cmd = 'cat $(SRCS) > $@', +) + +genrule( + name = 'bar', + srcs = ['foo.out', 'bar.in'], + outs = ['bar.out'], + cmd = 'cat $(SRCS) > $@', + tags = ['no-remote-exec'], +) +EOF + + echo foo > a/foo.in + echo bar > a/bar.in + + # Populate remote cache + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + //a:bar >& $TEST_log || fail "Failed to build" + + bazel clean + + # Clean build, foo.out isn't downloaded + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + //a:bar >& $TEST_log || fail "Failed to build" + + if [[ -f bazel-bin/a/foo.out ]]; then + fail "Expected intermediate output bazel-bin/a/foo.out to not be downloaded" + fi + + # Evict blobs from remote cache + stop_worker + start_worker + + echo "updated bar" > a/bar.in + + # Incremental build triggers remote cache eviction error but Bazel + # automatically retries the build and reruns the generating actions for + # missing blobs + bazel build \ + --remote_executor=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + --experimental_remote_cache_eviction_retries=5 \ + //a:bar >& $TEST_log || fail "Failed to build" + + expect_log "Found remote cache eviction error, retrying the build..." +} + run_suite "Build without the Bytes tests" From b100247ee593f61ba91a72fe6092619026cf6123 Mon Sep 17 00:00:00 2001 From: kshyanashree <109167932+kshyanashree@users.noreply.github.com> Date: Mon, 3 Apr 2023 00:25:46 -0700 Subject: [PATCH 2/7] Update ExecutionOptions.java --- .../devtools/build/lib/exec/ExecutionOptions.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java index 8811254af822e1..a32b9efc7e59d8 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java @@ -494,17 +494,6 @@ public boolean usingLocalTestJobs() { + "test log. Otherwise, Bazel generates a test.xml as part of the test action.") public boolean splitXmlGeneration; - @Option( - name = "incompatible_remote_use_new_exit_code_for_lost_inputs", - defaultValue = "true", - documentationCategory = OptionDocumentationCategory.REMOTE, - effectTags = {OptionEffectTag.UNKNOWN}, - metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, - help = - "If set to true, Bazel will use new exit code 39 instead of 34 if remote cache evicts" - + " blobs during the build.") - public boolean useNewExitCodeForLostInputs; - @Option( name = "experimental_remote_cache_eviction_retries", defaultValue = "0", From 05109239440f9f252c59bd55b109d821d92555f9 Mon Sep 17 00:00:00 2001 From: kshyanashree <109167932+kshyanashree@users.noreply.github.com> Date: Mon, 3 Apr 2023 01:08:28 -0700 Subject: [PATCH 3/7] Update BlazeCommandDispatcher.java --- .../lib/runtime/BlazeCommandDispatcher.java | 104 +++++++++++------- 1 file changed, 63 insertions(+), 41 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java index b91d69e7384ac3..036b244ece6c2d 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java @@ -43,7 +43,6 @@ import com.google.devtools.build.lib.events.PrintingEventHandler; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.events.StoredEventHandler; -import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.profiler.MemoryProfiler; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; @@ -150,6 +149,55 @@ public BlazeCommandResult exec( Optional>> startupOptionsTaggedWithBazelRc, List commandExtensions) throws InterruptedException { + var remoteCacheEvictionRetries = getRemoteCacheEvictionRetries(args, outErr); + while (true) { + var result = + execOnce( + invocationPolicy, + args, + outErr, + lockingMode, + clientDescription, + firstContactTimeMillis, + startupOptionsTaggedWithBazelRc, + commandExtensions); + if (result.getExitCode() == ExitCode.REMOTE_CACHE_EVICTED && remoteCacheEvictionRetries > 0) { + --remoteCacheEvictionRetries; + outErr.printErrLn("Found remote cache eviction error, retrying the build..."); + continue; + } + return result; + } + } + + private int getRemoteCacheEvictionRetries(List args, OutErr outErr) { + // Since flags are not parsed yet at this point, we manually extract value of the retry flag. + var retryFlagPrefix = "--experimental_remote_cache_eviction_retries="; + for (var arg : args) { + if (arg.startsWith(retryFlagPrefix)) { + try { + return Integer.parseInt(arg.substring(retryFlagPrefix.length())); + } catch (NumberFormatException e) { + outErr.printErrLn( + String.format( + "Failed to parse retry times: %s, remote cache eviction retry is disabled", e)); + return 0; + } + } + } + return 0; + } + + public BlazeCommandResult execOnce( + InvocationPolicy invocationPolicy, + List args, + OutErr outErr, + LockingMode lockingMode, + String clientDescription, + long firstContactTimeMillis, + Optional>> startupOptionsTaggedWithBazelRc, + List commandExtensions) + throws InterruptedException { OriginalUnstructuredCommandLineEvent originalCommandLine = new OriginalUnstructuredCommandLineEvent(args); Preconditions.checkNotNull(clientDescription); @@ -232,29 +280,18 @@ public BlazeCommandResult exec( return createDetailedCommandResult( retrievedShutdownReason, FailureDetails.Command.Code.PREVIOUSLY_SHUTDOWN); } - BlazeCommandResult result; - int attempt = 0; - while (true) { - try { - result = - execExclusively( - originalCommandLine, - invocationPolicy, - args, - outErr, - firstContactTimeMillis, - commandName, - command, - waitTimeInMs, - startupOptionsTaggedWithBazelRc, - commandExtensions, - attempt); - break; - } catch (RemoteCacheEvictedException e) { - outErr.printErrLn("Found remote cache eviction error, retrying the build..."); - attempt += 1; - } - } + BlazeCommandResult result = + execExclusively( + originalCommandLine, + invocationPolicy, + args, + outErr, + firstContactTimeMillis, + commandName, + command, + waitTimeInMs, + startupOptionsTaggedWithBazelRc, + commandExtensions); if (result.shutdown()) { setShutdownReason( "Server shut down " @@ -302,9 +339,7 @@ private BlazeCommandResult execExclusively( BlazeCommand command, long waitTimeInMs, Optional>> startupOptionsTaggedWithBazelRc, - List commandExtensions, - int attempt) - throws RemoteCacheEvictedException { + List commandExtensions) { // Record the start time for the profiler. Do not put anything before this! long execStartTimeNanos = runtime.getClock().nanoTime(); @@ -646,18 +681,7 @@ private BlazeCommandResult execExclusively( } needToCallAfterCommand = false; - var newResult = runtime.afterCommand(env, result); - if (newResult.getExitCode().equals(ExitCode.REMOTE_CACHE_EVICTED)) { - var executionOptions = - Preconditions.checkNotNull(options.getOptions(ExecutionOptions.class)); - if (attempt < executionOptions.remoteRetryOnCacheEviction) { - throw new RemoteCacheEvictedException(); - } - } - - return newResult; - } catch (RemoteCacheEvictedException e) { - throw e; + return runtime.afterCommand(env, result); } catch (Throwable e) { logger.atSevere().withCause(e).log("Shutting down due to exception"); Crash crash = Crash.from(e); @@ -691,8 +715,6 @@ private BlazeCommandResult execExclusively( } } - private static class RemoteCacheEvictedException extends IOException {} - private static void replayEarlyExitEvents( OutErr outErr, BlazeOptionHandler optionHandler, From e946129ab4b352b00ba2caade73114c441454817 Mon Sep 17 00:00:00 2001 From: kshyanashree <109167932+kshyanashree@users.noreply.github.com> Date: Mon, 3 Apr 2023 10:39:32 -0700 Subject: [PATCH 4/7] Update BlazeCommandDispatcher.java --- .../devtools/build/lib/runtime/BlazeCommandDispatcher.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java index 036b244ece6c2d..c627d4dc9fa60a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java @@ -43,6 +43,7 @@ import com.google.devtools.build.lib.events.PrintingEventHandler; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.profiler.MemoryProfiler; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.SilentCloseable; From ebddc7b9c48c6d5238b5a84a3d7e9031a728ab6c Mon Sep 17 00:00:00 2001 From: kshyanashree <109167932+kshyanashree@users.noreply.github.com> Date: Mon, 3 Apr 2023 10:51:21 -0700 Subject: [PATCH 5/7] Update ExecutionOptions.java --- .../devtools/build/lib/exec/ExecutionOptions.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java index a32b9efc7e59d8..55f669ac4bcc9d 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java @@ -493,6 +493,17 @@ public boolean usingLocalTestJobs() { + "Bazel uses a separate action to generate a dummy test.xml file containing the " + "test log. Otherwise, Bazel generates a test.xml as part of the test action.") public boolean splitXmlGeneration; + + @Option( + name = "incompatible_remote_use_new_exit_code_for_lost_inputs", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = + "If set to true, Bazel will use new exit code 39 instead of 34 if remote cache evicts" + + " blobs during the build.") + public boolean useNewExitCodeForLostInputs; @Option( name = "experimental_remote_cache_eviction_retries", From fc16ae6fa1b41bc3d2a2f2f3d9aa402cb9cc8887 Mon Sep 17 00:00:00 2001 From: kshyanashree <109167932+kshyanashree@users.noreply.github.com> Date: Mon, 3 Apr 2023 11:22:12 -0700 Subject: [PATCH 6/7] Update ExecutionOptions.java --- .../devtools/build/lib/exec/ExecutionOptions.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java index 55f669ac4bcc9d..e582cc157c5330 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java @@ -30,12 +30,14 @@ import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; +import com.google.devtools.common.options.OptionMetadataTag; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsBase; import com.google.devtools.common.options.OptionsParsingException; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; /** * Options affecting the execution phase of a build. @@ -554,16 +556,13 @@ public static class TestAttemptsConverter extends PerLabelOptions.PerLabelOption private static final int MAX_VALUE = 10; private void validateInput(String input) throws OptionsParsingException { - if ("default".equals(input)) { - return; - } else { - Integer value = Integer.parseInt(input); + if (!Objects.equals(input, "default")) { + int value = Integer.parseInt(input); if (value < MIN_VALUE) { throw new OptionsParsingException("'" + input + "' should be >= " + MIN_VALUE); - } else if (value < MIN_VALUE || value > MAX_VALUE) { + } else if (value > MAX_VALUE) { throw new OptionsParsingException("'" + input + "' should be <= " + MAX_VALUE); } - return; } } From cf006baef51eccb4c98ebd891814b0da1326b815 Mon Sep 17 00:00:00 2001 From: kshyanashree <109167932+kshyanashree@users.noreply.github.com> Date: Mon, 3 Apr 2023 12:00:39 -0700 Subject: [PATCH 7/7] Update RemoteSpawnRunner.java --- .../build/lib/remote/RemoteSpawnRunner.java | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 1b0baa3808c395..d6d3cfe6e894e8 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -30,7 +30,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; -import com.google.common.base.Throwables; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; @@ -63,7 +62,6 @@ import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.Utils; import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; -import com.google.devtools.build.lib.sandbox.SandboxHelpers; import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.util.ExitCode; @@ -254,10 +252,12 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) // subtract network time consumed here to ensure wall clock during upload is not // double // counted, and metrics time computation does not exceed total time - spawnMetrics.setUploadTime( - uploadTime - .elapsed() - .minus(action.getNetworkTime().getDuration().minus(networkTimeStart))); + spawnMetrics.setUploadTimeInMs( + (int) + uploadTime + .elapsed() + .minus(action.getNetworkTime().getDuration().minus(networkTimeStart)) + .toMillis()); } context.report(SPAWN_SCHEDULING_EVENT); @@ -462,8 +462,7 @@ private void maybeWriteParamFilesLocally(Spawn spawn) throws IOException { for (ActionInput actionInput : spawn.getInputFiles().toList()) { if (actionInput instanceof ParamFileActionInput) { ParamFileActionInput paramFileActionInput = (ParamFileActionInput) actionInput; - Path outputPath = execRoot.getRelative(paramFileActionInput.getExecPath()); - SandboxHelpers.atomicallyWriteVirtualInput(paramFileActionInput, outputPath, ".remote"); + paramFileActionInput.atomicallyWriteRelativeTo(execRoot, ".remote"); } } } @@ -570,12 +569,8 @@ private SpawnResult handleError( catastrophe = false; } - String errorMessage = Utils.grpcAwareErrorMessage(exception); - if (verboseFailures) { - // On --verbose_failures print the whole stack trace - errorMessage += "\n" + Throwables.getStackTraceAsString(exception); - } - + String errorMessage = Utils.grpcAwareErrorMessage(exception, verboseFailures); + if (exception.getCause() instanceof ExecutionStatusException) { ExecutionStatusException e = (ExecutionStatusException) exception.getCause(); if (e.getResponse() != null) {