From 4ca8946a8e1c4c2fd48d8fb8ce38adb8b282fef0 Mon Sep 17 00:00:00 2001 From: Chi Wang Date: Thu, 10 Jun 2021 04:49:49 -0700 Subject: [PATCH] Remote: Add --experimental_capture_corrupted_outputs flag. 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 https://github.com/bazelbuild/continuous-integration/issues/1175. Closes #13568. PiperOrigin-RevId: 378624823 --- .../remote/RemoteActionContextProvider.java | 15 +++++ .../build/lib/remote/RemoteCache.java | 66 ++++++++++++++++++- .../build/lib/remote/RemoteModule.java | 6 +- .../common/OutputDigestMismatchException.java | 60 +++++++++++++++++ .../lib/remote/options/RemoteOptions.java | 9 +++ .../devtools/build/lib/remote/util/Utils.java | 8 +-- 6 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/remote/common/OutputDigestMismatchException.java diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java index 1db6b1cd262b63..44fcd1d937786f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java @@ -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); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java index ce008c1668ef5d..150f61881a9223 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java @@ -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; @@ -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; @@ -110,6 +112,8 @@ interface OutputFilesLocker { protected final RemoteOptions options; protected final DigestUtil digestUtil; + private Path captureCorruptedOutputsDir; + public RemoteCache( RemoteCacheClient cacheProtocol, RemoteOptions options, DigestUtil digestUtil) { this.cacheProtocol = cacheProtocol; @@ -117,6 +121,10 @@ public RemoteCache( this.digestUtil = digestUtil; } + public void setCaptureCorruptedOutputsDir(Path captureCorruptedOutputsDir) { + this.captureCorruptedOutputsDir = captureCorruptedOutputsDir; + } + public ActionResult downloadActionResult( RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) throws IOException, InterruptedException { @@ -334,7 +342,11 @@ public void download( (file) -> { try { ListenableFuture 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.immediateFailedFuture(e); @@ -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()) { @@ -461,6 +497,34 @@ private void createSymlinks(Iterable symlinks) throws IOExcepti } } + public ListenableFuture downloadFile( + RemoteActionExecutionContext context, String outputPath, Path localPath, Digest digest) + throws IOException { + SettableFuture outerF = SettableFuture.create(); + ListenableFuture f = downloadFile(context, localPath, digest); + Futures.addCallback( + f, + new FutureCallback() { + @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 downloadFile( RemoteActionExecutionContext context, Path path, Digest digest) throws IOException { 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 bc0394b32296d8..7b5a74d229275c 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 @@ -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 @@ -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, @@ -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) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/OutputDigestMismatchException.java b/src/main/java/com/google/devtools/build/lib/remote/common/OutputDigestMismatchException.java new file mode 100644 index 00000000000000..f0316502e75251 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/common/OutputDigestMismatchException.java @@ -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()); + } +} 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 872fe3a8259ac7..4cd7bdc4aa0618 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 @@ -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", diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java index 8991267c4246b0..eef958ce7df90e 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java @@ -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; @@ -390,12 +391,7 @@ public static ListenableFuture 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); } }