From 224e157948c9d9740886c1fa580ee9e803f1ce23 Mon Sep 17 00:00:00 2001 From: Son Luong Ngoc Date: Wed, 22 Jul 2026 06:48:35 -0700 Subject: [PATCH] remote: add gRPC service config support (https://github.com/bazelbuild/bazel/pull/29912) Remote cache users currently have one `--remote_timeout` value that applies across remote gRPC calls. That is a poor fit because fast control-plane RPCs such as action cache lookups benefit from short deadlines, while ByteStream reads and writes may need much longer deadlines for large blobs. Setting the flag too low can make uploads or downloads fail consistently; setting it too high makes stale requests wait too long before Bazel retries. This follows the direction discussed in https://github.com/bazelbuild/bazel/discussions/26741. This PR keeps the default user experience unchanged while moving remote gRPC timeout handling onto service config, then adds an opt-in advanced override. It is structured as two commits: - `remote: drive gRPC deadlines from service config` generates the same timeout policy Bazel previously applied from `--remote_timeout`, installs it on remote gRPC channels, and removes per-stub deadline plumbing. - `remote: accept gRPC service config files` adds `--remote_grpc_service_config` for a user-owned JSON file. The initial supported schema is intentionally restricted to `methodConfig`, `name`, and `timeout`; unsupported fields such as retry, hedging, load balancing, and health checking are rejected so Bazel can expand support deliberately later. Closes #29912. PiperOrigin-RevId: 952089183 Change-Id: I0101d0cafb9e7593a16c4106372d288c8ef0237c (cherry picked from commit 3510aded10620dd557521ff0c74c0590e1be145f) --- .../build/lib/authandtls/GoogleAuthUtils.java | 7 +- .../BazelBuildEventServiceModule.java | 3 +- .../google/devtools/build/lib/remote/BUILD | 2 + .../build/lib/remote/ByteStreamUploader.java | 26 +-- .../build/lib/remote/ChannelFactory.java | 7 +- .../GoogleChannelConnectionFactory.java | 6 +- .../build/lib/remote/GrpcCacheClient.java | 11 +- .../lib/remote/RemoteGrpcServiceConfig.java | 153 ++++++++++++++++++ .../build/lib/remote/RemoteModule.java | 25 ++- .../lib/remote/RemoteServerCapabilities.java | 7 +- .../downloader/GrpcRemoteDownloader.java | 5 +- .../lib/remote/options/RemoteOptions.java | 14 ++ src/main/protobuf/failure_details.proto | 1 + .../lib/remote/ByteStreamUploaderTest.java | 25 --- .../remote/RemoteGrpcServiceConfigTest.java | 134 +++++++++++++++ .../build/lib/remote/RemoteModuleTest.java | 115 ++++++++++++- .../remote/RemoteServerCapabilitiesTest.java | 4 +- 17 files changed, 465 insertions(+), 80 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/remote/RemoteGrpcServiceConfig.java create mode 100644 src/test/java/com/google/devtools/build/lib/remote/RemoteGrpcServiceConfigTest.java diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java b/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java index b34cc4a8d78b6c..2071289832f1f0 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java @@ -76,7 +76,8 @@ public static ManagedChannel newChannel( String target, String proxy, AuthAndTLSOptions options, - @Nullable List interceptors) + @Nullable List interceptors, + @Nullable Map serviceConfig) throws IOException { Preconditions.checkNotNull(target); Preconditions.checkNotNull(options); @@ -133,6 +134,10 @@ public static ManagedChannel newChannel( if (interceptors != null) { builder.intercept(interceptors); } + if (serviceConfig != null) { + builder.disableServiceConfigLookUp(); + builder.defaultServiceConfig(serviceConfig); + } if (sslContext != null) { builder.sslContext(sslContext); if (options.tlsAuthorityOverride != null) { diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModule.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModule.java index 9a6f33017651f0..7789dc6a0da3b8 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModule.java +++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BazelBuildEventServiceModule.java @@ -173,7 +173,8 @@ protected ManagedChannel newGrpcChannel(BackendConfig config) throws IOException config.besBackend(), config.besProxy(), config.authAndTLSOptions(), - /* interceptors= */ null); + /* interceptors= */ null, + /* serviceConfig= */ null); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index 77783b0b33d07a..c0dadc449d492f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -158,6 +158,7 @@ java_library( "//third_party:auth", "//third_party:caffeine", "//third_party:flogger", + "//third_party:gson", "//third_party:guava", "//third_party:jsr305", "//third_party:netty", @@ -169,6 +170,7 @@ java_library( "@googleapis//google/bytestream:bytestream_java_proto", "@googleapis//google/longrunning:longrunning_java_proto", "@googleapis//google/rpc:rpc_java_proto", + "@remoteapis//:build_bazel_remote_asset_v1_remote_asset_java_grpc", "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_grpc", "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto", "@remoteapis//:build_bazel_semver_semver_java_proto", diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamUploader.java index 05b733f745484f..b9ef16ff36405a 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamUploader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamUploader.java @@ -13,11 +13,9 @@ // limitations under the License. package com.google.devtools.build.lib.remote; -import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.util.concurrent.Futures.immediateVoidFuture; import static com.google.devtools.build.lib.remote.util.DigestUtil.isOldStyleDigestFunction; import static java.lang.String.format; -import static java.util.concurrent.TimeUnit.SECONDS; import build.bazel.remote.execution.v2.Digest; import build.bazel.remote.execution.v2.DigestFunction; @@ -65,7 +63,6 @@ final class ByteStreamUploader { private final String instanceName; private final ReferenceCountedChannel channel; private final CallCredentialsProvider callCredentialsProvider; - private final long callTimeoutSecs; private final RemoteRetrier retrier; private final DigestFunction.Value digestFunction; private final AtomicBoolean queryWriteStatusImplemented = new AtomicBoolean(true); @@ -79,23 +76,18 @@ final class ByteStreamUploader { * call. See the {@code ByteStream} service definition for details * @param channel the {@link io.grpc.Channel} to use for calls * @param callCredentialsProvider the credentials provider to use for authentication. - * @param callTimeoutSecs the timeout in seconds after which a {@code Write} gRPC call must be - * complete. The timeout resets between retries * @param retrier the {@link RemoteRetrier} whose backoff strategy to use for retry timings. */ ByteStreamUploader( @Nullable String instanceName, ReferenceCountedChannel channel, CallCredentialsProvider callCredentialsProvider, - long callTimeoutSecs, RemoteRetrier retrier, int maximumOpenFiles, DigestFunction.Value digestFunction) { - checkArgument(callTimeoutSecs > 0, "callTimeoutSecs must be gt 0."); this.instanceName = instanceName; this.channel = channel; this.callCredentialsProvider = callCredentialsProvider; - this.callTimeoutSecs = callTimeoutSecs; this.retrier = retrier; this.openedFilePermits = maximumOpenFiles != -1 ? new Semaphore(maximumOpenFiles) : null; this.digestFunction = digestFunction; @@ -180,14 +172,7 @@ private ListenableFuture startAsyncUpload( } } AsyncUpload newUpload = - new AsyncUpload( - context, - channel, - callCredentialsProvider, - callTimeoutSecs, - retrier, - resourceName, - chunker); + new AsyncUpload(context, channel, callCredentialsProvider, retrier, resourceName, chunker); ListenableFuture currUpload = newUpload.start(); currUpload.addListener( () -> { @@ -213,7 +198,6 @@ private final class AsyncUpload implements AsyncCallable { private final RemoteActionExecutionContext context; private final ReferenceCountedChannel channel; private final CallCredentialsProvider callCredentialsProvider; - private final long callTimeoutSecs; private final Retrier retrier; private final String resourceName; private final Chunker chunker; @@ -225,14 +209,12 @@ private final class AsyncUpload implements AsyncCallable { RemoteActionExecutionContext context, ReferenceCountedChannel channel, CallCredentialsProvider callCredentialsProvider, - long callTimeoutSecs, Retrier retrier, String resourceName, Chunker chunker) { this.context = context; this.channel = channel; this.callCredentialsProvider = callCredentialsProvider; - this.callTimeoutSecs = callTimeoutSecs; this.retrier = retrier; this.progressiveBackoff = new ProgressiveBackoff(retrier::newBackoff); this.resourceName = resourceName; @@ -314,16 +296,14 @@ private ByteStreamFutureStub bsFutureStub(Channel channel) { return ByteStreamGrpc.newFutureStub(channel) .withInterceptors( TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata())) - .withCallCredentials(callCredentialsProvider.getCallCredentials()) - .withDeadlineAfter(callTimeoutSecs, SECONDS); + .withCallCredentials(callCredentialsProvider.getCallCredentials()); } private ByteStreamStub bsAsyncStub(Channel channel) { return ByteStreamGrpc.newStub(channel) .withInterceptors( TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata())) - .withCallCredentials(callCredentialsProvider.getCallCredentials()) - .withDeadlineAfter(callTimeoutSecs, SECONDS); + .withCallCredentials(callCredentialsProvider.getCallCredentials()); } private ListenableFuture query() { diff --git a/src/main/java/com/google/devtools/build/lib/remote/ChannelFactory.java b/src/main/java/com/google/devtools/build/lib/remote/ChannelFactory.java index 216f6cd529197f..5adaa655576b4f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ChannelFactory.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ChannelFactory.java @@ -18,10 +18,15 @@ import io.grpc.ManagedChannel; import java.io.IOException; import java.util.List; +import java.util.Map; /** A factory interface for creating a {@link ManagedChannel}. */ public interface ChannelFactory { ManagedChannel newChannel( - String target, String proxy, AuthAndTLSOptions options, List interceptors) + String target, + String proxy, + AuthAndTLSOptions options, + List interceptors, + Map serviceConfig) throws IOException; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/GoogleChannelConnectionFactory.java b/src/main/java/com/google/devtools/build/lib/remote/GoogleChannelConnectionFactory.java index a63f182e532b33..3cd9ed5b7ce087 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/GoogleChannelConnectionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/remote/GoogleChannelConnectionFactory.java @@ -39,6 +39,7 @@ import io.reactivex.rxjava3.core.Single; import java.io.IOException; import java.util.List; +import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; @@ -61,6 +62,7 @@ public class GoogleChannelConnectionFactory private final Reporter reporter; @Nullable private final RemoteServerCapabilities remoteServerCapabilities; private final RemoteOptions remoteOptions; + private final Map serviceConfig; private final DigestFunction.Value digestFunction; private final ServerCapabilitiesRequirement requirement; @@ -69,6 +71,7 @@ public GoogleChannelConnectionFactory( String target, String proxy, RemoteOptions remoteOptions, + Map serviceConfig, AuthAndTLSOptions options, List interceptors, int maxConcurrency, @@ -91,6 +94,7 @@ public GoogleChannelConnectionFactory( this.reporter = reporter; this.remoteServerCapabilities = remoteServerCapabilities; this.remoteOptions = remoteOptions; + this.serviceConfig = serviceConfig; this.digestFunction = digestFunction; this.requirement = requirement; } @@ -98,7 +102,7 @@ public GoogleChannelConnectionFactory( @Override public Single create() { return Single.fromCallable( - () -> channelFactory.newChannel(target, proxy, options, interceptors)) + () -> channelFactory.newChannel(target, proxy, options, interceptors, serviceConfig)) .flatMap( channel -> { var serverCapabilitiesSingle = diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java index 3e4eac4cb39c20..203a7f933765c6 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java @@ -76,7 +76,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Set; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import javax.annotation.Nullable; @@ -117,7 +116,6 @@ public GrpcCacheClient( options.remoteInstanceName, channel, callCredentialsProvider, - options.remoteTimeout.toSeconds(), retrier, options.maximumOpenFiles, digestUtil.getDigestFunction()); @@ -150,8 +148,7 @@ private ContentAddressableStorageFutureStub casFutureStub( .withInterceptors( TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata()), new NetworkTimeInterceptor(context::getNetworkTime)) - .withCallCredentials(callCredentialsProvider.getCallCredentials()) - .withDeadlineAfter(options.remoteTimeout.toSeconds(), TimeUnit.SECONDS); + .withCallCredentials(callCredentialsProvider.getCallCredentials()); } private ByteStreamStub bsAsyncStub(RemoteActionExecutionContext context, Channel channel) { @@ -159,8 +156,7 @@ private ByteStreamStub bsAsyncStub(RemoteActionExecutionContext context, Channel .withInterceptors( TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata()), new NetworkTimeInterceptor(context::getNetworkTime)) - .withCallCredentials(callCredentialsProvider.getCallCredentials()) - .withDeadlineAfter(options.remoteTimeout.toSeconds(), TimeUnit.SECONDS); + .withCallCredentials(callCredentialsProvider.getCallCredentials()); } private ActionCacheFutureStub acFutureStub( @@ -169,8 +165,7 @@ private ActionCacheFutureStub acFutureStub( .withInterceptors( TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata()), new NetworkTimeInterceptor(context::getNetworkTime)) - .withCallCredentials(callCredentialsProvider.getCallCredentials()) - .withDeadlineAfter(options.remoteTimeout.toSeconds(), TimeUnit.SECONDS); + .withCallCredentials(callCredentialsProvider.getCallCredentials()); } /** diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteGrpcServiceConfig.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteGrpcServiceConfig.java new file mode 100644 index 00000000000000..0bf25bd1b345c6 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteGrpcServiceConfig.java @@ -0,0 +1,153 @@ +// Copyright 2026 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.remote; + +import static java.nio.charset.StandardCharsets.UTF_8; + +import build.bazel.remote.asset.v1.FetchGrpc; +import build.bazel.remote.execution.v2.ActionCacheGrpc; +import build.bazel.remote.execution.v2.CapabilitiesGrpc; +import build.bazel.remote.execution.v2.ContentAddressableStorageGrpc; +import com.google.bytestream.ByteStreamGrpc; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.remote.options.RemoteOptions; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.gson.Gson; +import com.google.gson.JsonArray; +import com.google.gson.JsonElement; +import com.google.gson.JsonObject; +import com.google.gson.JsonParseException; +import com.google.gson.JsonParser; +import com.google.gson.reflect.TypeToken; +import java.io.IOException; +import java.io.InputStreamReader; +import java.io.Reader; +import java.lang.reflect.Type; +import java.time.Duration; +import java.util.Map; + +/** Builds Bazel's generated gRPC service config for remote services. */ +public final class RemoteGrpcServiceConfig { + private static final Gson GSON = new Gson(); + private static final Type MAP_TYPE = new TypeToken>() {}.getType(); + private static final ImmutableSet SUPPORTED_TOP_LEVEL_FIELDS = + ImmutableSet.of("methodConfig"); + private static final ImmutableSet SUPPORTED_METHOD_CONFIG_FIELDS = + ImmutableSet.of("name", "timeout"); + private static final ImmutableSet SUPPORTED_NAME_FIELDS = + ImmutableSet.of("service", "method"); + + private RemoteGrpcServiceConfig() {} + + public static ImmutableMap create(RemoteOptions options) { + return create(options.remoteTimeout); + } + + public static ImmutableMap create(RemoteOptions options, Path workingDirectory) + throws IOException { + PathFragment serviceConfigPath = options.remoteGrpcServiceConfig; + if (serviceConfigPath == null) { + return create(options); + } + return parse(workingDirectory.getRelative(serviceConfigPath)); + } + + static ImmutableMap create(Duration remoteTimeout) { + return ImmutableMap.of( + "methodConfig", + ImmutableList.of( + ImmutableMap.of( + "name", + ImmutableList.of( + ImmutableMap.of("service", ActionCacheGrpc.SERVICE_NAME), + ImmutableMap.of("service", CapabilitiesGrpc.SERVICE_NAME), + ImmutableMap.of("service", ContentAddressableStorageGrpc.SERVICE_NAME), + ImmutableMap.of("service", ByteStreamGrpc.SERVICE_NAME), + ImmutableMap.of("service", FetchGrpc.SERVICE_NAME)), + "timeout", + remoteTimeout.toSeconds() + "s"))); + } + + private static ImmutableMap parse(Path serviceConfigPath) throws IOException { + try (Reader reader = new InputStreamReader(serviceConfigPath.getInputStream(), UTF_8)) { + JsonObject root = requireObject(JsonParser.parseReader(reader), "service config"); + rejectUnsupportedFields(root, SUPPORTED_TOP_LEVEL_FIELDS, "service config"); + rejectUnsupportedMethodConfigFields(root); + Map serviceConfig = GSON.fromJson(root, MAP_TYPE); + return ImmutableMap.copyOf(serviceConfig); + } catch (JsonParseException e) { + throw new IOException( + "failed to parse " + serviceConfigPath.getPathString() + ": " + e.getMessage(), e); + } + } + + private static void rejectUnsupportedMethodConfigFields(JsonObject rootObject) + throws IOException { + JsonElement methodConfigsElement = rootObject.get("methodConfig"); + if (methodConfigsElement == null || !methodConfigsElement.isJsonArray()) { + return; + } + + JsonArray methodConfigs = methodConfigsElement.getAsJsonArray(); + for (int i = 0; i < methodConfigs.size(); ++i) { + JsonElement methodConfigElement = methodConfigs.get(i); + if (!methodConfigElement.isJsonObject()) { + continue; + } + + JsonObject methodConfig = methodConfigElement.getAsJsonObject(); + String methodConfigPath = "methodConfig[" + i + "]"; + rejectUnsupportedFields(methodConfig, SUPPORTED_METHOD_CONFIG_FIELDS, methodConfigPath); + rejectUnsupportedNameFields(methodConfig, methodConfigPath); + } + } + + private static void rejectUnsupportedNameFields(JsonObject methodConfig, String methodConfigPath) + throws IOException { + JsonElement namesElement = methodConfig.get("name"); + if (namesElement == null || !namesElement.isJsonArray()) { + return; + } + + JsonArray names = namesElement.getAsJsonArray(); + for (int i = 0; i < names.size(); ++i) { + JsonElement nameElement = names.get(i); + if (nameElement.isJsonObject()) { + rejectUnsupportedFields( + nameElement.getAsJsonObject(), + SUPPORTED_NAME_FIELDS, + methodConfigPath + ".name[" + i + "]"); + } + } + } + + private static void rejectUnsupportedFields( + JsonObject object, ImmutableSet supportedFields, String path) throws IOException { + for (Map.Entry entry : object.entrySet()) { + if (!supportedFields.contains(entry.getKey())) { + throw new IOException(path + " contains unsupported field '" + entry.getKey() + "'"); + } + } + } + + private static JsonObject requireObject(JsonElement element, String path) throws IOException { + if (element == null || !element.isJsonObject()) { + throw new IOException(path + " must be an object"); + } + return element.getAsJsonObject(); + } +} 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 495aa6b5a4eed0..2f600474bc60cc 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 @@ -125,6 +125,7 @@ import java.net.URISyntaxException; import java.nio.channels.ClosedChannelException; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutorService; @@ -164,14 +165,16 @@ public ManagedChannel newChannel( String target, String proxy, AuthAndTLSOptions options, - List interceptors) + List interceptors, + Map serviceConfig) throws IOException { return GoogleAuthUtils.newChannel( executorService, target, proxy, options, - interceptors.isEmpty() ? null : interceptors); + interceptors.isEmpty() ? null : interceptors, + serviceConfig); } }; @@ -622,6 +625,16 @@ private boolean setup(CommandEnvironment env) throws AbruptExitException { retryScheduler, circuitBreaker); + ImmutableMap remoteGrpcServiceConfig; + try { + remoteGrpcServiceConfig = + RemoteGrpcServiceConfig.create(remoteOptions, env.getWorkingDirectory()); + } catch (IOException e) { + throw createOptionsExitException( + "Invalid --remote_grpc_service_config: " + e.getMessage(), + FailureDetails.RemoteOptions.Code.REMOTE_GRPC_SERVICE_CONFIG_INVALID); + } + if (!Strings.isNullOrEmpty(remoteOptions.remoteOutputService)) { var bazelOutputServiceChannel = createChannel( @@ -631,6 +644,7 @@ private boolean setup(CommandEnvironment env) throws AbruptExitException { Options.getDefaults(AuthAndTLSOptions.class), null, null, + remoteGrpcServiceConfig, channelFactory, remoteOptions.remoteOutputService, null, @@ -691,7 +705,6 @@ private boolean setup(CommandEnvironment env) throws AbruptExitException { invocationId, remoteOptions.remoteInstanceName, callCredentials, - remoteOptions.remoteTimeout.toSeconds(), retrier); ReferenceCountedChannel execChannel = null; @@ -715,6 +728,7 @@ private boolean setup(CommandEnvironment env) throws AbruptExitException { authAndTlsOptions, TracingMetadataUtils.newExecHeadersInterceptor(remoteOptions), loggingInterceptor, + remoteGrpcServiceConfig, channelFactory, remoteOptions.remoteExecutor, remoteOptions.remoteProxy, @@ -734,6 +748,7 @@ private boolean setup(CommandEnvironment env) throws AbruptExitException { authAndTlsOptions, TracingMetadataUtils.newExecHeadersInterceptor(remoteOptions), loggingInterceptor, + remoteGrpcServiceConfig, channelFactory, remoteOptions.remoteExecutor, remoteOptions.remoteProxy, @@ -755,6 +770,7 @@ private boolean setup(CommandEnvironment env) throws AbruptExitException { authAndTlsOptions, TracingMetadataUtils.newCacheHeadersInterceptor(remoteOptions), loggingInterceptor, + remoteGrpcServiceConfig, channelFactory, remoteOptions.remoteCache, remoteOptions.remoteProxy, @@ -870,6 +886,7 @@ private boolean setup(CommandEnvironment env) throws AbruptExitException { authAndTlsOptions, /* headersInterceptor= */ null, loggingInterceptor, + remoteGrpcServiceConfig, channelFactory, remoteOptions.remoteDownloader, remoteOptions.remoteProxy, @@ -908,6 +925,7 @@ private static ReferenceCountedChannel createChannel( AuthAndTLSOptions authAndTlsOptions, @Nullable ClientInterceptor headersInterceptor, @Nullable ClientInterceptor loggingInterceptor, + Map serviceConfig, ChannelFactory channelFactory, String target, String proxy, @@ -932,6 +950,7 @@ private static ReferenceCountedChannel createChannel( target, proxy, remoteOptions, + serviceConfig, authAndTlsOptions, interceptors.build(), maxConcurrencyPerConnection, diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteServerCapabilities.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteServerCapabilities.java index 6d038e77e769bb..e38af01b3d7a5b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteServerCapabilities.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteServerCapabilities.java @@ -34,7 +34,6 @@ import io.grpc.Channel; import io.grpc.ManagedChannel; import java.util.List; -import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; /** Fetches the ServerCapabilities of the remote execution/cache server. */ @@ -43,7 +42,6 @@ class RemoteServerCapabilities { private final String commandId; @Nullable private final String instanceName; @Nullable private final CallCredentials callCredentials; - private final long callTimeoutSecs; private final RemoteRetrier retrier; public RemoteServerCapabilities( @@ -51,13 +49,11 @@ public RemoteServerCapabilities( String commandId, @Nullable String instanceName, @Nullable CallCredentials callCredentials, - long callTimeoutSecs, RemoteRetrier retrier) { this.buildRequestId = buildRequestId; this.commandId = commandId; this.instanceName = instanceName; this.callCredentials = callCredentials; - this.callTimeoutSecs = callTimeoutSecs; this.retrier = retrier; } @@ -66,8 +62,7 @@ private CapabilitiesFutureStub capabilitiesFutureStub( return CapabilitiesGrpc.newFutureStub(channel) .withInterceptors( TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata())) - .withCallCredentials(callCredentials) - .withDeadlineAfter(callTimeoutSecs, TimeUnit.SECONDS); + .withCallCredentials(callCredentials); } public ListenableFuture get(ManagedChannel channel) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java b/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java index 5b278bc5b33e3d..7bc38e146a5dd4 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloader.java @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.remote.downloader; - import build.bazel.remote.asset.v1.FetchBlobRequest; import build.bazel.remote.asset.v1.FetchBlobResponse; import build.bazel.remote.asset.v1.FetchGrpc; @@ -57,7 +56,6 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; /** @@ -311,8 +309,7 @@ private FetchBlockingStub fetchBlockingStub( .withInterceptors( TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata())) .withInterceptors(TracingMetadataUtils.newDownloaderHeadersInterceptor(options)) - .withCallCredentials(credentials.orElse(null)) - .withDeadlineAfter(options.remoteTimeout.toSeconds(), TimeUnit.SECONDS); + .withCallCredentials(credentials.orElse(null)); } private OutputStream newOutputStream(Path destination, Optional checksum) diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index f79d5184b912ba..af4338cb919ab8 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -242,6 +242,20 @@ concurrent requests (controlled by --remote_max_concurrency_per_connection), so + " the unit is omitted, the value is interpreted as seconds.") public Duration remoteTimeout; + @Option( + name = "remote_grpc_service_config", + defaultValue = "null", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + converter = OptionsUtils.EmptyToNullPathFragmentConverter.class, + help = + "Path to a gRPC service config JSON file for remote gRPC channels. This replaces the" + + " service config Bazel generates from --remote_timeout. Only a subset of the gRPC" + + " service config JSON schema is supported: top-level methodConfig entries with" + + " name objects containing service and optional method, plus timeout. Other service" + + " config fields are rejected and may be supported in the future.") + public PathFragment remoteGrpcServiceConfig; + @Option( name = "remote_bytestream_uri_prefix", defaultValue = "null", diff --git a/src/main/protobuf/failure_details.proto b/src/main/protobuf/failure_details.proto index 6f508ca48d51d2..d867e1db512a14 100644 --- a/src/main/protobuf/failure_details.proto +++ b/src/main/protobuf/failure_details.proto @@ -307,6 +307,7 @@ message RemoteOptions { CREDENTIALS_WRITE_FAILURE = 3 [(metadata) = { exit_code: 36 }]; DOWNLOADER_WITHOUT_GRPC_CACHE = 4 [(metadata) = { exit_code: 2 }]; EXECUTION_WITH_INVALID_CACHE = 5 [(metadata) = { exit_code: 2 }]; + REMOTE_GRPC_SERVICE_CONFIG_INVALID = 7 [(metadata) = { exit_code: 2 }]; reserved 6; } diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java index 1a921b05c8b755..4d9400e4158e56 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamUploaderTest.java @@ -171,7 +171,6 @@ public void singleBlobUploadShouldWork() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -200,7 +199,6 @@ public void singleChunkCompressedUploadAlreadyExists() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -265,7 +263,6 @@ public void progressiveUploadShouldWork() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - 3, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -383,7 +380,6 @@ public void progressiveCompressedUploadShouldWork() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - 300, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -506,7 +502,6 @@ public void progressiveCompressedUploadSeesAlreadyExistsAtTheEnd() throws Except INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - 300, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -566,7 +561,6 @@ public void concurrentlyCompletedUploadIsNotRetried() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - 1, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -626,7 +620,6 @@ public void unimplementedQueryShouldRestartUpload() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - 3, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -697,7 +690,6 @@ public void earlyWriteResponseShouldCompleteUpload() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - 3, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -737,7 +729,6 @@ public void incorrectCommittedSizeFailsCompletedUpload() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - 3, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -792,7 +783,6 @@ public void incorrectCommittedSizeDoesNotFailIncompleteUpload() throws Exception INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - 300, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -826,7 +816,6 @@ public void multipleBlobsUploadShouldWork() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -860,7 +849,6 @@ public void tooManyFilesIOException_adviseMaximumOpenFilesFlag() throws Exceptio INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -895,7 +883,6 @@ public void availablePermitsOpenFileSemaphore_fewerPermitsThanUploads_endWithAll INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, maximumOpenFiles, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -935,7 +922,6 @@ public void noMaximumOpenFilesFlags_nullSemaphore() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -973,7 +959,6 @@ public void contextShouldBePreservedUponRetries() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -1106,7 +1091,6 @@ public int maxConcurrency() { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -1169,7 +1153,6 @@ public void errorsShouldBeReported() throws IOException, InterruptedException { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -1207,7 +1190,6 @@ public void failureInRetryExecutorShouldBeHandled() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -1248,7 +1230,6 @@ public void resourceNameWithoutInstanceName() throws Exception { /* instanceName= */ null, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -1293,7 +1274,6 @@ public void resourceWithNewStyleDigestFunction() throws Exception { /* instanceName= */ null, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.BLAKE3); @@ -1340,7 +1320,6 @@ public void nonRetryableStatusShouldNotBeRetried() throws Exception { /* instanceName= */ null, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -1394,7 +1373,6 @@ public void refresh() throws IOException { INSTANCE_NAME, referenceCountedChannel, callCredentialsProvider, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -1451,7 +1429,6 @@ public void refresh() throws IOException { INSTANCE_NAME, referenceCountedChannel, callCredentialsProvider, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -1524,7 +1501,6 @@ public void failureAfterUploadCompletes() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, -1, /* digestFunction= */ DigestFunction.Value.SHA256); @@ -1585,7 +1561,6 @@ public void testCompressedUploads() throws Exception { INSTANCE_NAME, referenceCountedChannel, CallCredentialsProvider.NO_CREDENTIALS, - /* callTimeoutSecs= */ 60, retrier, /* maximumOpenFiles= */ -1, /* digestFunction= */ DigestFunction.Value.SHA256); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteGrpcServiceConfigTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteGrpcServiceConfigTest.java new file mode 100644 index 00000000000000..17df9d5d2ca6ae --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteGrpcServiceConfigTest.java @@ -0,0 +1,134 @@ +// Copyright 2026 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.remote; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.remote.options.RemoteOptions; +import com.google.devtools.build.lib.testutil.Scratch; +import com.google.devtools.build.lib.vfs.DigestHashFunction; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import com.google.devtools.common.options.OptionsParser; +import java.io.IOException; +import java.time.Duration; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link RemoteGrpcServiceConfig}. */ +@RunWith(JUnit4.class) +public final class RemoteGrpcServiceConfigTest { + + private static RemoteOptions parseRemoteOptions(String... args) throws Exception { + OptionsParser parser = OptionsParser.builder().optionsClasses(RemoteOptions.class).build(); + parser.parse(args); + return parser.getOptions(RemoteOptions.class); + } + + @Test + public void create_usesRemoteTimeoutForRemoteServices() { + assertThat(RemoteGrpcServiceConfig.create(Duration.ofSeconds(123))) + .containsExactly( + "methodConfig", + ImmutableList.of( + ImmutableMap.of( + "name", + ImmutableList.of( + ImmutableMap.of("service", "build.bazel.remote.execution.v2.ActionCache"), + ImmutableMap.of("service", "build.bazel.remote.execution.v2.Capabilities"), + ImmutableMap.of( + "service", "build.bazel.remote.execution.v2.ContentAddressableStorage"), + ImmutableMap.of("service", "google.bytestream.ByteStream"), + ImmutableMap.of("service", "build.bazel.remote.asset.v1.Fetch")), + "timeout", + "123s"))); + } + + @Test + public void create_usesUserSuppliedJsonFile() throws Exception { + Scratch scratch = new Scratch(new InMemoryFileSystem(DigestHashFunction.SHA256)); + Path workspace = scratch.dir("/workspace"); + scratch.file( + "/workspace/service_config.json", + """ + { + "methodConfig": [ + { + "name": [ + { + "service": "build.bazel.remote.execution.v2.ActionCache", + "method": "GetActionResult" + }, + {"service": "google.bytestream.ByteStream"} + ], + "timeout": "3.500s" + } + ] + } + """); + + assertThat( + RemoteGrpcServiceConfig.create( + parseRemoteOptions("--remote_grpc_service_config=service_config.json"), workspace)) + .containsExactly( + "methodConfig", + ImmutableList.of( + ImmutableMap.of( + "name", + ImmutableList.of( + ImmutableMap.of( + "service", + "build.bazel.remote.execution.v2.ActionCache", + "method", + "GetActionResult"), + ImmutableMap.of("service", "google.bytestream.ByteStream")), + "timeout", + "3.500s"))); + } + + @Test + public void create_rejectsUnsupportedJsonFields() throws Exception { + Scratch scratch = new Scratch(new InMemoryFileSystem(DigestHashFunction.SHA256)); + Path workspace = scratch.dir("/workspace"); + scratch.file( + "/workspace/service_config.json", + """ + { + "methodConfig": [ + { + "name": [{"service": "google.bytestream.ByteStream"}], + "timeout": "1s", + "retryPolicy": {} + } + ] + } + """); + + IOException e = + Assert.assertThrows( + IOException.class, + () -> + RemoteGrpcServiceConfig.create( + parseRemoteOptions("--remote_grpc_service_config=service_config.json"), + workspace)); + + assertThat(e) + .hasMessageThat() + .contains("methodConfig[0] contains unsupported field 'retryPolicy'"); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java index 06c9d25bf4fd69..e998aa53f49747 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteModuleTest.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.remote; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat; import static com.google.devtools.build.lib.util.io.CommandExtensionReporter.NO_OP_COMMAND_EXTENSION_REPORTER; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -70,6 +71,7 @@ import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsParser; @@ -86,6 +88,8 @@ import java.net.URI; import java.time.Duration; import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; import java.util.Optional; import org.junit.Assert; import org.junit.Before; @@ -136,8 +140,15 @@ public final class RemoteModuleTest { .build()) .build(); + @FunctionalInterface + private interface WorkspaceInitializer { + void initialize(Scratch scratch) throws IOException; + } + private static CommandEnvironment createTestCommandEnvironment( - RemoteModule remoteModule, RemoteOptions remoteOptions) + RemoteModule remoteModule, + RemoteOptions remoteOptions, + WorkspaceInitializer workspaceInitializer) throws IOException, AbruptExitException { CoreOptions coreOptions = Options.getDefaults(CoreOptions.class); CommonCommandOptions commonCommandOptions = Options.getDefaults(CommonCommandOptions.class); @@ -163,6 +174,8 @@ private static CommandEnvironment createTestCommandEnvironment( ServerDirectories serverDirectories = new ServerDirectories( scratch.dir("install"), scratch.dir("output"), scratch.dir("user_root")); + Path workspacePath = scratch.dir("/workspace"); + workspaceInitializer.initialize(scratch); BlazeRuntime runtime = new BlazeRuntime.Builder() @@ -186,7 +199,7 @@ public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) { BlazeDirectories directories = new BlazeDirectories( serverDirectories, - scratch.dir("/workspace"), + workspacePath, scratch.dir("/system_javabase"), productName); BlazeWorkspace workspace = runtime.initWorkspace(directories, BinTools.empty(directories)); @@ -240,18 +253,103 @@ private static Server createFakeServer(String serverName, BindableService... ser .build(); } + private static RemoteOptions parseRemoteOptions(String... args) throws Exception { + OptionsParser parser = OptionsParser.builder().optionsClasses(RemoteOptions.class).build(); + parser.parse(args); + return parser.getOptions(RemoteOptions.class); + } + private RemoteModule remoteModule; private RemoteOptions remoteOptions; + private Map> serviceConfigsByTarget; @Before public void initialize() { + serviceConfigsByTarget = new HashMap<>(); remoteModule = new RemoteModule(); remoteModule.setChannelFactory( - (target, proxy, options, interceptors) -> - InProcessChannelBuilder.forName(target).directExecutor().build()); + (target, proxy, options, interceptors, serviceConfig) -> { + serviceConfigsByTarget.put(target, serviceConfig); + return InProcessChannelBuilder.forName(target).directExecutor().build(); + }); remoteOptions = Options.getDefaults(RemoteOptions.class); } + @Test + public void remoteGrpcServiceConfig_passesRemoteTimeoutConfigToChannelFactory() throws Exception { + CapabilitiesImpl cacheCapabilitiesImpl = new CapabilitiesImpl(CACHE_ONLY_CAPS); + Server cacheServer = createFakeServer(CACHE_SERVER_NAME, cacheCapabilitiesImpl); + cacheServer.start(); + + try { + remoteOptions = + parseRemoteOptions("--remote_cache=" + CACHE_SERVER_NAME, "--remote_timeout=123s"); + + beforeCommand(); + + assertThat( + remoteModule + .getActionContextProvider() + .getCombinedCache() + .getRemoteCacheCapabilities()) + .isEqualTo(CACHE_ONLY_CAPS.getCacheCapabilities()); + assertThat(serviceConfigsByTarget.get(CACHE_SERVER_NAME)) + .isEqualTo(RemoteGrpcServiceConfig.create(Duration.ofSeconds(123))); + } finally { + cacheServer.shutdownNow(); + cacheServer.awaitTermination(); + } + } + + @Test + public void remoteGrpcServiceConfig_passesUserSuppliedJsonFileToChannelFactory() + throws Exception { + CapabilitiesImpl cacheCapabilitiesImpl = new CapabilitiesImpl(CACHE_ONLY_CAPS); + Server cacheServer = createFakeServer(CACHE_SERVER_NAME, cacheCapabilitiesImpl); + cacheServer.start(); + + try { + remoteOptions = + parseRemoteOptions( + "--remote_cache=" + CACHE_SERVER_NAME, + "--remote_grpc_service_config=service_config.json"); + + beforeCommand( + scratch -> + scratch.file( + "/workspace/service_config.json", + """ + { + "methodConfig": [ + { + "name": [{"service": "google.bytestream.ByteStream"}], + "timeout": "3.500s" + } + ] + } + """)); + + assertThat( + remoteModule + .getActionContextProvider() + .getCombinedCache() + .getRemoteCacheCapabilities()) + .isEqualTo(CACHE_ONLY_CAPS.getCacheCapabilities()); + assertThat(serviceConfigsByTarget.get(CACHE_SERVER_NAME)) + .containsExactly( + "methodConfig", + ImmutableList.of( + ImmutableMap.of( + "name", + ImmutableList.of(ImmutableMap.of("service", "google.bytestream.ByteStream")), + "timeout", + "3.500s"))); + } finally { + cacheServer.shutdownNow(); + cacheServer.awaitTermination(); + } + } + @Test public void testVerifyCapabilities_none() throws Exception { // Test that Bazel doesn't issue GetCapabilities calls if the requirement is NONE. @@ -633,7 +731,14 @@ public void repositoryRemoteHelpersFactory_initializedForDiskCacheOnlyPath() thr @CanIgnoreReturnValue private CommandEnvironment beforeCommand() throws IOException, AbruptExitException { - CommandEnvironment env = createTestCommandEnvironment(remoteModule, remoteOptions); + return beforeCommand(scratch -> {}); + } + + @CanIgnoreReturnValue + private CommandEnvironment beforeCommand(WorkspaceInitializer workspaceInitializer) + throws IOException, AbruptExitException { + CommandEnvironment env = + createTestCommandEnvironment(remoteModule, remoteOptions, workspaceInitializer); remoteModule.beforeCommand(env); env.throwPendingException(); return env; diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java index 0ffac81cda177d..e10ef5c0c93ded 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteServerCapabilitiesTest.java @@ -150,7 +150,7 @@ public void getCapabilities( .directExecutor() .build(); RemoteServerCapabilities client = - new RemoteServerCapabilities("build-req-id", "command-id", "instance", null, 3, retrier); + new RemoteServerCapabilities("build-req-id", "command-id", "instance", null, retrier); assertThat(client.get(channel).get()).isEqualTo(caps); } @@ -194,7 +194,7 @@ public void getCapabilities( InProcessChannelBuilder.forName(fakeServerName).directExecutor().build(); RemoteServerCapabilities client = new RemoteServerCapabilities( - "build-req-id", "command-id", "instance", /* callCredentials= */ null, 3, retrier); + "build-req-id", "command-id", "instance", /* callCredentials= */ null, retrier); assertThat(client.get(channel).get()).isEqualTo(caps); }