Skip to content

Commit

Permalink
Exit with code 39 if remote cache evicted blobs that Bazel need durin…
Browse files Browse the repository at this point in the history
…g an invocation

Part of #16660.

Closes #17358.

PiperOrigin-RevId: 509494072
Change-Id: Id6944da5d9a556dc9154fcb702948586b474875e
  • Loading branch information
coeuvre authored and hvadehra committed Feb 14, 2023
1 parent 7c9da98 commit 7c02dca
Show file tree
Hide file tree
Showing 12 changed files with 135 additions and 31 deletions.
1 change: 1 addition & 0 deletions site/en/run/scripts.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ interface SpawnExecutionContext {
* @see #prefetchInputs()
*/
default void prefetchInputsAndWait()
throws IOException, InterruptedException, ForbiddenActionInputException {
throws IOException, ExecException, InterruptedException, ForbiddenActionInputException {
ListenableFuture<Void> future = prefetchInputs();
try (SilentCloseable s =
Profiler.instance().profile(ProfilerTask.REMOTE_DOWNLOAD, "stage remote inputs")) {
Expand All @@ -170,6 +170,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);
}
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/remote/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/remote/util",
"//src/main/java/com/google/devtools/build/lib/remote/zstd",
"//src/main/java/com/google/devtools/build/lib/sandbox:sandbox_helpers",
"//src/main/java/com/google/devtools/build/lib/skyframe:mutable_supplier",
"//src/main/java/com/google/devtools/build/lib/skyframe:tree_artifact_value",
"//src/main/java/com/google/devtools/build/lib/util",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,18 @@
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.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.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.OutputPermissions;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
Expand All @@ -47,6 +50,7 @@ class RemoteActionInputFetcher extends AbstractActionInputPrefetcher {
private final String buildRequestId;
private final String commandId;
private final RemoteCache remoteCache;
private final boolean useNewExitCodeForLostInputs;

RemoteActionInputFetcher(
Reporter reporter,
Expand All @@ -56,11 +60,13 @@ class RemoteActionInputFetcher extends AbstractActionInputPrefetcher {
Path execRoot,
TempPathGenerator tempPathGenerator,
ImmutableList<Pattern> patternsToDownload,
OutputPermissions outputPermissions) {
OutputPermissions outputPermissions,
boolean useNewExitCodeForLostInputs) {
super(reporter, execRoot, tempPathGenerator, patternsToDownload, outputPermissions);
this.buildRequestId = Preconditions.checkNotNull(buildRequestId);
this.commandId = Preconditions.checkNotNull(commandId);
this.remoteCache = Preconditions.checkNotNull(remoteCache);
this.useNewExitCodeForLostInputs = useNewExitCodeForLostInputs;
}

@Override
Expand Down Expand Up @@ -96,19 +102,18 @@ protected ListenableFuture<Void> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,8 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB
env.getExecRoot(),
tempPathGenerator,
patternsToDownload,
outputPermissions);
outputPermissions,
remoteOptions.useNewExitCodeForLostInputs);
env.getEventBus().register(actionInputFetcher);
builder.setActionInputPrefetcher(actionInputFetcher);
remoteOutputService.setActionInputFetcher(actionInputFetcher);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions src/main/protobuf/failure_details.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -160,7 +160,8 @@ protected Pair<SpecialArtifact, ImmutableList<TreeFileArtifact>> createRemoteTre
protected abstract AbstractActionInputPrefetcher createPrefetcher(Map<HashCode, byte[]> cas);

@Test
public void prefetchFiles_fileExists_doNotDownload() throws IOException, InterruptedException {
public void prefetchFiles_fileExists_doNotDownload()
throws IOException, ExecException, InterruptedException {
Map<ActionInput, FileArtifactValue> metadata = new HashMap<>();
Map<HashCode, byte[]> cas = new HashMap<>();
Artifact a = createRemoteArtifact("file", "hello world", metadata, cas);
Expand All @@ -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<ActionInput, FileArtifactValue> metadata = new HashMap<>();
Map<HashCode, byte[]> cas = new HashMap<>();
Artifact a = createRemoteArtifact("file", "hello world remote", metadata, cas);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
}
});
Expand All @@ -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
}
});
Expand Down Expand Up @@ -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
}
});
Expand All @@ -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
}
});
Expand Down Expand Up @@ -489,13 +490,14 @@ public void downloadFile_onInterrupt_deletePartialDownloadedFile() throws Except
}

protected static void wait(ListenableFuture<Void> 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);
}
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/remote/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand All @@ -79,7 +81,6 @@ protected ImmutableList<BlazeModule> getSpawnModules() {
.addAll(super.getSpawnModules())
.add(new StandaloneModule())
.add(new CredentialModule())
.add(new RemoteModule())
.add(new DynamicExecutionModule())
.build();
}
Expand Down Expand Up @@ -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);
}
}
Loading

0 comments on commit 7c02dca

Please sign in to comment.