Skip to content

Commit

Permalink
Remote: Add --experimental_capture_corrupted_outputs flag.
Browse files Browse the repository at this point in the history
Which when set, Bazel will save outputs whose digest does not match the expected value to the target directories.

Also use OutputDigestMismatchException to indicate the error and include output path in the error message. The message for such an error will become e.g.:

```
com.google.devtools.build.lib.remote.common.OutputDigestMismatchException: Output bazel-out/darwin-fastbuild/bin/output.txt download failed: Expected digest '872af2fe77729717832d0a020ae87a93b8b944146a2af6b3490491e1eaf1dc74/29229' does not match received digest '872af2fe77729717832d0a020ae87a93b8b944146a2af6b3490491e1eaf1dc74/29229'.
```

Used to support bazelbuild/continuous-integration#1175.

Closes bazelbuild#13568.

PiperOrigin-RevId: 378624823
  • Loading branch information
coeuvre authored and copybara-github committed Jun 10, 2021
1 parent a48937f commit 4ca8946
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,22 +73,37 @@ public static RemoteActionContextProvider createForPlaceholder(
env, /*cache=*/ null, /*executor=*/ null, retryScheduler, digestUtil, /*logDir=*/ null);
}

private static void maybeSetCaptureCorruptedOutputsDir(
RemoteOptions remoteOptions, RemoteCache remoteCache, Path workingDirectory) {
if (remoteOptions.remoteCaptureCorruptedOutputs != null
&& !remoteOptions.remoteCaptureCorruptedOutputs.isEmpty()) {
remoteCache.setCaptureCorruptedOutputsDir(
workingDirectory.getRelative(remoteOptions.remoteCaptureCorruptedOutputs));
}
}

public static RemoteActionContextProvider createForRemoteCaching(
CommandEnvironment env,
RemoteOptions options,
RemoteCache cache,
ListeningScheduledExecutorService retryScheduler,
DigestUtil digestUtil) {
maybeSetCaptureCorruptedOutputsDir(options, cache, env.getWorkingDirectory());

return new RemoteActionContextProvider(
env, cache, /*executor=*/ null, retryScheduler, digestUtil, /*logDir=*/ null);
}

public static RemoteActionContextProvider createForRemoteExecution(
CommandEnvironment env,
RemoteOptions options,
RemoteExecutionCache cache,
RemoteExecutionClient executor,
ListeningScheduledExecutorService retryScheduler,
DigestUtil digestUtil,
Path logDir) {
maybeSetCaptureCorruptedOutputsDir(options, cache, env.getWorkingDirectory());

return new RemoteActionContextProvider(
env, cache, executor, retryScheduler, digestUtil, logDir);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.SettableFuture;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
Expand All @@ -55,6 +56,7 @@
import com.google.devtools.build.lib.remote.RemoteCache.ActionResultMetadata.DirectoryMetadata;
import com.google.devtools.build.lib.remote.RemoteCache.ActionResultMetadata.FileMetadata;
import com.google.devtools.build.lib.remote.RemoteCache.ActionResultMetadata.SymlinkMetadata;
import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteActionFileArtifactValue;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
Expand Down Expand Up @@ -110,13 +112,19 @@ interface OutputFilesLocker {
protected final RemoteOptions options;
protected final DigestUtil digestUtil;

private Path captureCorruptedOutputsDir;

public RemoteCache(
RemoteCacheClient cacheProtocol, RemoteOptions options, DigestUtil digestUtil) {
this.cacheProtocol = cacheProtocol;
this.options = options;
this.digestUtil = digestUtil;
}

public void setCaptureCorruptedOutputsDir(Path captureCorruptedOutputsDir) {
this.captureCorruptedOutputsDir = captureCorruptedOutputsDir;
}

public ActionResult downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr)
throws IOException, InterruptedException {
Expand Down Expand Up @@ -334,7 +342,11 @@ public void download(
(file) -> {
try {
ListenableFuture<Void> download =
downloadFile(context, toTmpDownloadPath(file.path()), file.digest());
downloadFile(
context,
remotePathResolver.localPathToOutputPath(file.path()),
toTmpDownloadPath(file.path()),
file.digest());
return Futures.transform(download, (d) -> file, directExecutor());
} catch (IOException e) {
return Futures.<FileMetadata>immediateFailedFuture(e);
Expand All @@ -355,6 +367,30 @@ public void download(
try {
waitForBulkTransfer(downloads, /* cancelRemainingOnInterrupt=*/ true);
} catch (Exception e) {
if (captureCorruptedOutputsDir != null) {
if (e instanceof BulkTransferException) {
for (Throwable suppressed : e.getSuppressed()) {
if (suppressed instanceof OutputDigestMismatchException) {
// Capture corrupted outputs
try {
String outputPath = ((OutputDigestMismatchException) suppressed).getOutputPath();
Path localPath = ((OutputDigestMismatchException) suppressed).getLocalPath();
Path dst = captureCorruptedOutputsDir.getRelative(outputPath);
dst.createDirectoryAndParents();

// Make sure dst is still under captureCorruptedOutputsDir, otherwise
// IllegalArgumentException will be thrown.
dst.relativeTo(captureCorruptedOutputsDir);

FileSystemUtils.copyFile(localPath, dst);
} catch (Exception ee) {
ee.addSuppressed(ee);
}
}
}
}
}

try {
// Delete any (partially) downloaded output files.
for (OutputFile file : result.getOutputFilesList()) {
Expand Down Expand Up @@ -461,6 +497,34 @@ private void createSymlinks(Iterable<SymlinkMetadata> symlinks) throws IOExcepti
}
}

public ListenableFuture<Void> downloadFile(
RemoteActionExecutionContext context, String outputPath, Path localPath, Digest digest)
throws IOException {
SettableFuture<Void> outerF = SettableFuture.create();
ListenableFuture<Void> f = downloadFile(context, localPath, digest);
Futures.addCallback(
f,
new FutureCallback<Void>() {
@Override
public void onSuccess(Void unused) {
outerF.set(null);
}

@Override
public void onFailure(Throwable throwable) {
if (throwable instanceof OutputDigestMismatchException) {
OutputDigestMismatchException e = ((OutputDigestMismatchException) throwable);
e.setOutputPath(outputPath);
e.setLocalPath(localPath);
}
outerF.setException(throwable);
}
},
MoreExecutors.directExecutor());

return outerF;
}

/** Downloads a file (that is not a directory). The content is fetched from the digest. */
public ListenableFuture<Void> downloadFile(
RemoteActionExecutionContext context, Path path, Digest digest) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ private void initHttpAndDiskCache(
RemoteCache remoteCache = new RemoteCache(cacheClient, remoteOptions, digestUtil);
actionContextProvider =
RemoteActionContextProvider.createForRemoteCaching(
env, remoteCache, /* retryScheduler= */ null, digestUtil);
env, remoteOptions, remoteCache, /* retryScheduler= */ null, digestUtil);
}

@Override
Expand Down Expand Up @@ -530,7 +530,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
new RemoteExecutionCache(cacheClient, remoteOptions, digestUtil);
actionContextProvider =
RemoteActionContextProvider.createForRemoteExecution(
env, remoteCache, remoteExecutor, retryScheduler, digestUtil, logDir);
env, remoteOptions, remoteCache, remoteExecutor, retryScheduler, digestUtil, logDir);
repositoryRemoteExecutorFactoryDelegate.init(
new RemoteRepositoryRemoteExecutorFactory(
remoteCache,
Expand Down Expand Up @@ -560,7 +560,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
RemoteCache remoteCache = new RemoteCache(cacheClient, remoteOptions, digestUtil);
actionContextProvider =
RemoteActionContextProvider.createForRemoteCaching(
env, remoteCache, retryScheduler, digestUtil);
env, remoteOptions, remoteCache, retryScheduler, digestUtil);
}

if (enableRemoteDownloader) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright 2021 The Bazel Authors. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.devtools.build.lib.remote.common;

import build.bazel.remote.execution.v2.Digest;
import com.google.devtools.build.lib.vfs.Path;
import java.io.IOException;

/** An exception to indicate the digest of downloaded output does not match the expected value. */
public class OutputDigestMismatchException extends IOException {
private final Digest expected;
private final Digest actual;

private Path localPath;
private String outputPath;

public OutputDigestMismatchException(Digest expected, Digest actual) {
this.expected = expected;
this.actual = actual;
}

public void setOutputPath(String outputPath) {
this.outputPath = outputPath;
}

public String getOutputPath() {
return outputPath;
}

public Path getLocalPath() {
return localPath;
}

public void setLocalPath(Path localPath) {
this.localPath = localPath;
}

@Override
public String getMessage() {
return String.format(
"Output %s download failed: Expected digest '%s/%d' does not match "
+ "received digest '%s/%d'.",
outputPath,
expected.getHash(),
expected.getSizeBytes(),
actual.getHash(),
actual.getSizeBytes());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,15 @@ public final class RemoteOptions extends OptionsBase {
help = "Whether to use keepalive for remote execution calls.")
public boolean remoteExecutionKeepalive;

@Option(
name = "experimental_remote_capture_corrupted_outputs",
defaultValue = "null",
documentationCategory = OptionDocumentationCategory.REMOTE,
effectTags = {OptionEffectTag.UNKNOWN},
converter = OptionsUtils.PathFragmentConverter.class,
help = "A path to a directory where the corrupted outputs will be captured to.")
public PathFragment remoteCaptureCorruptedOutputs;

@Option(
name = "remote_cache",
oldName = "remote_http_cache",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import com.google.devtools.build.lib.authandtls.CallCredentialsProvider;
import com.google.devtools.build.lib.remote.ExecutionStatusException;
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
import com.google.devtools.build.lib.server.FailureDetails;
Expand Down Expand Up @@ -390,12 +391,7 @@ public static ListenableFuture<ActionResult> downloadAsActionResult(

public static void verifyBlobContents(Digest expected, Digest actual) throws IOException {
if (!expected.equals(actual)) {
String msg =
String.format(
"Output download failed: Expected digest '%s/%d' does not match "
+ "received digest '%s/%d'.",
expected.getHash(), expected.getSizeBytes(), actual.getHash(), actual.getSizeBytes());
throw new IOException(msg);
throw new OutputDigestMismatchException(expected, actual);
}
}

Expand Down

0 comments on commit 4ca8946

Please sign in to comment.