Enable --experimental_remote_spawn_cache by default

- It is now an error to specify the gRPC remote execution backend in
  combination with a local disk or HTTP-based cache.
- It is now an error to specify both local disk and HTTP-based caches.

Note that before this CL, enabling the local disk cache silently disabled
remote execution - we now give an error in that case.

With these combination no longer being accepted, remote execution being enabled
now means that we only create a RemoteSpawnRunner, and don't provide a
SpawnCache. This is not a semantic change - we never created both.

In principle, it should be possible for users to combine local execution with
remote caching for actions that are marked local or no-remote, and still use
remote execution otherwise. However, Bazel cannot currently express this
combination of execution strategies.

RELNOTES: The --experimental_remote_spawn_cache flag is now enabled by default, and remote caching no longer needs --*_strategy=remote flags (it will fail if they are specified).
PiperOrigin-RevId: 198280398

4 files changed, 28 insertions, 15 deletions

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 e6222b4ebe..4bcbb74ff1 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
@@ -38,8 +38,8 @@ import javax.annotation.Nullable;
  */
 final class RemoteActionContextProvider extends ActionContextProvider {
   private final CommandEnvironment env;
-  private final AbstractRemoteActionCache cache;
-  private final GrpcRemoteExecutor executor;
+  @Nullable private final AbstractRemoteActionCache cache;
+  @Nullable private final GrpcRemoteExecutor executor;
   private final DigestUtil digestUtil;
   private final Path logDir;
 
@@ -64,7 +64,7 @@ final class RemoteActionContextProvider extends ActionContextProvider {
     String buildRequestId = env.getBuildRequestId().toString();
     String commandId = env.getCommandId().toString();
 
-    if (remoteOptions.experimentalRemoteSpawnCache || remoteOptions.diskCache != null) {
+    if (executor == null && cache != null) {
       RemoteSpawnCache spawnCache =
           new RemoteSpawnCache(
               env.getExecRoot(),
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 36521f987b..11a6119bae 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
@@ -97,7 +97,7 @@ public final class RemoteModule extends BlazeModule {
   }
 
   @Override
-  public void beforeCommand(CommandEnvironment env) {
+  public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
     env.getEventBus().register(this);
     String buildRequestId = env.getBuildRequestId().toString();
     String commandId = env.getCommandId().toString();
@@ -113,6 +113,7 @@ public final class RemoteModule extends BlazeModule {
     } catch (IOException e) {
       env.getReporter()
           .handle(Event.error("Could not create base directory for remote logs: " + logDir));
+      throw new AbruptExitException(ExitCode.LOCAL_ENVIRONMENTAL_ERROR, e);
     }
     RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class);
     AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
@@ -126,10 +127,22 @@ public final class RemoteModule extends BlazeModule {
       return;
     }
 
-    try {
-      boolean remoteOrLocalCache = SimpleBlobStoreFactory.isRemoteCacheOptions(remoteOptions);
-      boolean grpcCache = GrpcRemoteCache.isRemoteCacheOptions(remoteOptions);
+    boolean enableRestCache = SimpleBlobStoreFactory.isRestUrlOptions(remoteOptions);
+    boolean enableDiskCache = SimpleBlobStoreFactory.isDiskCache(remoteOptions);
+    if (enableRestCache && enableDiskCache) {
+      throw new AbruptExitException(
+          "Cannot enable HTTP-based and local disk cache simultaneously",
+          ExitCode.COMMAND_LINE_ERROR);
+    }
+    boolean enableBlobStoreCache = enableRestCache || enableDiskCache;
+    boolean enableGrpcCache = GrpcRemoteCache.isRemoteCacheOptions(remoteOptions);
+    if (enableBlobStoreCache && remoteOptions.remoteExecutor != null) {
+      throw new AbruptExitException(
+          "Cannot combine gRPC based remote execution with local disk or HTTP-based caching",
+          ExitCode.COMMAND_LINE_ERROR);
+    }
 
+    try {
       LoggingInterceptor logger = null;
       if (!remoteOptions.experimentalRemoteGrpcLog.isEmpty()) {
         rpcLogFile = new AsynchronousFileOutputStream(remoteOptions.experimentalRemoteGrpcLog);
@@ -139,10 +152,8 @@ public final class RemoteModule extends BlazeModule {
       RemoteRetrier retrier =
           new RemoteRetrier(
               remoteOptions, RemoteRetrier.RETRIABLE_GRPC_ERRORS, Retrier.ALLOW_ALL_CALLS);
-      // TODO(davido): The naming is wrong here. "Remote"-prefix in RemoteActionCache class has no
-      // meaning.
       final AbstractRemoteActionCache cache;
-      if (remoteOrLocalCache) {
+      if (enableBlobStoreCache) {
         cache =
             new SimpleBlobStoreActionCache(
                 remoteOptions,
@@ -151,9 +162,9 @@ public final class RemoteModule extends BlazeModule {
                     GoogleAuthUtils.newCredentials(authAndTlsOptions),
                     env.getWorkingDirectory()),
                 digestUtil);
-      } else if (grpcCache || remoteOptions.remoteExecutor != null) {
+      } else if (enableGrpcCache || remoteOptions.remoteExecutor != null) {
         // If a remote executor but no remote cache is specified, assume both at the same target.
-        String target = grpcCache ? remoteOptions.remoteCache : remoteOptions.remoteExecutor;
+        String target = enableGrpcCache ? remoteOptions.remoteCache : remoteOptions.remoteExecutor;
         Channel ch = GoogleAuthUtils.newChannel(target, authAndTlsOptions);
         if (logger != null) {
           ch = ClientInterceptors.intercept(ch, logger);
@@ -169,6 +180,8 @@ public final class RemoteModule extends BlazeModule {
         cache = null;
       }
 
+      // TODO(davido): The naming is wrong here. "Remote"-prefix in RemoteActionCache class has no
+      // meaning.
       final GrpcRemoteExecutor executor;
       if (remoteOptions.remoteExecutor != null) {
         Channel ch = GoogleAuthUtils.newChannel(remoteOptions.remoteExecutor, authAndTlsOptions);
@@ -184,7 +197,6 @@ public final class RemoteModule extends BlazeModule {
       } else {
         executor = null;
       }
-
       actionContextProvider =
           new RemoteActionContextProvider(env, cache, executor, digestUtil, logDir);
     } 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
index 3431e95dac..ebef95e8b2 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
@@ -164,11 +164,12 @@ public final class RemoteOptions extends OptionsBase {
   )
   public double experimentalRemoteRetryJitter;
 
+  @Deprecated
   @Option(
     name = "experimental_remote_spawn_cache",
     defaultValue = "false",
     documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-    effectTags = {OptionEffectTag.UNKNOWN},
+    effectTags = {OptionEffectTag.NO_OP},
     help =
         "Whether to use the experimental spawn cache infrastructure for remote caching. "
             + "Enabling this flag makes Bazel ignore any setting for remote_executor."
diff --git a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java
index 424a6886f4..47367560e9 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreFactory.java
@@ -77,7 +77,7 @@ public final class SimpleBlobStoreFactory {
     return options.diskCache != null;
   }
 
-  private static boolean isRestUrlOptions(RemoteOptions options) {
+  static boolean isRestUrlOptions(RemoteOptions options) {
     return options.remoteHttpCache != null;
   }
 }