RemoteCacheClient: add getServerCapabilities

bazelbuild · Sep 11, 2024 · 1bb3bf6 · 1bb3bf6
1 parent ffc5a49
commit 1bb3bf6
Show file tree

Hide file tree

Showing 18 changed files with 453 additions and 121 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/ApiVersion.java b/src/main/java/com/google/devtools/build/lib/remote/ApiVersion.java
@@ -23,12 +23,21 @@ public class ApiVersion implements Comparable<ApiVersion> {
   public final int patch;
   public final String prerelease;
 
+  // The version of the Remote Execution API that Bazel supports initially.
+  public static final ApiVersion twoPointZero =
+      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(0).build());
+  // The version of the Remote Execution API that starts supporting the
+  // Command.output_paths and ActionResult.output_symlinks fields.
+  public static final ApiVersion twoPointOne =
+      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(1).build());
+  // The latest version of the Remote Execution API that Bazel is compatible with.
+  public static final ApiVersion twoPointTwo =
+      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(2).build());
+
   // The current lowest/highest versions (inclusive) of the Remote Execution API that Bazel
   // supports. These fields will need to be updated together with all version changes.
-  public static final ApiVersion low =
-      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(0).build());
-  public static final ApiVersion high =
-      new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(0).build());
+  public static final ApiVersion low = twoPointZero;
+  public static final ApiVersion high = twoPointTwo;
 
   public ApiVersion(int major, int minor, int patch, String prerelease) {
     this.major = major;

diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java
@@ -29,6 +29,7 @@
 import build.bazel.remote.execution.v2.FindMissingBlobsResponse;
 import build.bazel.remote.execution.v2.GetActionResultRequest;
 import build.bazel.remote.execution.v2.RequestMetadata;
+import build.bazel.remote.execution.v2.ServerCapabilities;
 import build.bazel.remote.execution.v2.UpdateActionResultRequest;
 import com.google.bytestream.ByteStreamGrpc;
 import com.google.bytestream.ByteStreamGrpc.ByteStreamStub;
@@ -270,6 +271,11 @@ public CacheCapabilities getCacheCapabilities() throws IOException {
     return channel.getServerCapabilities().getCacheCapabilities();
   }
 
+  @Override
+  public ServerCapabilities getServerCapabilities() throws IOException {
+    return channel.getServerCapabilities();
+  }
+
   @Override
   public ListenableFuture<String> getAuthority() {
     return channel.withChannelFuture(ch -> Futures.immediateFuture(ch.authority()));

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
@@ -26,6 +26,7 @@
 import build.bazel.remote.execution.v2.ActionResult;
 import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
+import build.bazel.remote.execution.v2.ServerCapabilities;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.flogger.GoogleLogger;
@@ -120,6 +121,13 @@ public ListenableFuture<String> getRemoteAuthority() {
     return remoteCacheClient.getAuthority();
   }
 
+  public ServerCapabilities getServerCapabilities() throws IOException {
+    if (remoteCacheClient == null) {
+      return ServerCapabilities.getDefaultInstance();
+    }
+    return remoteCacheClient.getServerCapabilities();
+  }
+
   public CachedActionResult downloadActionResult(
       RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr)
       throws IOException, InterruptedException {

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -196,6 +196,8 @@ public class RemoteExecutionService {
   @Nullable private final Scrubber scrubber;
   private final Set<Digest> knownMissingCasDigests;
 
+  private Boolean useOutputPaths;
+
   public RemoteExecutionService(
       Executor executor,
       Reporter reporter,
@@ -242,32 +244,40 @@ public RemoteExecutionService(
   }
 
   private Command buildCommand(
+      boolean useOutputPaths,
       Collection<? extends ActionInput> outputs,
       List<String> arguments,
       ImmutableMap<String, String> env,
       @Nullable Platform platform,
       RemotePathResolver remotePathResolver,
       @Nullable SpawnScrubber spawnScrubber) {
     Command.Builder command = Command.newBuilder();
-    ArrayList<String> outputFiles = new ArrayList<>();
-    ArrayList<String> outputDirectories = new ArrayList<>();
-    ArrayList<String> outputPaths = new ArrayList<>();
-    for (ActionInput output : outputs) {
-      String pathString =
-          reencodeInternalToExternal(remotePathResolver.localPathToOutputPath(output));
-      if (output.isDirectory()) {
-        outputDirectories.add(pathString);
-      } else {
-        outputFiles.add(pathString);
+    if (useOutputPaths) {
+      var outputPaths = new ArrayList<String>();
+      for (ActionInput output : outputs) {
+        String pathString =
+            reencodeInternalToExternal(remotePathResolver.localPathToOutputPath(output));
+        outputPaths.add(pathString);
+      }
+      Collections.sort(outputPaths);
+      command.addAllOutputPaths(outputPaths);
+    } else {
+      var outputFiles = new ArrayList<String>();
+      var outputDirectories = new ArrayList<String>();
+      for (ActionInput output : outputs) {
+        String pathString =
+            reencodeInternalToExternal(remotePathResolver.localPathToOutputPath(output));
+        if (output.isDirectory()) {
+          outputDirectories.add(pathString);
+        } else {
+          outputFiles.add(pathString);
+        }
       }
-      outputPaths.add(pathString);
+      Collections.sort(outputFiles);
+      Collections.sort(outputDirectories);
+      command.addAllOutputFiles(outputFiles);
+      command.addAllOutputDirectories(outputDirectories);
     }
-    Collections.sort(outputFiles);
-    Collections.sort(outputDirectories);
-    Collections.sort(outputPaths);
-    command.addAllOutputFiles(outputFiles);
-    command.addAllOutputDirectories(outputDirectories);
-    command.addAllOutputPaths(outputPaths);
 
     if (platform != null) {
       command.setPlatform(platform);
@@ -542,6 +552,63 @@ private void maybeReleaseRemoteActionBuildingSemaphore() {
     remoteActionBuildingSemaphore.release();
   }
 
+  private synchronized void initUseOutputPaths() {
+    // If this has already been initialized, return
+    if (this.useOutputPaths != null) {
+      return;
+    }
+    this.useOutputPaths = remoteOptions.useOutputPaths;
+    try {
+      // If both Remote Executor and Remote Cache are configured,
+      // use the highest version that is supported by both server to compare.
+      if (remoteExecutor != null && remoteCache != null) {
+        var executorSupportStatus =
+            ClientApiVersion.current.checkServerSupportedVersions(
+                remoteExecutor.getServerCapabilities());
+        var cacheSupportStatus =
+            ClientApiVersion.current.checkServerSupportedVersions(
+                remoteCache.getServerCapabilities());
+
+        ApiVersion highestSupportedApiVersion = ApiVersion.low;
+        if (executorSupportStatus.isSupported() && cacheSupportStatus.isSupported()) {
+          var executorHighestVersion = executorSupportStatus.getHighestSupportedVersion();
+          var cacheHighestVersion = cacheSupportStatus.getHighestSupportedVersion();
+
+          var commonServerVersion =
+              executorHighestVersion.compareTo(cacheHighestVersion) <= 0
+                  ? executorHighestVersion
+                  : cacheHighestVersion;
+          this.useOutputPaths = commonServerVersion.compareTo(ApiVersion.twoPointOne) >= 0;
+          return;
+        }
+      }
+
+      if (remoteExecutor != null) {
+        var capabilities = remoteExecutor.getServerCapabilities();
+        if (capabilities != null) {
+          var supportStatus = ClientApiVersion.current.checkServerSupportedVersions(capabilities);
+          if (supportStatus.isSupported()) {
+            this.useOutputPaths =
+                supportStatus.getHighestSupportedVersion().compareTo(ApiVersion.twoPointOne) >= 0;
+            return;
+          }
+        }
+      }
+      if (remoteCache != null) {
+        var capabilities = remoteCache.getServerCapabilities();
+        if (capabilities != null) {
+          var supportStatus = ClientApiVersion.current.checkServerSupportedVersions(capabilities);
+          if (supportStatus.isSupported()) {
+            this.useOutputPaths =
+                supportStatus.getHighestSupportedVersion().compareTo(ApiVersion.twoPointOne) >= 0;
+          }
+        }
+      }
+    } catch (IOException e) {
+      // If we can't determine the server capabilities, use the default value.
+    }
+  }
+
   /** Creates a new {@link RemoteAction} instance from spawn. */
   public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context)
       throws IOException, ExecException, ForbiddenActionInputException, InterruptedException {
@@ -568,8 +635,12 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context
         platform = PlatformUtils.getPlatformProto(spawn, remoteOptions);
       }
 
+      if (this.useOutputPaths == null) {
+        initUseOutputPaths();
+      }
       Command command =
           buildCommand(
+              this.useOutputPaths,
               spawn.getOutputFiles(),
               spawn.getArguments(),
               spawn.getEnvironment(),

diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java
@@ -18,6 +18,7 @@
 import build.bazel.remote.execution.v2.ActionResult;
 import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
+import build.bazel.remote.execution.v2.ServerCapabilities;
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.ListenableFuture;
@@ -35,6 +36,8 @@
 public interface RemoteCacheClient extends MissingDigestsFinder {
   CacheCapabilities getCacheCapabilities() throws IOException;
 
+  ServerCapabilities getServerCapabilities() throws IOException;
+
   ListenableFuture<String> getAuthority();
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java
@@ -22,6 +22,7 @@
 import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
 import build.bazel.remote.execution.v2.Directory;
+import build.bazel.remote.execution.v2.ServerCapabilities;
 import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy;
 import build.bazel.remote.execution.v2.Tree;
 import com.google.common.base.Ascii;
@@ -253,6 +254,11 @@ public CacheCapabilities getCacheCapabilities() {
         .build();
   }
 
+  @Override
+  public ServerCapabilities getServerCapabilities() {
+    return ServerCapabilities.newBuilder().setCacheCapabilities(getCacheCapabilities()).build();
+  }
+
   @Override
   public ListenableFuture<String> getAuthority() {
     return immediateFuture("");

diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java
@@ -17,6 +17,7 @@
 import build.bazel.remote.execution.v2.ActionResult;
 import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Digest;
+import build.bazel.remote.execution.v2.ServerCapabilities;
 import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy;
 import com.google.auth.Credentials;
 import com.google.common.collect.ImmutableList;
@@ -614,6 +615,11 @@ public CacheCapabilities getCacheCapabilities() {
         .build();
   }
 
+  @Override
+  public ServerCapabilities getServerCapabilities() {
+    return ServerCapabilities.newBuilder().setCacheCapabilities(getCacheCapabilities()).build();
+  }
+
   @Override
   public ListenableFuture<String> getAuthority() {
     return Futures.immediateFuture("");

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
@@ -413,6 +413,16 @@ public RemoteBuildEventUploadModeConverter() {
               + " --experimental_remote_cache_compression_threshold.")
   public boolean cacheCompression;
 
+  @Option(
+      name = "incompatible_remote_use_output_paths",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.REMOTE,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      help =
+          "If enabled, check if the remote server(s) support remote execution API v2.1 or newer."
+              + "If yes, use the newer Command.output_paths field. Default is false.")
+  public boolean useOutputPaths;
+
   @Option(
       name = "experimental_remote_cache_compression_threshold",
       // Based on discussions in #18997, `~100` is the break even point where the compression