Skip to content

Commit 164705b

Browse files
sluongngcopybara-github
sluongng
authored and
copybara-github
committed
Conditionally set output_paths based on Remote Executor capabilities
This is a follow up to #18269, toward the discussion in #18202. Bump the Remote API supported version to v2.1. Based on the Capability of the Remote Executor, either use output_paths field or the legacy fields output_files and output_directories. Closes #18270. PiperOrigin-RevId: 675060530 Change-Id: If08975b48696e06da0da91898ba2dd4b1c6677d2
1 parent 1f2225b commit 164705b

15 files changed

Lines changed: 423 additions & 113 deletions

File tree

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,21 @@ public class ApiVersion implements Comparable<ApiVersion> {
2323
public final int patch;
2424
public final String prerelease;
2525

26+
// The version of the Remote Execution API that Bazel supports initially.
27+
public static final ApiVersion twoPointZero =
28+
new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(0).build());
29+
// The version of the Remote Execution API that starts supporting the
30+
// Command.output_paths and ActionResult.output_symlinks fields.
31+
public static final ApiVersion twoPointOne =
32+
new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(1).build());
33+
// The latest version of the Remote Execution API that Bazel is compatible with.
34+
public static final ApiVersion twoPointTwo =
35+
new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(2).build());
36+
2637
// The current lowest/highest versions (inclusive) of the Remote Execution API that Bazel
2738
// supports. These fields will need to be updated together with all version changes.
28-
public static final ApiVersion low =
29-
new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(0).build());
30-
public static final ApiVersion high =
31-
new ApiVersion(SemVer.newBuilder().setMajor(2).setMinor(0).build());
39+
public static final ApiVersion low = twoPointZero;
40+
public static final ApiVersion high = twoPointTwo;
3241

3342
public ApiVersion(int major, int minor, int patch, String prerelease) {
3443
this.major = major;

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import build.bazel.remote.execution.v2.FindMissingBlobsResponse;
3030
import build.bazel.remote.execution.v2.GetActionResultRequest;
3131
import build.bazel.remote.execution.v2.RequestMetadata;
32+
import build.bazel.remote.execution.v2.ServerCapabilities;
3233
import build.bazel.remote.execution.v2.UpdateActionResultRequest;
3334
import com.google.bytestream.ByteStreamGrpc;
3435
import com.google.bytestream.ByteStreamGrpc.ByteStreamStub;
@@ -264,6 +265,11 @@ public CacheCapabilities getCacheCapabilities() throws IOException {
264265
return channel.getServerCapabilities().getCacheCapabilities();
265266
}
266267

268+
@Override
269+
public ServerCapabilities getServerCapabilities() throws IOException {
270+
return channel.getServerCapabilities();
271+
}
272+
267273
@Override
268274
public ListenableFuture<String> getAuthority() {
269275
return channel.withChannelFuture(ch -> Futures.immediateFuture(ch.authority()));

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import build.bazel.remote.execution.v2.ActionResult;
2727
import build.bazel.remote.execution.v2.CacheCapabilities;
2828
import build.bazel.remote.execution.v2.Digest;
29+
import build.bazel.remote.execution.v2.ServerCapabilities;
2930
import com.google.common.collect.ImmutableSet;
3031
import com.google.common.collect.Iterables;
3132
import com.google.common.flogger.GoogleLogger;
@@ -125,6 +126,13 @@ public ListenableFuture<String> getRemoteAuthority() {
125126
return remoteCacheClient.getAuthority();
126127
}
127128

129+
public ServerCapabilities getRemoteServerCapabilities() throws IOException {
130+
if (remoteCacheClient == null) {
131+
return ServerCapabilities.getDefaultInstance();
132+
}
133+
return remoteCacheClient.getServerCapabilities();
134+
}
135+
128136
/**
129137
* Class to keep track of which cache (disk or remote) a given [cached] ActionResult comes from.
130138
*/

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

Lines changed: 96 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import static com.google.devtools.build.lib.remote.util.Utils.waitForBulkTransfer;
3030
import static com.google.devtools.build.lib.util.StringUtil.reencodeExternalToInternal;
3131
import static com.google.devtools.build.lib.util.StringUtil.reencodeInternalToExternal;
32+
import static java.util.Collections.min;
3233

3334
import build.bazel.remote.execution.v2.Action;
3435
import build.bazel.remote.execution.v2.ActionResult;
@@ -156,6 +157,7 @@
156157
import java.util.concurrent.Semaphore;
157158
import java.util.concurrent.atomic.AtomicBoolean;
158159
import java.util.concurrent.atomic.AtomicReference;
160+
import java.util.stream.Stream;
159161
import javax.annotation.Nullable;
160162

161163
/**
@@ -196,6 +198,8 @@ public class RemoteExecutionService {
196198
@Nullable private final Scrubber scrubber;
197199
private final Set<Digest> knownMissingCasDigests;
198200

201+
private Boolean useOutputPaths;
202+
199203
public RemoteExecutionService(
200204
Executor executor,
201205
Reporter reporter,
@@ -242,32 +246,39 @@ public RemoteExecutionService(
242246
}
243247

244248
private Command buildCommand(
249+
boolean useOutputPaths,
245250
Collection<? extends ActionInput> outputs,
246251
List<String> arguments,
247252
ImmutableMap<String, String> env,
248253
@Nullable Platform platform,
249254
RemotePathResolver remotePathResolver,
250255
@Nullable SpawnScrubber spawnScrubber) {
251256
Command.Builder command = Command.newBuilder();
252-
ArrayList<String> outputFiles = new ArrayList<>();
253-
ArrayList<String> outputDirectories = new ArrayList<>();
254-
ArrayList<String> outputPaths = new ArrayList<>();
255-
for (ActionInput output : outputs) {
256-
String pathString =
257-
reencodeInternalToExternal(remotePathResolver.localPathToOutputPath(output));
258-
if (output.isDirectory()) {
259-
outputDirectories.add(pathString);
260-
} else {
261-
outputFiles.add(pathString);
257+
if (useOutputPaths) {
258+
var outputPaths = new ArrayList<String>();
259+
for (ActionInput output : outputs) {
260+
String pathString =
261+
reencodeInternalToExternal(remotePathResolver.localPathToOutputPath(output));
262+
outputPaths.add(pathString);
262263
}
263-
outputPaths.add(pathString);
264+
Collections.sort(outputPaths);
265+
command.addAllOutputPaths(outputPaths);
266+
} else {
267+
var outputFiles = new ArrayList<String>();
268+
var outputDirectories = new ArrayList<String>();
269+
for (ActionInput output : outputs) {
270+
String pathString =
271+
reencodeInternalToExternal(remotePathResolver.localPathToOutputPath(output));
272+
if (output.isDirectory()) {
273+
outputDirectories.add(pathString);
274+
} else {
275+
outputFiles.add(pathString);
276+
}
277+
}
278+
Collections.sort(outputFiles);
279+
Collections.sort(outputDirectories);
280+
command.addAllOutputFiles(outputFiles).addAllOutputDirectories(outputDirectories);
264281
}
265-
Collections.sort(outputFiles);
266-
Collections.sort(outputDirectories);
267-
Collections.sort(outputPaths);
268-
command.addAllOutputFiles(outputFiles);
269-
command.addAllOutputDirectories(outputDirectories);
270-
command.addAllOutputPaths(outputPaths);
271282

272283
if (platform != null) {
273284
command.setPlatform(platform);
@@ -542,6 +553,65 @@ private void maybeReleaseRemoteActionBuildingSemaphore() {
542553
remoteActionBuildingSemaphore.release();
543554
}
544555

556+
private boolean useOutputPaths() {
557+
if (this.useOutputPaths == null) {
558+
initUseOutputPaths();
559+
}
560+
return this.useOutputPaths;
561+
}
562+
563+
private synchronized void initUseOutputPaths() {
564+
// If this has already been initialized, return
565+
if (this.useOutputPaths != null) {
566+
return;
567+
}
568+
ApiVersion serverHighestVersion = null;
569+
try {
570+
// If both Remote Executor and Remote Cache are configured,
571+
// use the highest version supported by both.
572+
573+
ClientApiVersion.ServerSupportedStatus executorSupportStatus = null;
574+
if (remoteExecutor != null) {
575+
var serverCapabilities = remoteExecutor.getServerCapabilities();
576+
if (serverCapabilities != null) {
577+
executorSupportStatus =
578+
ClientApiVersion.current.checkServerSupportedVersions(serverCapabilities);
579+
}
580+
}
581+
582+
ClientApiVersion.ServerSupportedStatus cacheSupportStatus = null;
583+
if (remoteCache != null) {
584+
var serverCapabilities = remoteCache.getRemoteServerCapabilities();
585+
if (serverCapabilities != null) {
586+
cacheSupportStatus =
587+
ClientApiVersion.current.checkServerSupportedVersions(serverCapabilities);
588+
}
589+
}
590+
591+
ApiVersion executorHighestVersion = null;
592+
if (executorSupportStatus != null && executorSupportStatus.isSupported()) {
593+
executorHighestVersion = executorSupportStatus.getHighestSupportedVersion();
594+
}
595+
596+
ApiVersion cacheHighestVersion = null;
597+
if (cacheSupportStatus != null && cacheSupportStatus.isSupported()) {
598+
cacheHighestVersion = cacheSupportStatus.getHighestSupportedVersion();
599+
}
600+
601+
if (executorHighestVersion != null && cacheHighestVersion != null) {
602+
serverHighestVersion = min(ImmutableList.of(executorHighestVersion, cacheHighestVersion));
603+
} else if (executorHighestVersion != null) {
604+
serverHighestVersion = executorHighestVersion;
605+
} else if (cacheHighestVersion != null) {
606+
serverHighestVersion = cacheHighestVersion;
607+
}
608+
} catch (IOException e) {
609+
// Intentionally ignored.
610+
}
611+
this.useOutputPaths =
612+
serverHighestVersion == null || serverHighestVersion.compareTo(ApiVersion.twoPointOne) >= 0;
613+
}
614+
545615
/** Creates a new {@link RemoteAction} instance from spawn. */
546616
public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context)
547617
throws IOException, ExecException, ForbiddenActionInputException, InterruptedException {
@@ -570,6 +640,7 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context
570640

571641
Command command =
572642
buildCommand(
643+
useOutputPaths(),
573644
spawn.getOutputFiles(),
574645
spawn.getArguments(),
575646
spawn.getEnvironment(),
@@ -1540,8 +1611,15 @@ public SpawnResult waitForAndReuseOutputs(RemoteAction action, LocalExecution pr
15401611
Map<Path, Path> realToTmpPath = new HashMap<>();
15411612
ByteString inMemoryOutputContent = null;
15421613
String inMemoryOutputPath = null;
1614+
var outputPathsList =
1615+
useOutputPaths()
1616+
? action.getCommand().getOutputPathsList()
1617+
: Stream.concat(
1618+
action.getCommand().getOutputFilesList().stream(),
1619+
action.getCommand().getOutputDirectoriesList().stream())
1620+
.toList();
15431621
try {
1544-
for (String output : action.getCommand().getOutputPathsList()) {
1622+
for (String output : outputPathsList) {
15451623
String reencodedOutput = reencodeExternalToInternal(output);
15461624
Path sourcePath =
15471625
previousExecution.action.getRemotePathResolver().outputPathToLocalPath(reencodedOutput);

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import build.bazel.remote.execution.v2.ActionResult;
1919
import build.bazel.remote.execution.v2.CacheCapabilities;
2020
import build.bazel.remote.execution.v2.Digest;
21+
import build.bazel.remote.execution.v2.ServerCapabilities;
2122
import com.google.common.base.Preconditions;
2223
import com.google.common.util.concurrent.ListenableFuture;
2324
import com.google.devtools.build.lib.vfs.Path;
@@ -34,6 +35,8 @@
3435
public interface RemoteCacheClient extends MissingDigestsFinder {
3536
CacheCapabilities getCacheCapabilities() throws IOException;
3637

38+
ServerCapabilities getServerCapabilities() throws IOException;
39+
3740
ListenableFuture<String> getAuthority();
3841

3942
/**

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import build.bazel.remote.execution.v2.ActionResult;
1818
import build.bazel.remote.execution.v2.CacheCapabilities;
1919
import build.bazel.remote.execution.v2.Digest;
20+
import build.bazel.remote.execution.v2.ServerCapabilities;
2021
import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy;
2122
import com.google.auth.Credentials;
2223
import com.google.common.collect.ImmutableList;
@@ -611,6 +612,11 @@ public CacheCapabilities getCacheCapabilities() {
611612
.build();
612613
}
613614

615+
@Override
616+
public ServerCapabilities getServerCapabilities() {
617+
return ServerCapabilities.newBuilder().setCacheCapabilities(getCacheCapabilities()).build();
618+
}
619+
614620
@Override
615621
public ListenableFuture<String> getAuthority() {
616622
return Futures.immediateFuture("");

0 commit comments

Comments
 (0)