Add --build_event_upload_max_threads option

This is a follow up for bazelbuild/bazel@7fc967c, related issue bazelbuild/bazel#6806 RELNOTES: None PiperOrigin-RevId: 223944430
luca-digrazia · Sep 4, 2022 · ab91e4c · ab91e4c
1 parent 66b27ae
commit ab91e4c
Showing 4 changed files with 6 additions and 21 deletions.
diff --git a/.../main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/.../main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java
@@ -17,7 +17,6 @@
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.hash.HashCode;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
@@ -68,11 +67,6 @@ class ByteStreamBuildEventArtifactUploader implements BuildEventArtifactUploader
             Executors.newFixedThreadPool(Math.min(maxUploadThreads, 1000)));
   }
 
-  private static boolean isRemoteFile(Path file) {
-    return file.getFileSystem() instanceof RemoteActionFileSystem
-        && ((RemoteActionFileSystem) file.getFileSystem()).isRemote(file);
-  }
-
   @Override
   public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) {
     if (files.isEmpty()) {
@@ -91,16 +85,11 @@ public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) {
                 }
                 DigestUtil digestUtil = new DigestUtil(file.getFileSystem().getDigestFunction());
                 Digest digest = digestUtil.compute(file);
-                if (isRemoteFile(file)) {
-                  return Futures.immediateFuture(new PathDigestPair(file, digest));
-                }
-                Chunker chunker = Chunker.builder().setInput(digest.getSizeBytes(), file).build();
+                Chunker chunker = Chunker.builder(digestUtil).setInput(digest, file).build();
                 final ListenableFuture<Void> upload;
                 Context prevCtx = ctx.attach();
                 try {
-                  upload =
-                      uploader.uploadBlobAsync(
-                          HashCode.fromString(digest.getHash()), chunker, /* forceUpload=*/ false);
+                  upload = uploader.uploadBlobAsync(chunker, /*forceUpload=*/ false);
                 } finally {
                   ctx.detach(prevCtx);
                 }
@@ -116,11 +105,6 @@ public ListenableFuture<PathConverter> upload(Map<Path, LocalFile> files) {
         MoreExecutors.directExecutor());
   }
 
-  @Override
-  public boolean mayBeSlow() {
-    return true;
-  }
-
   @Override
   public void shutdown() {
     if (shutdown.getAndSet(true)) {

diff --git a/...ava/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java b/...ava/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.remote;
 
 import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
-import com.google.devtools.build.lib.remote.options.RemoteOptions;
 import com.google.devtools.build.lib.runtime.BuildEventArtifactUploaderFactory;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import io.grpc.Context;
@@ -29,7 +28,7 @@ class ByteStreamBuildEventArtifactUploaderFactory implements
   private final ByteStreamUploader uploader;
   private final String remoteServerName;
   private final Context ctx;
-  @Nullable private final String remoteInstanceName;
+  private final @Nullable String remoteInstanceName;
 
   ByteStreamBuildEventArtifactUploaderFactory(
       ByteStreamUploader uploader, String remoteServerName, Context ctx,

diff --git a/...va/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java b/...va/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
@@ -237,7 +237,7 @@ public final class RemoteOptions extends OptionsBase {
 
   @Option(
       name = "incompatible_remote_symlinks",
-      defaultValue = "true",
+      defaultValue = "false",
       category = "remote",
       documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
       effectTags = {OptionEffectTag.EXECUTION},

diff --git a/...t/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/...t/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java
@@ -29,6 +29,7 @@
 import com.google.devtools.build.lib.clock.JavaClock;
 import com.google.devtools.build.lib.remote.ByteStreamUploaderTest.FixedBackoff;
 import com.google.devtools.build.lib.remote.ByteStreamUploaderTest.MaybeFailOnceUploadService;
+import com.google.devtools.build.lib.remote.Retrier.RetryException;
 import com.google.devtools.build.lib.remote.util.DigestUtil;
 import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
 import com.google.devtools.build.lib.vfs.DigestHashFunction;
@@ -239,6 +240,7 @@ public void onCompleted() {
       artifactUploader.upload(filesToUpload).get();
       fail("exception expected.");
     } catch (ExecutionException e) {
+      assertThat(e.getCause()).isInstanceOf(RetryException.class);
       assertThat(Status.fromThrowable(e).getCode()).isEqualTo(Status.CANCELLED.getCode());
     }