Add no-remote-cache and no-remote-exec execution requirements

This resolves bazelbuild#7932 It introduces two new execution requirements: - `no-remote-exec`: disallows remote execution without preventing remote caching - `no-remote-cache`: disallows remote caching without preventing remote execution or local caching The current behavior of `no-remote` and `no-cache` remain unchanged. Closes bazelbuild#8710. PiperOrigin-RevId: 259901280
exoson · Jul 25, 2019 · 8860c3e · 8860c3e
1 parent a1db1ed
commit 8860c3e
Show file tree

Hide file tree

Showing 8 changed files with 281 additions and 119 deletions.
diff --git a/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html b/src/main/java/com/google/devtools/build/docgen/templates/attributes/common/tags.html
@@ -28,13 +28,25 @@
   </li>
 
   <li><code>no-cache</code> keyword results in the action or test never being
-    cached.
+    cached (remotely or locally)
   </li>
 
-  <li><code>no-remote</code> keyword results in the action or test never being
+  <li><code>no-remote-cache</code> keyword results in the action or test never being
+    cached remotely (but it may be cached locally; it may also be executed remotely).
+    Note: for the purposes of this tag, the disk-cache is considered a local cache, whereas
+    the http and gRPC caches are considered remote.
+    If a combined cache is specified (i.e. a cache with local and remote components),
+    it's treated as a remote cache and disabled entirely.
+  </li>
+
+  <li><code>no-remote-exec</code> keyword results in the action or test never being
     executed remotely (but it may be cached remotely).
   </li>
 
+  <li><code>no-remote</code> keyword prevents the action or test from being executed remotely
+    or cached remotely. This is equivalent to using both no-remote-cache and no-remote-exec.
+  </li>
+
   <li><code>local</code> keyword precludes the action or test from being remotely cached,
     remotely executed, or run inside the
     <a href="../user-manual.html#sandboxing">sandbox</a>.

diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java
@@ -176,14 +176,24 @@ public String parseIfMatches(String tag) throws ValidationException {
    */
   public static final String NO_CACHE = "no-cache";
 
+  /** Disables remote caching of a spawn. Note: does not disable remote execution */
+  public static final String NO_REMOTE_CACHE = "no-remote-cache";
+
+  /** Disables remote execution of a spawn. Note: does not disable remote caching */
+  public static final String NO_REMOTE_EXEC = "no-remote-exec";
+
+  /**
+   * Disables both remote execution and remote caching of a spawn. This is the equivalent of using
+   * no-remote-cache and no-remote-exec together.
+   */
+  public static final String NO_REMOTE = "no-remote";
+
   /** Disables local sandboxing of a spawn. */
   public static final String LEGACY_NOSANDBOX = "nosandbox";
 
   /** Disables local sandboxing of a spawn. */
   public static final String NO_SANDBOX = "no-sandbox";
 
-  /** Disables remote execution of a spawn. */
-  public static final String NO_REMOTE = "no-remote";
 
   /**
    * Enables networking for a spawn if possible (only if sandboxing is enabled and if the sandbox

diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java
@@ -33,6 +33,20 @@ public static boolean mayBeCached(Spawn spawn) {
         && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL);
   }
 
+  /** Returns {@code true} if the result of {@code spawn} may be cached remotely. */
+  public static boolean mayBeCachedRemotely(Spawn spawn) {
+    return mayBeCached(spawn)
+        && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE)
+        && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE_CACHE);
+  }
+
+  /** Returns {@code true} if {@code spawn} may be executed remotely. */
+  public static boolean mayBeExecutedRemotely(Spawn spawn) {
+    return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL)
+        && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE)
+        && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE_EXEC);
+  }
+
   /** Returns whether a Spawn can be executed in a sandbox environment. */
   public static boolean mayBeSandboxed(Spawn spawn) {
     return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LEGACY_NOSANDBOX)
@@ -52,15 +66,6 @@ public static boolean requiresNetwork(Spawn spawn, boolean defaultSandboxDisallo
     return defaultSandboxDisallowNetwork;
   }
 
-  /**
-   * Returns whether a Spawn claims to support being executed remotely according to its execution
-   * info tags.
-   */
-  public static boolean mayBeExecutedRemotely(Spawn spawn) {
-    return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL)
-        && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE);
-  }
-
   /**
    * Returns whether a Spawn claims to support being executed with the persistent worker strategy
    * according to its execution info tags.

diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnCache.java
@@ -162,7 +162,8 @@ interface CacheHandle extends Closeable {
    * object is for the cache to store expensive intermediate values (such as the cache key) that are
    * needed both for the lookup and the subsequent store operation.
    *
-   * <p>The lookup must not succeed for non-cachable spawns. See {@link Spawns#mayBeCached()}.
+   * <p>The lookup must not succeed for non-cachable spawns. See {@link Spawns#mayBeCached()} and
+   * {@link Spawns#mayBeCachedRemotely}.
    *
    * <p>Note that cache stores may be disabled, in which case the returned {@link CacheHandle}
    * instance's {@link CacheHandle#store} is a no-op.

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -111,11 +111,13 @@ final class RemoteSpawnCache implements SpawnCache {
     this.topLevelOutputs = Preconditions.checkNotNull(topLevelOutputs, "topLevelOutputs");
   }
 
-
   @Override
   public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
       throws InterruptedException, IOException, ExecException {
-    if (!Spawns.mayBeCached(spawn) || !Spawns.mayBeExecutedRemotely(spawn)) {
+    if (!Spawns.mayBeCached(spawn)
+        || (!Spawns.mayBeCachedRemotely(spawn) && isRemoteCache(options))) {
+      // returning SpawnCache.NO_RESULT_NO_STORE in case the caching is disabled or in case
+      // the remote caching is disabled and the only configured cache is remote.
       return SpawnCache.NO_RESULT_NO_STORE;
     }
     boolean checkCache = options.remoteAcceptCached;
@@ -291,4 +293,8 @@ private void report(Event evt) {
       cmdlineReporter.handle(evt);
     }
   }
+
+  private static boolean isRemoteCache(RemoteOptions options) {
+    return !isNullOrEmpty(options.remoteCache);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -156,10 +156,10 @@ public String getName() {
   @Override
   public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
       throws ExecException, InterruptedException, IOException {
-    if (!Spawns.mayBeExecutedRemotely(spawn)) {
-      return execLocally(spawn, context);
-    }
-    boolean spawnCachable = Spawns.mayBeCached(spawn);
+
+    boolean spawnCacheableRemotely = Spawns.mayBeCachedRemotely(spawn);
+    boolean uploadLocalResults = remoteOptions.remoteUploadLocalResults && spawnCacheableRemotely;
+    boolean acceptCachedResult = remoteOptions.remoteAcceptCached && spawnCacheableRemotely;
 
     context.report(ProgressStatus.EXECUTING, getName());
     RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode;
@@ -177,21 +177,20 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
     Digest commandHash = digestUtil.compute(command);
     Action action =
         buildAction(
-            commandHash,
-            merkleTree.getRootDigest(),
-            context.getTimeout(),
-            Spawns.mayBeCached(spawn));
+            commandHash, merkleTree.getRootDigest(), context.getTimeout(), spawnCacheableRemotely);
     ActionKey actionKey = digestUtil.computeActionKey(action);
 
+    if (!Spawns.mayBeExecutedRemotely(spawn)) {
+      return execLocallyAndUpload(
+          spawn, context, inputMap, actionKey, action, command, uploadLocalResults);
+    }
+
     // Look up action cache, and reuse the action output if it is found.
     Context withMetadata =
         TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey);
     Context previous = withMetadata.attach();
     Profiler prof = Profiler.instance();
     try {
-      boolean acceptCachedResult = remoteOptions.remoteAcceptCached && spawnCachable;
-      boolean uploadLocalResults = remoteOptions.remoteUploadLocalResults && spawnCachable;
-
       try {
         // Try to lookup the action in the action cache.
         ActionResult cachedResult;
@@ -223,7 +222,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
       if (remoteExecutor == null) {
         // Remote execution is disabled and so execute the spawn on the local machine.
         return execLocallyAndUpload(
-            spawn, context, inputMap, remoteCache, actionKey, action, command, uploadLocalResults);
+            spawn, context, inputMap, actionKey, action, command, uploadLocalResults);
       }
 
       ExecuteRequest.Builder requestBuilder =
@@ -246,13 +245,17 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
             () -> {
               ExecuteRequest request = requestBuilder.build();
 
-              // Upload the command and all the inputs into the remote cache.
-              try (SilentCloseable c = prof.profile(UPLOAD_TIME, "upload missing inputs")) {
-                Map<Digest, Message> additionalInputs = Maps.newHashMapWithExpectedSize(2);
-                additionalInputs.put(actionKey.getDigest(), action);
-                additionalInputs.put(commandHash, command);
-                remoteCache.ensureInputsPresent(merkleTree, additionalInputs, execRoot);
+              // Upload the command and all the inputs into the remote cache, if remote caching
+              // is enabled and not disabled using tags, {@see Spawns#mayBeCachedRemotely}
+              if (spawnCacheableRemotely) {
+                try (SilentCloseable c = prof.profile(UPLOAD_TIME, "upload missing inputs")) {
+                  Map<Digest, Message> additionalInputs = Maps.newHashMapWithExpectedSize(2);
+                  additionalInputs.put(actionKey.getDigest(), action);
+                  additionalInputs.put(commandHash, command);
+                  remoteCache.ensureInputsPresent(merkleTree, additionalInputs, execRoot);
+                }
               }
+
               ExecuteResponse reply;
               try (SilentCloseable c = prof.profile(REMOTE_EXECUTION, "execute remotely")) {
                 reply = remoteExecutor.executeRemotely(request);
@@ -398,7 +401,7 @@ private SpawnResult execLocallyAndUploadOrFail(
     }
     if (remoteOptions.remoteLocalFallback && !RemoteRetrierUtils.causedByExecTimeout(cause)) {
       return execLocallyAndUpload(
-          spawn, context, inputMap, remoteCache, actionKey, action, command, uploadLocalResults);
+          spawn, context, inputMap, actionKey, action, command, uploadLocalResults);
     }
     return handleError(cause, context.getFileOutErr(), actionKey, context);
   }
@@ -522,7 +525,6 @@ SpawnResult execLocallyAndUpload(
       Spawn spawn,
       SpawnExecutionContext context,
       SortedMap<PathFragment, ActionInput> inputMap,
-      AbstractRemoteActionCache remoteCache,
       ActionKey actionKey,
       Action action,
       Command command,