Skip to content

Commit bfc2413

Browse files
brentleyjonescoeuvre
brentleyjones
and
coeuvre
authored
5.x: Remote: Ignore blobs referenced in BEP if the generating action cannot be cached remotely. (bazelbuild#14389)
* 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 bazelbuild#14338. PiperOrigin-RevId: 414708472 (cherry picked from commit 0d7d44d) * Remote: Ignore blobs referenced in BEP if the generating action cannot be cached remotely. Introduces new flag --incompatible_remote_build_event_upload_respect_no_cache which when set to true, remote uploader won't upload blobs referenced in BEP if the generating action cannot be cached remotely. Part of bazelbuild#14338. Closes bazelbuild#14338. PiperOrigin-RevId: 414721139 (cherry picked from commit 2ec457d) Co-authored-by: chiwang <[email protected]>
1 parent 24a340a commit bfc2413

File tree

11 files changed

+361
-50
lines changed

11 files changed

+361
-50
lines changed

‎src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,9 @@ public enum WorkerProtocolFormat {
194194
/** Disables remote caching of a spawn. Note: does not disable remote execution */
195195
public static final String NO_REMOTE_CACHE = "no-remote-cache";
196196

197+
/** Disables upload part of remote caching of a spawn. Note: does not disable remote execution */
198+
public static final String NO_REMOTE_CACHE_UPLOAD = "no-remote-cache-upload";
199+
197200
/** Disables remote execution of a spawn. Note: does not disable remote caching */
198201
public static final String NO_REMOTE_EXEC = "no-remote-exec";
199202

‎src/main/java/com/google/devtools/build/lib/actions/Spawns.java‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,33 @@
1919
import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code;
2020
import java.io.IOException;
2121
import java.time.Duration;
22+
import java.util.Map;
2223

2324
/** Helper methods relating to implementations of {@link Spawn}. */
2425
public final class Spawns {
2526
private Spawns() {}
2627

27-
/**
28-
* Returns {@code true} if the result of {@code spawn} may be cached.
29-
*/
28+
/** Returns {@code true} if the result of {@code spawn} may be cached. */
3029
public static boolean mayBeCached(Spawn spawn) {
31-
return !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_CACHE)
32-
&& !spawn.getExecutionInfo().containsKey(ExecutionRequirements.LOCAL);
30+
return mayBeCached(spawn.getExecutionInfo());
31+
}
32+
33+
/** Returns {@code true} if the result of {@code spawn} may be cached. */
34+
public static boolean mayBeCached(Map<String, String> executionInfo) {
35+
return !executionInfo.containsKey(ExecutionRequirements.NO_CACHE)
36+
&& !executionInfo.containsKey(ExecutionRequirements.LOCAL);
3337
}
3438

3539
/** Returns {@code true} if the result of {@code spawn} may be cached remotely. */
3640
public static boolean mayBeCachedRemotely(Spawn spawn) {
37-
return mayBeCached(spawn)
38-
&& !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE)
39-
&& !spawn.getExecutionInfo().containsKey(ExecutionRequirements.NO_REMOTE_CACHE);
41+
return mayBeCachedRemotely(spawn.getExecutionInfo());
42+
}
43+
44+
/** Returns {@code true} if the result of {@code spawn} may be cached remotely. */
45+
public static boolean mayBeCachedRemotely(Map<String, String> executionInfo) {
46+
return mayBeCached(executionInfo)
47+
&& !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE)
48+
&& !executionInfo.containsKey(ExecutionRequirements.NO_REMOTE_CACHE);
4049
}
4150

4251
/** Returns {@code true} if {@code spawn} may be executed remotely. */

‎src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import build.bazel.remote.execution.v2.RequestMetadata;
2323
import com.google.common.base.Preconditions;
2424
import com.google.common.collect.ImmutableSet;
25+
import com.google.common.collect.Sets;
2526
import com.google.common.util.concurrent.ListenableFuture;
2627
import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile;
2728
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
@@ -65,6 +66,9 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted
6566
private final AtomicBoolean shutdown = new AtomicBoolean();
6667
private final Scheduler scheduler;
6768

69+
private final Set<Path> omittedFiles = Sets.newConcurrentHashSet();
70+
private final Set<Path> omittedTreeRoots = Sets.newConcurrentHashSet();
71+
6872
ByteStreamBuildEventArtifactUploader(
6973
Executor executor,
7074
ExtendedEventHandler reporter,
@@ -83,6 +87,14 @@ class ByteStreamBuildEventArtifactUploader extends AbstractReferenceCounted
8387
this.scheduler = Schedulers.from(executor);
8488
}
8589

90+
public void omitFile(Path file) {
91+
omittedFiles.add(file);
92+
}
93+
94+
public void omitTree(Path treeRoot) {
95+
omittedTreeRoots.add(treeRoot);
96+
}
97+
8698
/** Returns {@code true} if Bazel knows that the file is stored on a remote system. */
8799
private static boolean isRemoteFile(Path file) {
88100
return file.getFileSystem() instanceof RemoteActionFileSystem
@@ -124,10 +136,21 @@ public boolean isRemote() {
124136
* Collects metadata for {@code file}. Depending on the underlying filesystem used this method
125137
* might do I/O.
126138
*/
127-
private static PathMetadata readPathMetadata(Path file) throws IOException {
139+
private PathMetadata readPathMetadata(Path file) throws IOException {
128140
if (file.isDirectory()) {
129141
return new PathMetadata(file, /* digest= */ null, /* directory= */ true, /* remote= */ false);
130142
}
143+
if (omittedFiles.contains(file)) {
144+
return new PathMetadata(file, /*digest=*/ null, /*directory=*/ false, /*remote=*/ false);
145+
}
146+
147+
for (Path treeRoot : omittedTreeRoots) {
148+
if (file.startsWith(treeRoot)) {
149+
omittedFiles.add(file);
150+
return new PathMetadata(file, /*digest=*/ null, /*directory=*/ false, /*remote=*/ false);
151+
}
152+
}
153+
131154
DigestUtil digestUtil = new DigestUtil(file.getFileSystem().getDigestFunction());
132155
Digest digest = digestUtil.compute(file);
133156
return new PathMetadata(file, digest, /* directory= */ false, isRemoteFile(file));
@@ -248,7 +271,7 @@ private Single<PathConverter> upload(Set<Path> files) {
248271
.collect(Collectors.toList())
249272
.flatMap(paths -> queryRemoteCache(remoteCache, context, paths))
250273
.flatMap(paths -> uploadLocalFiles(remoteCache, context, paths))
251-
.map(paths -> new PathConverterImpl(remoteServerInstanceName, paths)),
274+
.map(paths -> new PathConverterImpl(remoteServerInstanceName, paths, omittedFiles)),
252275
RemoteCache::release);
253276
}
254277

@@ -280,8 +303,10 @@ private static class PathConverterImpl implements PathConverter {
280303
private final String remoteServerInstanceName;
281304
private final Map<Path, Digest> pathToDigest;
282305
private final Set<Path> skippedPaths;
306+
private final Set<Path> localPaths;
283307

284-
PathConverterImpl(String remoteServerInstanceName, List<PathMetadata> uploads) {
308+
PathConverterImpl(
309+
String remoteServerInstanceName, List<PathMetadata> uploads, Set<Path> localPaths) {
285310
Preconditions.checkNotNull(uploads);
286311
this.remoteServerInstanceName = remoteServerInstanceName;
287312
pathToDigest = new HashMap<>(uploads.size());
@@ -296,11 +321,17 @@ private static class PathConverterImpl implements PathConverter {
296321
}
297322
}
298323
this.skippedPaths = skippedPaths.build();
324+
this.localPaths = localPaths;
299325
}
300326

301327
@Override
302328
public String apply(Path path) {
303329
Preconditions.checkNotNull(path);
330+
331+
if (localPaths.contains(path)) {
332+
return String.format("file://%s", path.getPathString());
333+
}
334+
304335
Digest digest = pathToDigest.get(path);
305336
if (digest == null) {
306337
if (skippedPaths.contains(path)) {

‎src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderFactory.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,14 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.remote;
1515

16+
import static com.google.common.base.Preconditions.checkState;
17+
1618
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
1719
import com.google.devtools.build.lib.events.ExtendedEventHandler;
1820
import com.google.devtools.build.lib.runtime.BuildEventArtifactUploaderFactory;
1921
import com.google.devtools.build.lib.runtime.CommandEnvironment;
2022
import java.util.concurrent.Executor;
23+
import javax.annotation.Nullable;
2124

2225
/** A factory for {@link ByteStreamBuildEventArtifactUploader}. */
2326
class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactUploaderFactory {
@@ -30,6 +33,8 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactU
3033
private final String buildRequestId;
3134
private final String commandId;
3235

36+
@Nullable private ByteStreamBuildEventArtifactUploader uploader;
37+
3338
ByteStreamBuildEventArtifactUploaderFactory(
3439
Executor executor,
3540
ExtendedEventHandler reporter,
@@ -49,13 +54,21 @@ class ByteStreamBuildEventArtifactUploaderFactory implements BuildEventArtifactU
4954

5055
@Override
5156
public BuildEventArtifactUploader create(CommandEnvironment env) {
52-
return new ByteStreamBuildEventArtifactUploader(
53-
executor,
54-
reporter,
55-
verboseFailures,
56-
remoteCache.retain(),
57-
remoteServerInstanceName,
58-
buildRequestId,
59-
commandId);
57+
checkState(uploader == null, "Already created");
58+
uploader =
59+
new ByteStreamBuildEventArtifactUploader(
60+
executor,
61+
reporter,
62+
verboseFailures,
63+
remoteCache.retain(),
64+
remoteServerInstanceName,
65+
buildRequestId,
66+
commandId);
67+
return uploader;
68+
}
69+
70+
@Nullable
71+
public ByteStreamBuildEventArtifactUploader get() {
72+
return uploader;
6073
}
6174
}

‎src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,19 @@ public boolean shouldAcceptCachedResult(Spawn spawn) {
356356
}
357357
}
358358

359+
public static boolean shouldUploadLocalResults(
360+
RemoteOptions remoteOptions, @Nullable Map<String, String> executionInfo) {
361+
if (useRemoteCache(remoteOptions)) {
362+
if (useDiskCache(remoteOptions)) {
363+
return shouldUploadLocalResultsToCombinedDisk(remoteOptions, executionInfo);
364+
} else {
365+
return shouldUploadLocalResultsToRemoteCache(remoteOptions, executionInfo);
366+
}
367+
} else {
368+
return shouldUploadLocalResultsToDiskCache(remoteOptions, executionInfo);
369+
}
370+
}
371+
359372
/**
360373
* Returns {@code true} if the local results of the {@code spawn} should be uploaded to remote
361374
* cache.
@@ -365,15 +378,7 @@ public boolean shouldUploadLocalResults(Spawn spawn) {
365378
return false;
366379
}
367380

368-
if (useRemoteCache(remoteOptions)) {
369-
if (useDiskCache(remoteOptions)) {
370-
return shouldUploadLocalResultsToCombinedDisk(remoteOptions, spawn);
371-
} else {
372-
return shouldUploadLocalResultsToRemoteCache(remoteOptions, spawn);
373-
}
374-
} else {
375-
return shouldUploadLocalResultsToDiskCache(remoteOptions, spawn);
376-
}
381+
return shouldUploadLocalResults(remoteOptions, spawn.getExecutionInfo());
377382
}
378383

379384
/** Returns {@code true} if the spawn may be executed remotely. */

‎src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java‎

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.google.common.util.concurrent.ListeningScheduledExecutorService;
3131
import com.google.common.util.concurrent.MoreExecutors;
3232
import com.google.common.util.concurrent.ThreadFactoryBuilder;
33+
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
3334
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
3435
import com.google.devtools.build.lib.actions.ActionGraph;
3536
import com.google.devtools.build.lib.actions.ActionInput;
@@ -111,6 +112,7 @@
111112
import java.util.concurrent.LinkedBlockingQueue;
112113
import java.util.concurrent.ThreadFactory;
113114
import java.util.concurrent.ThreadPoolExecutor;
115+
import javax.annotation.Nullable;
114116

115117
/** RemoteModule provides distributed cache and remote execution for Bazel. */
116118
public final class RemoteModule extends BlazeModule {
@@ -126,7 +128,7 @@ public final class RemoteModule extends BlazeModule {
126128

127129
private RemoteActionContextProvider actionContextProvider;
128130
private RemoteActionInputFetcher actionInputFetcher;
129-
private RemoteOutputsMode remoteOutputsMode;
131+
private RemoteOptions remoteOptions;
130132
private RemoteOutputService remoteOutputService;
131133

132134
private ChannelFactory channelFactory =
@@ -244,15 +246,15 @@ private void initHttpAndDiskCache(
244246
public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
245247
Preconditions.checkState(actionContextProvider == null, "actionContextProvider must be null");
246248
Preconditions.checkState(actionInputFetcher == null, "actionInputFetcher must be null");
247-
Preconditions.checkState(remoteOutputsMode == null, "remoteOutputsMode must be null");
249+
Preconditions.checkState(remoteOptions == null, "remoteOptions must be null");
248250

249251
RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class);
250252
if (remoteOptions == null) {
251253
// Quit if no supported command is being used. See getCommandOptions for details.
252254
return;
253255
}
254256

255-
remoteOutputsMode = remoteOptions.remoteOutputsMode;
257+
this.remoteOptions = remoteOptions;
256258

257259
AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
258260
DigestHashFunction hashFn = env.getRuntime().getFileSystem().getDigestFunction();
@@ -705,8 +707,8 @@ public void afterAnalysis(
705707
AnalysisResult analysisResult) {
706708
// The actionContextProvider may be null if remote execution is disabled or if there was an
707709
// error during initialization.
708-
if (remoteOutputsMode != null
709-
&& remoteOutputsMode.downloadToplevelOutputsOnly()
710+
if (remoteOptions != null
711+
&& remoteOptions.remoteOutputsMode.downloadToplevelOutputsOnly()
710712
&& actionContextProvider != null) {
711713
boolean isTestCommand = env.getCommandName().equals("test");
712714
TopLevelArtifactContext artifactContext = request.getTopLevelArtifactContext();
@@ -726,6 +728,41 @@ public void afterAnalysis(
726728
}
727729
actionContextProvider.setFilesToDownload(ImmutableSet.copyOf(filesToDownload));
728730
}
731+
732+
if (remoteOptions != null && remoteOptions.incompatibleRemoteBuildEventUploadRespectNoCache) {
733+
parseNoCacheOutputs(analysisResult);
734+
}
735+
}
736+
737+
private void parseNoCacheOutputs(AnalysisResult analysisResult) {
738+
if (actionContextProvider == null || actionContextProvider.getRemoteCache() == null) {
739+
return;
740+
}
741+
742+
ByteStreamBuildEventArtifactUploader uploader = buildEventArtifactUploaderFactoryDelegate.get();
743+
if (uploader == null) {
744+
return;
745+
}
746+
747+
for (ConfiguredTarget configuredTarget : analysisResult.getTargetsToBuild()) {
748+
if (configuredTarget instanceof RuleConfiguredTarget) {
749+
RuleConfiguredTarget ruleConfiguredTarget = (RuleConfiguredTarget) configuredTarget;
750+
for (ActionAnalysisMetadata action : ruleConfiguredTarget.getActions()) {
751+
boolean uploadLocalResults =
752+
RemoteExecutionService.shouldUploadLocalResults(
753+
remoteOptions, action.getExecutionInfo());
754+
if (!uploadLocalResults) {
755+
for (Artifact output : action.getOutputs()) {
756+
if (output.isTreeArtifact()) {
757+
uploader.omitTree(output.getPath());
758+
} else {
759+
uploader.omitFile(output.getPath());
760+
}
761+
}
762+
}
763+
}
764+
}
765+
}
729766
}
730767

731768
// This is a short-term fix for top-level outputs that are symlinks. Unfortunately, we cannot
@@ -829,7 +866,7 @@ public void afterCommand() throws AbruptExitException {
829866
remoteDownloaderSupplier.set(null);
830867
actionContextProvider = null;
831868
actionInputFetcher = null;
832-
remoteOutputsMode = null;
869+
remoteOptions = null;
833870
remoteOutputService = null;
834871

835872
if (failure != null) {
@@ -873,7 +910,7 @@ public void registerActionContexts(
873910
@Override
874911
public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorBuilder builder) {
875912
Preconditions.checkState(actionInputFetcher == null, "actionInputFetcher must be null");
876-
Preconditions.checkNotNull(remoteOutputsMode, "remoteOutputsMode must not be null");
913+
Preconditions.checkNotNull(remoteOptions, "remoteOptions must not be null");
877914

878915
if (actionContextProvider == null) {
879916
return;
@@ -897,7 +934,7 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB
897934
@Override
898935
public OutputService getOutputService() {
899936
Preconditions.checkState(remoteOutputService == null, "remoteOutputService must be null");
900-
if (remoteOutputsMode != null && !remoteOutputsMode.downloadAllOutputs()) {
937+
if (remoteOptions != null && !remoteOptions.remoteOutputsMode.downloadAllOutputs()) {
901938
remoteOutputService = new RemoteOutputService();
902939
}
903940
return remoteOutputService;
@@ -913,13 +950,21 @@ public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command)
913950
private static class BuildEventArtifactUploaderFactoryDelegate
914951
implements BuildEventArtifactUploaderFactory {
915952

916-
private volatile BuildEventArtifactUploaderFactory uploaderFactory;
953+
@Nullable private ByteStreamBuildEventArtifactUploaderFactory uploaderFactory;
917954

918-
public void init(BuildEventArtifactUploaderFactory uploaderFactory) {
955+
public void init(ByteStreamBuildEventArtifactUploaderFactory uploaderFactory) {
919956
Preconditions.checkState(this.uploaderFactory == null);
920957
this.uploaderFactory = uploaderFactory;
921958
}
922959

960+
@Nullable
961+
public ByteStreamBuildEventArtifactUploader get() {
962+
if (uploaderFactory == null) {
963+
return null;
964+
}
965+
return uploaderFactory.get();
966+
}
967+
923968
public void reset() {
924969
this.uploaderFactory = null;
925970
}

‎src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,12 @@ public void close() {
7575
public ListenableFuture<Void> uploadFile(
7676
RemoteActionExecutionContext context, Digest digest, Path file) {
7777
ListenableFuture<Void> future = diskCache.uploadFile(context, digest, file);
78-
if (options.isRemoteExecutionEnabled()
78+
79+
boolean uploadForSpawn = context.getSpawn() != null;
80+
// If not upload for spawn e.g. for build event artifacts, we always upload files to remote
81+
// cache.
82+
if (!uploadForSpawn
83+
|| options.isRemoteExecutionEnabled()
7984
|| shouldUploadLocalResultsToRemoteCache(options, context.getSpawn())) {
8085
future =
8186
Futures.transformAsync(

0 commit comments

Comments
 (0)