From 0d7d44d5b2e65741812ca644542a24c7618e193f Mon Sep 17 00:00:00 2001 From: chiwang Date: Tue, 7 Dec 2021 06:33:35 -0800 Subject: [PATCH] Remote: Add support for tag no-remote-cache-upload. When executing a spawn that is tagged with no-remote-cache-upload, Bazel still looks up the remote cache. However, its local outputs are not uploaded after local execution. Part of #14338. PiperOrigin-RevId: 414708472 --- .../lib/actions/ExecutionRequirements.java | 3 ++ .../devtools/build/lib/actions/Spawns.java | 25 ++++++++----- .../devtools/build/lib/remote/util/Utils.java | 25 +++++++++---- .../bazel/remote/remote_execution_test.sh | 36 +++++++++++++++++++ 4 files changed, 74 insertions(+), 15 deletions(-) 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 index dcdd4358b497c4..a21f1417860655 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java @@ -194,6 +194,9 @@ public enum WorkerProtocolFormat { /** Disables remote caching of a spawn. Note: does not disable remote execution */ public static final String NO_REMOTE_CACHE = "no-remote-cache"; + /** Disables upload part of remote caching of a spawn. Note: does not disable remote execution */ + public static final String NO_REMOTE_CACHE_UPLOAD = "no-remote-cache-upload"; + /** Disables remote execution of a spawn. Note: does not disable remote caching */ public static final String NO_REMOTE_EXEC = "no-remote-exec"; 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 index 182fc0c6e6d448..82db8bd1a7c9e6 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawns.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawns.java @@ -19,24 +19,33 @@ import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code; import java.io.IOException; import java.time.Duration; +import java.util.Map; /** Helper methods relating to implementations of {@link Spawn}. */ public final class Spawns { private Spawns() {} - /** - * Returns {@code true} if the result of {@code spawn} may be cached. - */ + /** Returns {@code true} if the result of {@code spawn} may be cached. */ public static boolean mayBeCached(Spawn spawn) { - return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_CACHE) - && !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL); + return mayBeCached(spawn.getExecutionInfo()); + } + + /** Returns {@code true} if the result of {@code spawn} may be cached. */ + public static boolean mayBeCached(Map executionInfo) { + return !executionInfo.containsKey(ExecutionRequirements.NO_CACHE) + && !executionInfo.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); + return mayBeCachedRemotely(spawn.getExecutionInfo()); + } + + /** Returns {@code true} if the result of {@code spawn} may be cached remotely. */ + public static boolean mayBeCachedRemotely(Map executionInfo) { + return mayBeCached(executionInfo) + && !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE) + && !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE_CACHE); } /** Returns {@code true} if {@code spawn} may be executed remotely. */ diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java index a13faab04af198..16365bb1847d8d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java @@ -25,6 +25,7 @@ import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.AsyncCallable; import com.google.common.util.concurrent.FluentFuture; @@ -71,6 +72,7 @@ import java.util.Arrays; import java.util.Collection; import java.util.Locale; +import java.util.Map; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.function.BiFunction; @@ -596,9 +598,20 @@ public static boolean shouldAcceptCachedResultFromCombinedCache( } public static boolean shouldUploadLocalResultsToRemoteCache( - RemoteOptions remoteOptions, @Nullable Spawn spawn) { + RemoteOptions remoteOptions, @Nullable Map executionInfo) { return remoteOptions.remoteUploadLocalResults - && (spawn == null || Spawns.mayBeCachedRemotely(spawn)); + && (executionInfo == null + || (Spawns.mayBeCachedRemotely(executionInfo) + && !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE_CACHE_UPLOAD))); + } + + public static boolean shouldUploadLocalResultsToRemoteCache( + RemoteOptions remoteOptions, @Nullable Spawn spawn) { + ImmutableMap executionInfo = null; + if (spawn != null) { + executionInfo = spawn.getExecutionInfo(); + } + return shouldUploadLocalResultsToRemoteCache(remoteOptions, executionInfo); } public static boolean shouldUploadLocalResultsToDiskCache( @@ -614,13 +627,11 @@ public static boolean shouldUploadLocalResultsToCombinedDisk( RemoteOptions remoteOptions, @Nullable Spawn spawn) { if (remoteOptions.incompatibleRemoteResultsIgnoreDisk) { // If --incompatible_remote_results_ignore_disk is set, we treat the disk cache part as local - // cache. Actions - // which are tagged with `no-remote-cache` can still hit the disk cache. - return spawn == null || Spawns.mayBeCached(spawn); + // cache. Actions which are tagged with `no-remote-cache` can still hit the disk cache. + return shouldUploadLocalResultsToDiskCache(remoteOptions, spawn); } else { // Otherwise, it's treated as a remote cache and disabled for `no-remote-cache`. - return remoteOptions.remoteUploadLocalResults - && (spawn == null || Spawns.mayBeCachedRemotely(spawn)); + return shouldUploadLocalResultsToRemoteCache(remoteOptions, spawn); } } } diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index 286e508c07c3f0..e84635ce7ebc63 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_test.sh @@ -1900,6 +1900,42 @@ EOF expect_not_log "remote cache hit" } +function test_tag_no_remote_cache_upload() { + mkdir -p a + cat > a/BUILD <<'EOF' +genrule( + name = "foo", + srcs = [], + outs = ["foo.txt"], + cmd = "echo \"foo\" > \"$@\"", +) +EOF + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --modify_execution_info=.*=+no-remote-cache-upload \ + //a:foo >& $TEST_log || "Failed to build //a:foo" + + remote_ac_files="$(count_remote_ac_files)" + [[ "$remote_ac_files" == 0 ]] || fail "Expected 0 remote action cache entries, not $remote_ac_files" + + # populate the cache + bazel clean + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + //a:foo >& $TEST_log || "Failed to build //a:foo" + + bazel clean + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --modify_execution_info=.*=+no-remote-cache-upload \ + //a:foo >& $TEST_log || "Failed to build //a:foo" + + expect_log "remote cache hit" +} + function test_tag_no_remote_cache_for_disk_cache() { mkdir -p a cat > a/BUILD <<'EOF'