From 3142ba564bd44e84f4be31432312065a041d2c77 Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Tue, 14 Feb 2023 05:01:31 -0800 Subject: [PATCH] Exit with code 39 if remote cache evicted blobs that Bazel need during an invocation Part of #16660. Closes #17358. PiperOrigin-RevId: 509494072 Change-Id: Id6944da5d9a556dc9154fcb702948586b474875e --- site/en/run/scripts.md | 1 + .../devtools/build/lib/exec/SpawnRunner.java | 3 +- .../lib/remote/RemoteActionInputFetcher.java | 35 ++++++------ .../build/lib/remote/RemoteModule.java | 3 +- .../lib/remote/options/RemoteOptions.java | 11 ++++ .../devtools/build/lib/util/ExitCode.java | 2 + src/main/protobuf/failure_details.proto | 1 + .../remote/ActionInputPrefetcherTestBase.java | 20 +++---- .../google/devtools/build/lib/remote/BUILD | 1 + .../BuildWithoutTheBytesIntegrationTest.java | 53 ++++++++++++++++++- .../remote/RemoteActionInputFetcherTest.java | 35 ++++++++++-- 11 files changed, 135 insertions(+), 30 deletions(-) diff --git a/site/en/run/scripts.md b/site/en/run/scripts.md index d324b407fea357..6ba0547a2fe2e5 100644 --- a/site/en/run/scripts.md +++ b/site/en/run/scripts.md @@ -55,6 +55,7 @@ Bazel execution can result in following exit codes: - `36` - Local Environmental Issue, suspected permanent. - `37` - Unhandled Exception / Internal Bazel Error. - `38` - Reserved for Google-internal use. +- `39` - Blobs required by Bazel are evicted from Remote Cache. - `41-44` - Reserved for Google-internal use. - `45` - Error publishing results to the Build Event Service. - `47` - Reserved for Google-internal use. diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java index ccfc3c8146043a..15724b0552d8b2 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java @@ -162,7 +162,7 @@ interface SpawnExecutionContext { * @see #prefetchInputs() */ default void prefetchInputsAndWait() - throws IOException, InterruptedException, ForbiddenActionInputException { + throws IOException, ExecException, InterruptedException, ForbiddenActionInputException { ListenableFuture future = prefetchInputs(); try (SilentCloseable s = Profiler.instance().profile(ProfilerTask.REMOTE_DOWNLOAD, "stage remote inputs")) { @@ -171,6 +171,7 @@ default void prefetchInputsAndWait() Throwable cause = e.getCause(); if (cause != null) { throwIfInstanceOf(cause, IOException.class); + throwIfInstanceOf(cause, ExecException.class); throwIfInstanceOf(cause, ForbiddenActionInputException.class); throwIfInstanceOf(cause, RuntimeException.class); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java index 696fa80394c225..20c739f4499bdd 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java @@ -20,17 +20,20 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.ListenableFuture; +import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput; import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.remote.common.BulkTransferException; -import com.google.devtools.build.lib.remote.common.CacheNotFoundException; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TempPathGenerator; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; 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.server.FailureDetails.Spawn.Code; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import io.reactivex.rxjava3.core.Completable; @@ -48,6 +51,7 @@ class RemoteActionInputFetcher extends AbstractActionInputPrefetcher { private final String buildRequestId; private final String commandId; private final RemoteCache remoteCache; + private final boolean useNewExitCodeForLostInputs; RemoteActionInputFetcher( Reporter reporter, @@ -56,11 +60,13 @@ class RemoteActionInputFetcher extends AbstractActionInputPrefetcher { RemoteCache remoteCache, Path execRoot, TempPathGenerator tempPathGenerator, - ImmutableList patternsToDownload) { + ImmutableList patternsToDownload, + boolean useNewExitCodeForLostInputs) { super(reporter, execRoot, tempPathGenerator, patternsToDownload); this.buildRequestId = Preconditions.checkNotNull(buildRequestId); this.commandId = Preconditions.checkNotNull(commandId); this.remoteCache = Preconditions.checkNotNull(remoteCache); + this.useNewExitCodeForLostInputs = useNewExitCodeForLostInputs; } @Override @@ -99,19 +105,18 @@ protected ListenableFuture doDownloadFile( protected Completable onErrorResumeNext(Throwable error) { if (error instanceof BulkTransferException) { if (((BulkTransferException) error).onlyCausedByCacheNotFoundException()) { - BulkTransferException bulkAnnotatedException = new BulkTransferException(); - for (Throwable t : error.getSuppressed()) { - IOException annotatedException = - new IOException( - String.format( - "Failed to fetch file with hash '%s' because it does not" - + " exist remotely. --remote_download_outputs=minimal" - + " does not work if your remote cache evicts files" - + " during builds.", - ((CacheNotFoundException) t).getMissingDigest().getHash())); - bulkAnnotatedException.add(annotatedException); - } - error = bulkAnnotatedException; + var code = + useNewExitCodeForLostInputs ? Code.REMOTE_CACHE_EVICTED : Code.REMOTE_CACHE_FAILED; + error = + new EnvironmentalExecException( + (BulkTransferException) error, + FailureDetail.newBuilder() + .setMessage( + "Failed to fetch blobs because they do not exist remotely." + + " Build without the Bytes does not work if your remote" + + " cache evicts blobs during builds") + .setSpawn(FailureDetails.Spawn.newBuilder().setCode(code)) + .build()); } } return Completable.error(error); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index 46d7a183193fc2..701275a3e6de46 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java @@ -917,7 +917,8 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB actionContextProvider.getRemoteCache(), env.getExecRoot(), tempPathGenerator, - patternsToDownload); + patternsToDownload, + remoteOptions.useNewExitCodeForLostInputs); env.getEventBus().register(actionInputFetcher); builder.setActionInputPrefetcher(actionInputFetcher); remoteOutputService.setActionInputFetcher(actionInputFetcher); diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index c812a423aa3f77..ba8488382cb3b1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -658,6 +658,17 @@ public RemoteOutputsStrategyConverter() { + "cache misses and retries.") public boolean remoteDiscardMerkleTrees; + @Option( + name = "incompatible_remote_use_new_exit_code_for_lost_inputs", + defaultValue = "false", + 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; + // The below options are not configurable by users, only tests. // This is part of the effort to reduce the overall number of flags. diff --git a/src/main/java/com/google/devtools/build/lib/util/ExitCode.java b/src/main/java/com/google/devtools/build/lib/util/ExitCode.java index f91fffa8ca1ab0..24d51c542379b9 100644 --- a/src/main/java/com/google/devtools/build/lib/util/ExitCode.java +++ b/src/main/java/com/google/devtools/build/lib/util/ExitCode.java @@ -65,6 +65,8 @@ public final class ExitCode { ExitCode.createInfrastructureFailure(37, "BLAZE_INTERNAL_ERROR"); public static final ExitCode TRANSIENT_BUILD_EVENT_SERVICE_UPLOAD_ERROR = ExitCode.createInfrastructureFailure(38, "PUBLISH_ERROR"); + public static final ExitCode REMOTE_CACHE_EVICTED = + ExitCode.createInfrastructureFailure(39, "REMOTE_CACHE_EVICTED"); public static final ExitCode PERSISTENT_BUILD_EVENT_SERVICE_UPLOAD_ERROR = ExitCode.create(45, "PERSISTENT_BUILD_EVENT_SERVICE_UPLOAD_ERROR"); public static final ExitCode EXTERNAL_DEPS_ERROR = ExitCode.create(48, "EXTERNAL_DEPS_ERROR"); diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index ea0873c3ea9a12..99c919d2e4cdf4 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -226,6 +226,7 @@ message Spawn { // refactored to prohibit undetailed failures UNSPECIFIED_EXECUTION_FAILURE = 12 [(metadata) = { exit_code: 1 }]; FORBIDDEN_INPUT = 13 [(metadata) = { exit_code: 1 }]; + REMOTE_CACHE_EVICTED = 14 [(metadata) = { exit_code: 39 }]; } Code code = 1; diff --git a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java index 2b0124d61af773..912c269d6e9a3f 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java @@ -37,12 +37,12 @@ import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.ArtifactRoot.RootType; +import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.clock.JavaClock; -import com.google.devtools.build.lib.remote.common.BulkTransferException; import com.google.devtools.build.lib.remote.util.StaticMetadataProvider; import com.google.devtools.build.lib.remote.util.TempPathGenerator; import com.google.devtools.build.lib.skyframe.TreeArtifactValue; @@ -160,7 +160,8 @@ protected Pair> createRemoteTre protected abstract AbstractActionInputPrefetcher createPrefetcher(Map cas); @Test - public void prefetchFiles_fileExists_doNotDownload() throws IOException, InterruptedException { + public void prefetchFiles_fileExists_doNotDownload() + throws IOException, ExecException, InterruptedException { Map metadata = new HashMap<>(); Map cas = new HashMap<>(); Artifact a = createRemoteArtifact("file", "hello world", metadata, cas); @@ -177,7 +178,7 @@ public void prefetchFiles_fileExists_doNotDownload() throws IOException, Interru @Test public void prefetchFiles_fileExistsButContentMismatches_download() - throws IOException, InterruptedException { + throws IOException, ExecException, InterruptedException { Map metadata = new HashMap<>(); Map cas = new HashMap<>(); Artifact a = createRemoteArtifact("file", "hello world remote", metadata, cas); @@ -304,7 +305,7 @@ public void prefetchFiles_missingFiles_fails() throws Exception { AbstractActionInputPrefetcher prefetcher = createPrefetcher(new HashMap<>()); assertThrows( - BulkTransferException.class, + Exception.class, () -> wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadataProvider))); assertThat(prefetcher.downloadedFiles()).isEmpty(); @@ -347,7 +348,7 @@ public void prefetchFiles_multipleThreads_downloadIsCancelled() throws Exception () -> { try { wait(prefetcher.prefetchFiles(ImmutableList.of(artifact), metadataProvider)); - } catch (IOException | InterruptedException ignored) { + } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } }); @@ -357,7 +358,7 @@ public void prefetchFiles_multipleThreads_downloadIsCancelled() throws Exception () -> { try { wait(prefetcher.prefetchFiles(ImmutableList.of(artifact), metadataProvider)); - } catch (IOException | InterruptedException ignored) { + } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } }); @@ -394,7 +395,7 @@ public void prefetchFiles_multipleThreads_downloadIsNotCancelledByOtherThreads() () -> { try { wait(prefetcher.prefetchFiles(ImmutableList.of(artifact), metadataProvider)); - } catch (IOException | InterruptedException ignored) { + } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } }); @@ -406,7 +407,7 @@ public void prefetchFiles_multipleThreads_downloadIsNotCancelledByOtherThreads() try { wait(prefetcher.prefetchFiles(ImmutableList.of(artifact), metadataProvider)); successful.set(true); - } catch (IOException | InterruptedException ignored) { + } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } }); @@ -489,13 +490,14 @@ public void downloadFile_onInterrupt_deletePartialDownloadedFile() throws Except } protected static void wait(ListenableFuture future) - throws IOException, InterruptedException { + throws IOException, ExecException, InterruptedException { try { future.get(); } catch (ExecutionException e) { Throwable cause = e.getCause(); if (cause != null) { throwIfInstanceOf(cause, IOException.class); + throwIfInstanceOf(cause, ExecException.class); throwIfInstanceOf(cause, InterruptedException.class); throwIfInstanceOf(cause, RuntimeException.class); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index f438d4dce01bc2..0c838a094b745b 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -161,6 +161,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/remote", "//src/main/java/com/google/devtools/build/lib/standalone", "//src/main/java/com/google/devtools/build/lib/util:os", + "//src/main/java/com/google/devtools/build/lib/vfs", "//src/test/java/com/google/devtools/build/lib/remote/util:integration_test_utils", "//third_party:guava", "//third_party:junit4", 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 8c49c36b9c7d6f..3709b2c9003300 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 @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.runtime.BuildSummaryStatsModule; import com.google.devtools.build.lib.standalone.StandaloneModule; import com.google.devtools.build.lib.util.OS; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import java.io.IOException; import org.junit.After; import org.junit.Test; @@ -69,6 +70,7 @@ protected void setDownloadAll() { @Override protected BlazeRuntime.Builder getRuntimeBuilder() throws Exception { return super.getRuntimeBuilder() + .addBlazeModule(new RemoteModule()) .addBlazeModule(new BuildSummaryStatsModule()) .addBlazeModule(new BlockWaitingModule()); } @@ -79,7 +81,6 @@ protected ImmutableList getSpawnModules() { .addAll(super.getSpawnModules()) .add(new StandaloneModule()) .add(new CredentialModule()) - .add(new RemoteModule()) .add(new DynamicExecutionModule()) .build(); } @@ -415,4 +416,54 @@ public void symlinkToNestedDirectory() throws Exception { buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote"); } + + @Test + public void remoteCacheEvictBlobs_exitWithCode39() throws Exception { + // Arrange: Prepare workspace and populate remote cache + write( + "a/BUILD", + "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'],", + ")"); + write("a/foo.in", "foo"); + write("a/bar.in", "bar"); + + // Populate remote cache + buildTarget("//a:bar"); + var bytes = FileSystemUtils.readContent(getOutputPath("a/foo.out")); + var hashCode = getDigestHashFunction().getHashFunction().hashBytes(bytes); + getOutputPath("a/foo.out").delete(); + getOutputPath("a/bar.out").delete(); + getOutputBase().getRelative("action_cache").deleteTreesBelow(); + restartServer(); + addOptions("--incompatible_remote_use_new_exit_code_for_lost_inputs"); + + // Clean build, foo.out isn't downloaded + buildTarget("//a:bar"); + assertOutputDoesNotExist("a/foo.out"); + + // Act: Evict blobs from remote cache and do an incremental build + getFileSystem().getPath(worker.getCasPath().getSafePathString()).deleteTreesBelow(); + write("a/bar.in", "updated bar"); + var error = assertThrows(BuildFailedException.class, () -> buildTarget("//a:bar")); + + // Assert: Exit code is 39 + assertThat(error) + .hasMessageThat() + .contains( + "Build without the Bytes does not work if your remote cache evicts blobs" + + " during builds"); + assertThat(error).hasMessageThat().contains(String.format("%s/%s", hashCode, bytes.length)); + assertThat(error.getDetailedExitCode().getExitCode().getNumericExitCode()).isEqualTo(39); + } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java index ce034b84045aa2..e82fcfede54211 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.remote; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import build.bazel.remote.execution.v2.CacheCapabilities; import build.bazel.remote.execution.v2.Digest; @@ -21,6 +22,10 @@ import com.google.common.collect.Maps; import com.google.common.eventbus.EventBus; import com.google.common.hash.HashCode; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; @@ -68,7 +73,8 @@ protected AbstractActionInputPrefetcher createPrefetcher(Map c remoteCache, execRoot, tempPathGenerator, - ImmutableList.of()); + ImmutableList.of(), + /* useNewExitCodeForLostInputs= */ false); } @Test @@ -84,7 +90,8 @@ public void testStagingVirtualActionInput() throws Exception { remoteCache, execRoot, tempPathGenerator, - ImmutableList.of()); + ImmutableList.of(), + /* useNewExitCodeForLostInputs= */ false); VirtualActionInput a = ActionsTestUtil.createVirtualActionInput("file1", "hello world"); // act @@ -111,7 +118,8 @@ public void testStagingEmptyVirtualActionInput() throws Exception { remoteCache, execRoot, tempPathGenerator, - ImmutableList.of()); + ImmutableList.of(), + /* useNewExitCodeForLostInputs= */ false); // act wait( @@ -123,6 +131,27 @@ public void testStagingEmptyVirtualActionInput() throws Exception { assertThat(actionInputFetcher.downloadsInProgress()).isEmpty(); } + @Test + public void prefetchFiles_missingFiles_failsWithSpecificMessage() throws Exception { + Map metadata = new HashMap<>(); + Artifact a = createRemoteArtifact("file1", "hello world", metadata, /* cas= */ new HashMap<>()); + MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); + AbstractActionInputPrefetcher prefetcher = createPrefetcher(new HashMap<>()); + + var error = + assertThrows( + ExecException.class, + () -> wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadataProvider))); + + assertThat(prefetcher.downloadedFiles()).isEmpty(); + assertThat(prefetcher.downloadsInProgress()).isEmpty(); + var m = metadataProvider.getMetadata(a); + var digest = DigestUtil.buildDigest(m.getDigest(), m.getSize()); + assertThat(error) + .hasMessageThat() + .contains(String.format("%s/%s", digest.getHash(), digest.getSizeBytes())); + } + private RemoteCache newCache( RemoteOptions options, DigestUtil digestUtil, Map cas) { Map cacheEntries = Maps.newHashMapWithExpectedSize(cas.size());