remote: Refactor GrpcRemoteExecutor to only take what it needs.

- Only pass to the GrpcRemoteExecutor constructor the objects it really
needs.

- Share a Retrier between GrpcRemoteCache and GrpcRemoteExecutor.

RELNOTES: None
PiperOrigin-RevId: 161660054

5 files changed, 34 insertions, 25 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
index 5b21e249b5..6a6e0bee9a 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
@@ -77,11 +77,12 @@ public class GrpcRemoteCache implements RemoteActionCache {
       MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
 
   @VisibleForTesting
-  public GrpcRemoteCache(Channel channel, ChannelOptions channelOptions, RemoteOptions options) {
+  public GrpcRemoteCache(Channel channel, ChannelOptions channelOptions, RemoteOptions options,
+      Retrier retrier) {
     this.options = options;
     this.channelOptions = channelOptions;
     this.channel = channel;
-    this.retrier = new Retrier(options);
+    this.retrier = retrier;
 
     uploader = new ByteStreamUploader(options.remoteInstanceName, channel,
         channelOptions.getCallCredentials(), options.remoteTimeout, retrier, retryScheduler);
diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutor.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutor.java
index 173f10ddb0..ad4a4ddbe7 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutor.java
@@ -28,6 +28,7 @@ import com.google.watcher.v1.ChangeBatch;
 import com.google.watcher.v1.Request;
 import com.google.watcher.v1.WatcherGrpc;
 import com.google.watcher.v1.WatcherGrpc.WatcherBlockingStub;
+import io.grpc.CallCredentials;
 import io.grpc.Channel;
 import io.grpc.Status.Code;
 import io.grpc.StatusRuntimeException;
@@ -39,32 +40,31 @@ import javax.annotation.Nullable;
 
 /** A remote work executor that uses gRPC for communicating the work, inputs and outputs. */
 @ThreadSafe
-public class GrpcRemoteExecutor {
-  private final RemoteOptions options;
-  private final ChannelOptions channelOptions;
+final class GrpcRemoteExecutor {
+
   private final Channel channel;
+  private final CallCredentials callCredentials;
+  private final int callTimeoutSecs;
   private final Retrier retrier;
 
-  public static boolean isRemoteExecutionOptions(RemoteOptions options) {
-    return options.remoteExecutor != null;
-  }
-
-  public GrpcRemoteExecutor(Channel channel, ChannelOptions channelOptions, RemoteOptions options) {
-    this.options = options;
-    this.channelOptions = channelOptions;
+  public GrpcRemoteExecutor(Channel channel, @Nullable CallCredentials callCredentials,
+      int callTimeoutSecs, Retrier retrier) {
+    Preconditions.checkArgument(callTimeoutSecs > 0, "callTimeoutSecs must be gt 0.");
     this.channel = channel;
-    this.retrier = new Retrier(options);
+    this.callCredentials = callCredentials;
+    this.callTimeoutSecs = callTimeoutSecs;
+    this.retrier = retrier;
   }
 
   private ExecutionBlockingStub execBlockingStub() {
     return ExecutionGrpc.newBlockingStub(channel)
-        .withCallCredentials(channelOptions.getCallCredentials())
-        .withDeadlineAfter(options.remoteTimeout, TimeUnit.SECONDS);
+        .withCallCredentials(callCredentials)
+        .withDeadlineAfter(callTimeoutSecs, TimeUnit.SECONDS);
   }
 
   private WatcherBlockingStub watcherBlockingStub() {
     return WatcherGrpc.newBlockingStub(channel)
-        .withCallCredentials(channelOptions.getCallCredentials());
+        .withCallCredentials(callCredentials);
   }
 
   private @Nullable ExecuteResponse getOperationResponse(Operation op)
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
index 01d99aefd2..9b6e42c35d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -50,8 +50,8 @@ final class RemoteActionContextProvider extends ActionContextProvider {
     AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
     ChannelOptions channelOptions = ChannelOptions.create(authAndTlsOptions);
 
-    // Initialize remote cache and execution handlers. We use separate handlers for every
-    // action to enable server-side parallelism (need a different gRPC channel per action).
+    Retrier retrier = new Retrier(remoteOptions);
+
     RemoteActionCache remoteCache;
     if (SimpleBlobStoreFactory.isRemoteCacheOptions(remoteOptions)) {
       remoteCache = new SimpleBlobStoreActionCache(SimpleBlobStoreFactory.create(remoteOptions));
@@ -60,19 +60,21 @@ final class RemoteActionContextProvider extends ActionContextProvider {
           new GrpcRemoteCache(
               GrpcUtils.createChannel(remoteOptions.remoteCache, channelOptions),
               channelOptions,
-              remoteOptions);
+              remoteOptions,
+              retrier);
     } else {
       remoteCache = null;
     }
 
     // Otherwise remoteCache remains null and remote caching/execution are disabled.
     GrpcRemoteExecutor remoteExecutor;
-    if (remoteCache != null && GrpcRemoteExecutor.isRemoteExecutionOptions(remoteOptions)) {
+    if (remoteCache != null && remoteOptions.remoteExecutor != null) {
       remoteExecutor =
           new GrpcRemoteExecutor(
               GrpcUtils.createChannel(remoteOptions.remoteExecutor, channelOptions),
-              channelOptions,
-              remoteOptions);
+              channelOptions.getCallCredentials(),
+              remoteOptions.remoteTimeout,
+              retrier);
     } else {
       remoteExecutor = null;
     }
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
index 7b1f359b91..d0d1d0bd57 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
@@ -140,12 +140,15 @@ public class GrpcRemoteCacheTest {
     ChannelOptions channelOptions =
         ChannelOptions.create(
             authTlsOptions, scratch.resolve(authTlsOptions.authCredentials).getInputStream());
+    RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class);
+    Retrier retrier = new Retrier(remoteOptions);
     return new GrpcRemoteCache(
         ClientInterceptors.intercept(
             InProcessChannelBuilder.forName(fakeServerName).directExecutor().build(),
             ImmutableList.of(new ChannelOptionsInterceptor(channelOptions))),
         channelOptions,
-        Options.getDefaults(RemoteOptions.class));
+        remoteOptions,
+        retrier);
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
index 238b831deb..05e4814f87 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
@@ -175,9 +175,12 @@ public class GrpcRemoteExecutionClientTest {
     FileSystemUtils.createDirectoryAndParents(stderr.getParentDirectory());
     outErr = new FileOutErr(stdout, stderr);
     RemoteOptions options = Options.getDefaults(RemoteOptions.class);
+    Retrier retrier = new Retrier(options);
     Channel channel = InProcessChannelBuilder.forName(fakeServerName).directExecutor().build();
-    GrpcRemoteExecutor executor = new GrpcRemoteExecutor(channel, ChannelOptions.DEFAULT, options);
-    GrpcRemoteCache remoteCache = new GrpcRemoteCache(channel, ChannelOptions.DEFAULT, options);
+    GrpcRemoteExecutor executor =
+        new GrpcRemoteExecutor(channel, null, options.remoteTimeout, retrier);
+    GrpcRemoteCache remoteCache =
+        new GrpcRemoteCache(channel, ChannelOptions.DEFAULT, options, retrier);
     client = new RemoteSpawnRunner(execRoot, options, null, remoteCache, executor);
     inputDigest = fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().get(0), "xyz");
   }