Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exit with code 39 if remote cache evicted blobs that Bazel need durin… #17358

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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 Down Expand Up @@ -96,19 +99,16 @@ 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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know people use the hash from the error message for debugging. I also know they can look at the gRPC log to get the same hash, but not everyone collects that all the time. Are we able to preserve the hash in the error?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hash is saved in original BulkTransferException exception and will be printed out. e.g.

Failed to fetch blobs because they do not exist remotely. --remote_download_outputs=minimal does not work if your remote cache evicts blobs during builds: Missing digest: b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9/11

Updated test to assert on the hash.

+ " 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;
error =
new EnvironmentalExecException(
(BulkTransferException) error,
FailureDetail.newBuilder()
.setMessage(
"Failed to fetch blobs because they do not exist remotely."
+ " --remote_download_outputs=minimal does not work if your remote"
+ " cache evicts blobs during builds")
.setSpawn(FailureDetails.Spawn.newBuilder().setCode(Code.REMOTE_CACHE_EVICTED))
.build());
}
}
return Completable.error(error);
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,13 @@
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.DigestUtil;
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 +161,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 +179,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 @@ -303,12 +305,17 @@ public void prefetchFiles_missingFiles_fails() throws Exception {
MetadataProvider metadataProvider = new StaticMetadataProvider(metadata);
AbstractActionInputPrefetcher prefetcher = createPrefetcher(new HashMap<>());

assertThrows(
BulkTransferException.class,
() -> wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadataProvider)));
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.getMessage())
.contains(String.format("%s/%s", digest.getHash(), digest.getSizeBytes()));
}

@Test
Expand Down Expand Up @@ -347,7 +354,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 +364,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 +401,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 +413,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 +496,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,52 @@ 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();

// 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.getMessage())
.contains(
"--remote_download_outputs=minimal does not work if your remote cache evicts blobs"
+ " during builds");
assertThat(error.getMessage()).contains(String.format("%s/%s", hashCode, bytes.length));
assertThat(error.getDetailedExitCode().getExitCode().getNumericExitCode()).isEqualTo(39);
}
}