remote: add --remote_verify_downloads

Add this flag as a performance optimization for users who trust the outputs of their remote cache i.e. because the remote cache already verifies the correctness of the hashes. @benjaminp Closes bazelbuild#7251. PiperOrigin-RevId: 231190355
buchgr · Jan 28, 2019 · 81f3566 · 81f3566
1 parent 6a97e13
commit 81f3566
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 12 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
@@ -66,6 +66,7 @@
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+import javax.annotation.Nullable;
 
 /** A RemoteActionCache implementation that uses gRPC calls to a remote cache server. */
 @ThreadSafe
@@ -254,7 +255,9 @@ protected ListenableFuture<Void> downloadBlob(Digest digest, OutputStream out) {
     }
     resourceName += "blobs/" + digestUtil.toString(digest);
 
-    HashingOutputStream hashOut = digestUtil.newHashingOutputStream(out);
+    @Nullable
+    HashingOutputStream hashOut =
+        options.remoteVerifyDownloads ? digestUtil.newHashingOutputStream(out) : null;
     SettableFuture<Void> outerF = SettableFuture.create();
     bsAsyncStub()
         .read(
@@ -263,7 +266,7 @@ protected ListenableFuture<Void> downloadBlob(Digest digest, OutputStream out) {
               @Override
               public void onNext(ReadResponse readResponse) {
                 try {
-                  readResponse.getData().writeTo(hashOut);
+                  readResponse.getData().writeTo(hashOut != null ? hashOut : out);
                 } catch (IOException e) {
                   outerF.setException(e);
                   // Cancel the call.
@@ -285,7 +288,9 @@ public void onError(Throwable t) {
               @Override
               public void onCompleted() {
                 try {
-                  verifyContents(digest, hashOut);
+                  if (hashOut != null) {
+                    verifyContents(digest, hashOut);
+                  }
                   out.flush();
                   outerF.set(null);
                 } catch (IOException e) {

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
@@ -274,12 +274,6 @@ public final class RemoteOptions extends OptionsBase {
               + "cachable actions that output symlinks will fail.")
   public boolean allowSymlinkUpload;
 
-  // The below options are not configurable by users, only tests.
-  // This is part of the effort to reduce the overall number of flags.
-
-  /** The maximum size of an outbound message sent via a gRPC channel. */
-  public int maxOutboundMessageSize = 1024 * 1024;
-
   @Option(
       name = "remote_result_cache_priority",
       defaultValue = "0",
@@ -312,4 +306,20 @@ public final class RemoteOptions extends OptionsBase {
               + "This value will also be used if the host platform is selected as the execution "
               + "platform for remote execution.")
   public String remoteDefaultPlatformProperties;
+
+  @Option(
+      name = "remote_verify_downloads",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      help =
+          "If set to true, Bazel will compute the hash sum of all remote downloads and "
+              + " discard the remotely cached values if they don't match the expected value.")
+  public boolean remoteVerifyDownloads;
+
+  // The below options are not configurable by users, only tests.
+  // This is part of the effort to reduce the overall number of flags.
+
+  /** The maximum size of an outbound message sent via a gRPC channel. */
+  public int maxOutboundMessageSize = 1024 * 1024;
 }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java
@@ -44,6 +44,7 @@
 import java.util.Collection;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import javax.annotation.Nullable;
 
 /**
  * A RemoteActionCache implementation that uses a simple blob store for files and action output.
@@ -226,15 +227,19 @@ public void close() {
   @Override
   protected ListenableFuture<Void> downloadBlob(Digest digest, OutputStream out) {
     SettableFuture<Void> outerF = SettableFuture.create();
-    HashingOutputStream hashOut = digestUtil.newHashingOutputStream(out);
+    @Nullable
+    HashingOutputStream hashOut =
+        options.remoteVerifyDownloads ? digestUtil.newHashingOutputStream(out) : null;
     Futures.addCallback(
-        blobStore.get(digest.getHash(), hashOut),
+        blobStore.get(digest.getHash(), hashOut != null ? hashOut : out),
         new FutureCallback<Boolean>() {
           @Override
           public void onSuccess(Boolean found) {
             if (found) {
               try {
-                verifyContents(digest, hashOut);
+                if (hashOut != null) {
+                  verifyContents(digest, hashOut);
+                }
                 out.flush();
                 outerF.set(null);
               } catch (IOException e) {