From 71e35b165f924e2649a078fcf6007645d58039af Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 29 Jan 2021 00:03:35 -0800 Subject: [PATCH] Remote: Use parameters instead of thread-local storage to provide tracing metadata. (Part 5) Change MissingDigestsFinder#findMissingDigests and RemoteExecutionClient#executeRemotely to use RemoteActionExecutionContext. Removed all the usages of io.grpc.Context in the client code. Fixed the regression about NetworkTime introduced by https://github.com/bazelbuild/bazel/commit/bc54c648aa1f99509c7c36d5e6b570d066689209. PiperOrigin-RevId: 354479787 --- .../ByteStreamBuildEventArtifactUploader.java | 12 +- .../build/lib/remote/ByteStreamUploader.java | 1 - .../ExperimentalGrpcRemoteExecutor.java | 15 +- .../build/lib/remote/GrpcCacheClient.java | 21 +- .../build/lib/remote/GrpcRemoteExecutor.java | 13 +- .../lib/remote/NetworkTimeInterceptor.java | 2 - .../lib/remote/RemoteActionInputFetcher.java | 75 ++--- .../build/lib/remote/RemoteCache.java | 3 +- .../lib/remote/RemoteExecutionCache.java | 2 +- .../RemoteRepositoryRemoteExecutor.java | 90 +++--- .../lib/remote/RemoteServerCapabilities.java | 18 +- .../build/lib/remote/RemoteSpawnCache.java | 24 +- .../build/lib/remote/RemoteSpawnRunner.java | 288 +++++++++--------- .../remote/common/MissingDigestsFinder.java | 3 +- .../common/RemoteActionExecutionContext.java | 5 + .../remote/common/RemoteExecutionClient.java | 3 +- ...> SimpleRemoteActionExecutionContext.java} | 4 +- .../remote/disk/DiskAndRemoteCacheClient.java | 9 +- .../lib/remote/disk/DiskCacheClient.java | 3 +- .../downloader/GrpcRemoteDownloader.java | 4 +- .../lib/remote/http/HttpCacheClient.java | 3 +- .../lib/remote/util/TracingMetadataUtils.java | 46 +-- ...eStreamBuildEventArtifactUploaderTest.java | 22 +- .../lib/remote/ByteStreamUploaderTest.java | 73 +---- .../ExperimentalGrpcRemoteExecutorTest.java | 54 ++-- .../build/lib/remote/GrpcCacheClientTest.java | 84 ++--- .../build/lib/remote/RemoteCacheTests.java | 197 +++++------- .../RemoteRepositoryRemoteExecutorTest.java | 8 +- .../lib/remote/RemoteSpawnCacheTest.java | 50 +-- .../lib/remote/RemoteSpawnRunnerTest.java | 242 ++++++++++++--- .../downloader/GrpcRemoteDownloaderTest.java | 12 +- .../lib/remote/http/HttpCacheClientTest.java | 7 +- .../lib/remote/util/InMemoryCacheClient.java | 3 +- .../remote/worker/ActionCacheServer.java | 8 +- .../build/remote/worker/ByteStreamServer.java | 8 +- .../build/remote/worker/CasServer.java | 8 +- .../build/remote/worker/ExecutionServer.java | 15 +- 37 files changed, 660 insertions(+), 775 deletions(-) rename src/main/java/com/google/devtools/build/lib/remote/common/{RemoteActionExecutionContextImpl.java => SimpleRemoteActionExecutionContext.java} (89%) diff --git a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java index 0ca33b6bd4c8ed..92177fcf62b26b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploader.java @@ -30,13 +30,10 @@ import com.google.devtools.build.lib.buildeventstream.PathConverter; import com.google.devtools.build.lib.collect.ImmutableIterable; import com.google.devtools.build.lib.remote.common.MissingDigestsFinder; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.vfs.Path; -import io.grpc.Context; import io.netty.util.AbstractReferenceCounted; import io.netty.util.ReferenceCounted; import java.io.IOException; @@ -161,7 +158,9 @@ private static List processQueryResult( */ private ListenableFuture> queryRemoteCache( ImmutableList> allPaths) throws Exception { - Context ctx = TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, "bes-upload"); + RequestMetadata metadata = + TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "bes-upload"); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(metadata); List knownRemotePaths = new ArrayList<>(allPaths.size()); List filesToQuery = new ArrayList<>(); @@ -181,7 +180,7 @@ private ListenableFuture> queryRemoteCache( return Futures.immediateFuture(ImmutableIterable.from(knownRemotePaths)); } return Futures.transform( - ctx.call(() -> missingDigestsFinder.findMissingDigests(digestsToQuery)), + missingDigestsFinder.findMissingDigests(context, digestsToQuery), (missingDigests) -> { List filesToQueryUpdated = processQueryResult(missingDigests, filesToQuery); return ImmutableIterable.from(Iterables.concat(knownRemotePaths, filesToQueryUpdated)); @@ -197,8 +196,7 @@ private ListenableFuture> uploadLocalFiles( ImmutableIterable allPaths) { RequestMetadata metadata = TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "bes-upload"); - RemoteActionExecutionContext context = - new RemoteActionExecutionContextImpl(metadata, new NetworkTime()); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(metadata); ImmutableList.Builder> allPathsUploaded = ImmutableList.builder(); 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 bba8593ed91050..0301037a558174 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 @@ -208,7 +208,6 @@ void shutdown() { * boolean)} instead. */ @Deprecated - @VisibleForTesting public ListenableFuture uploadBlobAsync( RemoteActionExecutionContext context, HashCode hash, Chunker chunker, boolean forceUpload) { Digest digest = diff --git a/src/main/java/com/google/devtools/build/lib/remote/ExperimentalGrpcRemoteExecutor.java b/src/main/java/com/google/devtools/build/lib/remote/ExperimentalGrpcRemoteExecutor.java index ea999318932b61..41f5306624d29e 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ExperimentalGrpcRemoteExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ExperimentalGrpcRemoteExecutor.java @@ -19,6 +19,7 @@ import build.bazel.remote.execution.v2.ExecuteResponse; import build.bazel.remote.execution.v2.ExecutionGrpc; import build.bazel.remote.execution.v2.ExecutionGrpc.ExecutionBlockingStub; +import build.bazel.remote.execution.v2.RequestMetadata; import build.bazel.remote.execution.v2.WaitExecutionRequest; import com.google.common.base.Preconditions; import com.google.devtools.build.lib.authandtls.CallCredentialsProvider; @@ -26,6 +27,7 @@ import com.google.devtools.build.lib.remote.RemoteRetrier.ProgressiveBackoff; import com.google.devtools.build.lib.remote.Retrier.Backoff; import com.google.devtools.build.lib.remote.common.OperationObserver; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; @@ -71,9 +73,9 @@ public ExperimentalGrpcRemoteExecutor( this.retrier = retrier; } - private ExecutionBlockingStub executionBlockingStub() { + private ExecutionBlockingStub executionBlockingStub(RequestMetadata metadata) { return ExecutionGrpc.newBlockingStub(channel) - .withInterceptors(TracingMetadataUtils.attachMetadataFromContextInterceptor()) + .withInterceptors(TracingMetadataUtils.attachMetadataInterceptor(metadata)) .withCallCredentials(callCredentialsProvider.getCallCredentials()) .withDeadlineAfter(remoteOptions.remoteTimeout.getSeconds(), SECONDS); } @@ -310,11 +312,16 @@ static ExecuteResponse extractResponseOrThrowIfError(Operation operation) throws } @Override - public ExecuteResponse executeRemotely(ExecuteRequest request, OperationObserver observer) + public ExecuteResponse executeRemotely( + RemoteActionExecutionContext context, ExecuteRequest request, OperationObserver observer) throws IOException, InterruptedException { Execution execution = new Execution( - request, observer, retrier, callCredentialsProvider, this::executionBlockingStub); + request, + observer, + retrier, + callCredentialsProvider, + () -> this.executionBlockingStub(context.getRequestMetadata())); return execution.start(); } 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 b2aeb4a8b0ef3a..29ee7acf7d9d27 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 @@ -56,7 +56,6 @@ import com.google.devtools.build.lib.remote.util.Utils; import com.google.devtools.build.lib.vfs.Path; import com.google.protobuf.ByteString; -import io.grpc.Context; import io.grpc.Status; import io.grpc.Status.Code; import io.grpc.StatusRuntimeException; @@ -122,9 +121,11 @@ private int computeMaxMissingBlobsDigestsPerMessage() { return (options.maxOutboundMessageSize - overhead) / digestSize; } - private ContentAddressableStorageFutureStub casFutureStub() { + private ContentAddressableStorageFutureStub casFutureStub(RemoteActionExecutionContext context) { return ContentAddressableStorageGrpc.newFutureStub(channel) - .withInterceptors(TracingMetadataUtils.attachMetadataFromContextInterceptor()) + .withInterceptors( + TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata()), + new NetworkTimeInterceptor(context::getNetworkTime)) .withCallCredentials(callCredentialsProvider.getCallCredentials()) .withDeadlineAfter(options.remoteTimeout.getSeconds(), TimeUnit.SECONDS); } @@ -176,7 +177,8 @@ public static boolean isRemoteCacheOptions(RemoteOptions options) { } @Override - public ListenableFuture> findMissingDigests(Iterable digests) { + public ListenableFuture> findMissingDigests( + RemoteActionExecutionContext context, Iterable digests) { if (Iterables.isEmpty(digests)) { return Futures.immediateFuture(ImmutableSet.of()); } @@ -187,13 +189,13 @@ public ListenableFuture> findMissingDigests(Iterable 0) { - getMissingDigestCalls.add(getMissingDigests(requestBuilder.build())); + getMissingDigestCalls.add(getMissingDigests(context, requestBuilder.build())); } ListenableFuture> success = @@ -209,7 +211,7 @@ public ListenableFuture> findMissingDigests(Iterable> findMissingDigests(Iterable getMissingDigests( - FindMissingBlobsRequest request) { - Context ctx = Context.current(); + RemoteActionExecutionContext context, FindMissingBlobsRequest request) { return Utils.refreshIfUnauthenticatedAsync( - () -> retrier.executeAsync(() -> ctx.call(() -> casFutureStub().findMissingBlobs(request))), + () -> retrier.executeAsync(() -> casFutureStub(context).findMissingBlobs(request)), callCredentialsProvider); } 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 22b068bf5b6a66..0b8c3fa312585e 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 @@ -18,11 +18,13 @@ import build.bazel.remote.execution.v2.ExecuteResponse; import build.bazel.remote.execution.v2.ExecutionGrpc; import build.bazel.remote.execution.v2.ExecutionGrpc.ExecutionBlockingStub; +import build.bazel.remote.execution.v2.RequestMetadata; import build.bazel.remote.execution.v2.WaitExecutionRequest; import com.google.common.base.Preconditions; import com.google.devtools.build.lib.authandtls.CallCredentialsProvider; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.remote.common.OperationObserver; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.remote.util.Utils; @@ -55,9 +57,9 @@ public GrpcRemoteExecutor( this.retrier = retrier; } - private ExecutionBlockingStub execBlockingStub() { + private ExecutionBlockingStub execBlockingStub(RequestMetadata metadata) { return ExecutionGrpc.newBlockingStub(channel) - .withInterceptors(TracingMetadataUtils.attachMetadataFromContextInterceptor()) + .withInterceptors(TracingMetadataUtils.attachMetadataInterceptor(metadata)) .withCallCredentials(callCredentialsProvider.getCallCredentials()); } @@ -105,7 +107,8 @@ private ExecuteResponse getOperationResponse(Operation op) throws IOException { * trigger a retry of the Execute call, resulting in a new Operation. * */ @Override - public ExecuteResponse executeRemotely(ExecuteRequest request, OperationObserver observer) + public ExecuteResponse executeRemotely( + RemoteActionExecutionContext context, ExecuteRequest request, OperationObserver observer) throws IOException, InterruptedException { // Execute has two components: the Execute call and (optionally) the WaitExecution call. // This is the simple flow without any errors: @@ -149,9 +152,9 @@ public ExecuteResponse executeRemotely(ExecuteRequest request, OperationObserver WaitExecutionRequest.newBuilder() .setName(operation.get().getName()) .build(); - replies = execBlockingStub().waitExecution(wr); + replies = execBlockingStub(context.getRequestMetadata()).waitExecution(wr); } else { - replies = execBlockingStub().execute(request); + replies = execBlockingStub(context.getRequestMetadata()).execute(request); } try { while (replies.hasNext()) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/NetworkTimeInterceptor.java b/src/main/java/com/google/devtools/build/lib/remote/NetworkTimeInterceptor.java index ac953aa48f9d42..571456c70d8d0b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/NetworkTimeInterceptor.java +++ b/src/main/java/com/google/devtools/build/lib/remote/NetworkTimeInterceptor.java @@ -19,7 +19,6 @@ import io.grpc.Channel; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; -import io.grpc.Context; import io.grpc.ForwardingClientCall; import io.grpc.ForwardingClientCallListener; import io.grpc.Metadata; @@ -30,7 +29,6 @@ /** The ClientInterceptor used to track network time. */ public class NetworkTimeInterceptor implements ClientInterceptor { - public static final Context.Key CONTEXT_KEY = Context.key("remote-network-time"); private final Supplier networkTimeSupplier; public NetworkTimeInterceptor(Supplier networkTimeSupplier) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java index e78c168e82e197..a9e7246bd132fc 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java @@ -33,15 +33,12 @@ import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.remote.util.Utils; import com.google.devtools.build.lib.sandbox.SandboxHelpers; import com.google.devtools.build.lib.vfs.Path; -import io.grpc.Context; import java.io.IOException; import java.util.HashMap; import java.util.HashSet; @@ -167,48 +164,42 @@ private ListenableFuture downloadFileAsync(Path path, FileArtifactValue me if (download == null) { RequestMetadata requestMetadata = TracingMetadataUtils.buildMetadata(buildRequestId, commandId, metadata.getActionId()); - RemoteActionExecutionContext remoteActionExecutionContext = - new RemoteActionExecutionContextImpl(requestMetadata, new NetworkTime()); - Context ctx = TracingMetadataUtils.contextWithMetadata(requestMetadata); - Context prevCtx = ctx.attach(); - try { - Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize()); - download = remoteCache.downloadFile(remoteActionExecutionContext, path, digest); - downloadsInProgress.put(path, download); - Futures.addCallback( - download, - new FutureCallback() { - @Override - public void onSuccess(Void v) { - synchronized (lock) { - downloadsInProgress.remove(path); - downloadedPaths.add(path); - } - - try { - path.chmod(0755); - } catch (IOException e) { - logger.atWarning().withCause(e).log("Failed to chmod 755 on %s", path); - } + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata); + + Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize()); + download = remoteCache.downloadFile(context, path, digest); + downloadsInProgress.put(path, download); + Futures.addCallback( + download, + new FutureCallback() { + @Override + public void onSuccess(Void v) { + synchronized (lock) { + downloadsInProgress.remove(path); + downloadedPaths.add(path); } - @Override - public void onFailure(Throwable t) { - synchronized (lock) { - downloadsInProgress.remove(path); - } - try { - path.delete(); - } catch (IOException e) { - logger.atWarning().withCause(e).log( - "Failed to delete output file after incomplete download: %s", path); - } + try { + path.chmod(0755); + } catch (IOException e) { + logger.atWarning().withCause(e).log("Failed to chmod 755 on %s", path); } - }, - MoreExecutors.directExecutor()); - } finally { - ctx.detach(prevCtx); - } + } + + @Override + public void onFailure(Throwable t) { + synchronized (lock) { + downloadsInProgress.remove(path); + } + try { + path.delete(); + } catch (IOException e) { + logger.atWarning().withCause(e).log( + "Failed to delete output file after incomplete download: %s", path); + } + } + }, + MoreExecutors.directExecutor()); } return download; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java index 3496243044c913..6bfc730351d02b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java @@ -188,7 +188,8 @@ private void uploadOutputs( digests.addAll(digestToFile.keySet()); digests.addAll(digestToBlobs.keySet()); - ImmutableSet digestsToUpload = getFromFuture(cacheProtocol.findMissingDigests(digests)); + ImmutableSet digestsToUpload = + getFromFuture(cacheProtocol.findMissingDigests(context, digests)); ImmutableList.Builder> uploads = ImmutableList.builder(); for (Digest digest : digestsToUpload) { Path file = digestToFile.get(digest); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java index 439840492abddc..c0db565594bd55 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionCache.java @@ -62,7 +62,7 @@ public void ensureInputsPresent( Iterable allDigests = Iterables.concat(merkleTree.getAllDigests(), additionalInputs.keySet()); ImmutableSet missingDigests = - getFromFuture(cacheProtocol.findMissingDigests(allDigests)); + getFromFuture(cacheProtocol.findMissingDigests(context, allDigests)); List> uploadFutures = new ArrayList<>(); for (Digest missingDigest : missingDigests) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java index f4a05dc30a831a..a74c42f653e866 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java @@ -29,10 +29,8 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.OperationObserver; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; import com.google.devtools.build.lib.remote.merkletree.MerkleTree; @@ -43,7 +41,6 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.Message; -import io.grpc.Context; import java.io.IOException; import java.time.Duration; import java.util.Map; @@ -110,61 +107,48 @@ public ExecutionResult execute( throws IOException, InterruptedException { RequestMetadata metadata = TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "repository_rule"); - Context requestCtx = TracingMetadataUtils.contextWithMetadata(metadata); - Context prev = requestCtx.attach(); - try { - Platform platform = PlatformUtils.buildPlatformProto(executionProperties); - Command command = - RemoteSpawnRunner.buildCommand( - /* outputs= */ ImmutableList.of(), - arguments, - environment, - platform, - workingDirectory); - Digest commandHash = digestUtil.compute(command); - MerkleTree merkleTree = MerkleTree.build(inputFiles, digestUtil); - Action action = - RemoteSpawnRunner.buildAction( - commandHash, merkleTree.getRootDigest(), timeout, acceptCached); - Digest actionDigest = digestUtil.compute(action); - ActionKey actionKey = new ActionKey(actionDigest); - RemoteActionExecutionContext remoteActionExecutionContext = - new RemoteActionExecutionContextImpl(metadata, new NetworkTime()); - ActionResult actionResult; + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(metadata); + + Platform platform = PlatformUtils.buildPlatformProto(executionProperties); + Command command = + RemoteSpawnRunner.buildCommand( + /* outputs= */ ImmutableList.of(), arguments, environment, platform, workingDirectory); + Digest commandHash = digestUtil.compute(command); + MerkleTree merkleTree = MerkleTree.build(inputFiles, digestUtil); + Action action = + RemoteSpawnRunner.buildAction( + commandHash, merkleTree.getRootDigest(), timeout, acceptCached); + Digest actionDigest = digestUtil.compute(action); + ActionKey actionKey = new ActionKey(actionDigest); + ActionResult actionResult; + try (SilentCloseable c = + Profiler.instance().profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { + actionResult = remoteCache.downloadActionResult(context, actionKey, /* inlineOutErr= */ true); + } + if (actionResult == null || actionResult.getExitCode() != 0) { try (SilentCloseable c = - Profiler.instance().profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { - actionResult = - remoteCache.downloadActionResult( - remoteActionExecutionContext, actionKey, /* inlineOutErr= */ true); - } - if (actionResult == null || actionResult.getExitCode() != 0) { - try (SilentCloseable c = - Profiler.instance().profile(ProfilerTask.UPLOAD_TIME, "upload missing inputs")) { - Map additionalInputs = Maps.newHashMapWithExpectedSize(2); - additionalInputs.put(actionDigest, action); - additionalInputs.put(commandHash, command); + Profiler.instance().profile(ProfilerTask.UPLOAD_TIME, "upload missing inputs")) { + Map additionalInputs = Maps.newHashMapWithExpectedSize(2); + additionalInputs.put(actionDigest, action); + additionalInputs.put(commandHash, command); - remoteCache.ensureInputsPresent( - remoteActionExecutionContext, merkleTree, additionalInputs); - } + remoteCache.ensureInputsPresent(context, merkleTree, additionalInputs); + } - try (SilentCloseable c = - Profiler.instance().profile(ProfilerTask.REMOTE_EXECUTION, "execute remotely")) { - ExecuteRequest executeRequest = - ExecuteRequest.newBuilder() - .setActionDigest(actionDigest) - .setInstanceName(remoteInstanceName) - .setSkipCacheLookup(!acceptCached) - .build(); + try (SilentCloseable c = + Profiler.instance().profile(ProfilerTask.REMOTE_EXECUTION, "execute remotely")) { + ExecuteRequest executeRequest = + ExecuteRequest.newBuilder() + .setActionDigest(actionDigest) + .setInstanceName(remoteInstanceName) + .setSkipCacheLookup(!acceptCached) + .build(); - ExecuteResponse response = - remoteExecutor.executeRemotely(executeRequest, OperationObserver.NO_OP); - actionResult = response.getResult(); - } + ExecuteResponse response = + remoteExecutor.executeRemotely(context, executeRequest, OperationObserver.NO_OP); + actionResult = response.getResult(); } - return downloadOutErr(remoteActionExecutionContext, actionResult); - } finally { - requestCtx.detach(prev); } + return downloadOutErr(context, actionResult); } } 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 bd528c394bf4f3..2cc9d813b99c54 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 @@ -22,13 +22,14 @@ import build.bazel.remote.execution.v2.GetCapabilitiesRequest; import build.bazel.remote.execution.v2.PriorityCapabilities; import build.bazel.remote.execution.v2.PriorityCapabilities.PriorityRange; +import build.bazel.remote.execution.v2.RequestMetadata; import build.bazel.remote.execution.v2.ServerCapabilities; import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import io.grpc.CallCredentials; -import io.grpc.Context; import io.grpc.StatusRuntimeException; import java.io.IOException; import java.util.List; @@ -57,31 +58,30 @@ public RemoteServerCapabilities( this.retrier = retrier; } - private CapabilitiesBlockingStub capabilitiesBlockingStub() { + private CapabilitiesBlockingStub capabilitiesBlockingStub(RemoteActionExecutionContext context) { return CapabilitiesGrpc.newBlockingStub(channel) - .withInterceptors(TracingMetadataUtils.attachMetadataFromContextInterceptor()) + .withInterceptors( + TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata())) .withCallCredentials(callCredentials) .withDeadlineAfter(callTimeoutSecs, TimeUnit.SECONDS); } public ServerCapabilities get(String buildRequestId, String commandId) throws IOException, InterruptedException { - Context withMetadata = - TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, "capabilities"); - Context previous = withMetadata.attach(); + RequestMetadata metadata = + TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "capabilities"); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(metadata); try { GetCapabilitiesRequest request = instanceName == null ? GetCapabilitiesRequest.getDefaultInstance() : GetCapabilitiesRequest.newBuilder().setInstanceName(instanceName).build(); - return retrier.execute(() -> capabilitiesBlockingStub().getCapabilities(request)); + return retrier.execute(() -> capabilitiesBlockingStub(context).getCapabilities(request)); } catch (StatusRuntimeException e) { if (e.getCause() instanceof IOException) { throw (IOException) e.getCause(); } throw new IOException(e); - } finally { - withMetadata.detach(previous); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index d213beea64c1cf..ba70436abc89a8 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -24,6 +24,7 @@ import build.bazel.remote.execution.v2.Command; import build.bazel.remote.execution.v2.Digest; import build.bazel.remote.execution.v2.Platform; +import build.bazel.remote.execution.v2.RequestMetadata; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; import com.google.common.base.Throwables; @@ -48,9 +49,7 @@ import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.merkletree.MerkleTree; import com.google.devtools.build.lib.remote.options.RemoteOptions; @@ -61,7 +60,6 @@ import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; -import io.grpc.Context; import java.io.IOException; import java.util.Collection; import java.util.HashSet; @@ -125,7 +123,6 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) return SpawnCache.NO_RESULT_NO_STORE; } - NetworkTime networkTime = new NetworkTime(); Stopwatch totalTime = Stopwatch.createStarted(); SortedMap inputMap = context.getInputMapping(); @@ -154,14 +151,11 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) // Look up action cache, and reuse the action output if it is found. ActionKey actionKey = digestUtil.computeActionKey(action); + RequestMetadata metadata = + TracingMetadataUtils.buildMetadata( + buildRequestId, commandId, actionKey.getDigest().getHash()); RemoteActionExecutionContext remoteActionExecutionContext = - new RemoteActionExecutionContextImpl( - TracingMetadataUtils.buildMetadata( - buildRequestId, commandId, actionKey.getDigest().getHash()), - networkTime); - Context withMetadata = - TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey) - .withValue(NetworkTimeInterceptor.CONTEXT_KEY, networkTime); + RemoteActionExecutionContext.create(metadata); Profiler prof = Profiler.instance(); if (options.remoteAcceptCached @@ -169,7 +163,6 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) context.report(ProgressStatus.CHECKING_CACHE, "remote-cache"); // Metadata will be available in context.current() until we detach. // This is done via a thread-local variable. - Context previous = withMetadata.attach(); try { ActionResult result; try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { @@ -220,7 +213,7 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) spawnMetrics .setFetchTime(fetchTime.elapsed()) .setTotalTime(totalTime.elapsed()) - .setNetworkTime(networkTime.getDuration()); + .setNetworkTime(remoteActionExecutionContext.getNetworkTime().getDuration()); SpawnResult spawnResult = createSpawnResult( result.getExitCode(), @@ -250,8 +243,6 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) errorMessage = "Reading from Remote Cache:\n" + errorMessage; report(Event.warn(errorMessage)); } - } finally { - withMetadata.detach(previous); } } @@ -291,7 +282,6 @@ public void store(SpawnResult result) throws ExecException, InterruptedException } } - Context previous = withMetadata.attach(); Collection files = RemoteSpawnRunner.resolveActionInputs(execRoot, spawn.getOutputFiles()); try (SilentCloseable c = prof.profile(ProfilerTask.UPLOAD_TIME, "upload outputs")) { @@ -316,8 +306,6 @@ public void store(SpawnResult result) throws ExecException, InterruptedException } errorMessage = "Writing to Remote Cache:\n" + errorMessage; report(Event.warn(errorMessage)); - } finally { - withMetadata.detach(previous); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 3e9486c9b34907..49777032608b57 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -35,6 +35,7 @@ import build.bazel.remote.execution.v2.ExecutionStage.Value; import build.bazel.remote.execution.v2.LogFile; import build.bazel.remote.execution.v2.Platform; +import build.bazel.remote.execution.v2.RequestMetadata; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; @@ -66,10 +67,8 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.OperationObserver; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; import com.google.devtools.build.lib.remote.merkletree.MerkleTree; @@ -91,7 +90,6 @@ import com.google.protobuf.Timestamp; import com.google.protobuf.util.Durations; import com.google.protobuf.util.Timestamps; -import io.grpc.Context; import io.grpc.Status.Code; import java.io.IOException; import java.time.Duration; @@ -243,169 +241,165 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) Preconditions.checkArgument( Spawns.mayBeExecutedRemotely(spawn), "Spawn can't be executed remotely. This is a bug."); - NetworkTime networkTime = new NetworkTime(); // Look up action cache, and reuse the action output if it is found. ActionKey actionKey = digestUtil.computeActionKey(action); + RequestMetadata metadata = + TracingMetadataUtils.buildMetadata( + buildRequestId, commandId, actionKey.getDigest().getHash()); RemoteActionExecutionContext remoteActionExecutionContext = - new RemoteActionExecutionContextImpl( - TracingMetadataUtils.buildMetadata( - buildRequestId, commandId, actionKey.getDigest().getHash()), - networkTime); - Context withMetadata = - TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey) - .withValue(NetworkTimeInterceptor.CONTEXT_KEY, networkTime); - Context previous = withMetadata.attach(); + RemoteActionExecutionContext.create(metadata); Profiler prof = Profiler.instance(); try { - try { - // Try to lookup the action in the action cache. - ActionResult cachedResult; - try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { - cachedResult = - acceptCachedResult - ? remoteCache.downloadActionResult( - remoteActionExecutionContext, actionKey, /* inlineOutErr= */ false) - : null; - } - if (cachedResult != null) { - if (cachedResult.getExitCode() != 0) { - // Failed actions are treated as a cache miss mostly in order to avoid caching flaky - // actions (tests). - // Set acceptCachedResult to false in order to force the action re-execution + // Try to lookup the action in the action cache. + ActionResult cachedResult; + try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { + cachedResult = + acceptCachedResult + ? remoteCache.downloadActionResult( + remoteActionExecutionContext, actionKey, /* inlineOutErr= */ false) + : null; + } + if (cachedResult != null) { + if (cachedResult.getExitCode() != 0) { + // Failed actions are treated as a cache miss mostly in order to avoid caching flaky + // actions (tests). + // Set acceptCachedResult to false in order to force the action re-execution + acceptCachedResult = false; + } else { + try { + return downloadAndFinalizeSpawnResult( + remoteActionExecutionContext, + actionKey.getDigest().getHash(), + cachedResult, + /* cacheHit= */ true, + spawn, + context, + remoteOutputsMode, + totalTime, + () -> remoteActionExecutionContext.getNetworkTime().getDuration(), + spawnMetrics); + } catch (BulkTransferException e) { + if (!e.onlyCausedByCacheNotFoundException()) { + throw e; + } + // No cache hit, so we fall through to local or remote execution. + // We set acceptCachedResult to false in order to force the action re-execution. acceptCachedResult = false; - } else { + } + } + } + } catch (IOException e) { + return execLocallyAndUploadOrFail( + remoteActionExecutionContext, + spawn, + context, + inputMap, + actionKey, + action, + command, + uploadLocalResults, + e); + } + + ExecuteRequest.Builder requestBuilder = + ExecuteRequest.newBuilder() + .setInstanceName(remoteOptions.remoteInstanceName) + .setActionDigest(actionKey.getDigest()) + .setSkipCacheLookup(!acceptCachedResult); + if (remoteOptions.remoteResultCachePriority != 0) { + requestBuilder + .getResultsCachePolicyBuilder() + .setPriority(remoteOptions.remoteResultCachePriority); + } + if (remoteOptions.remoteExecutionPriority != 0) { + requestBuilder.getExecutionPolicyBuilder().setPriority(remoteOptions.remoteExecutionPriority); + } + try { + return retrier.execute( + () -> { + ExecuteRequest request = requestBuilder.build(); + + // Upload the command and all the inputs into the remote cache. + try (SilentCloseable c = prof.profile(UPLOAD_TIME, "upload missing inputs")) { + Map additionalInputs = Maps.newHashMapWithExpectedSize(2); + additionalInputs.put(actionKey.getDigest(), action); + additionalInputs.put(commandHash, command); + Duration networkTimeStart = + remoteActionExecutionContext.getNetworkTime().getDuration(); + Stopwatch uploadTime = Stopwatch.createStarted(); + remoteCache.ensureInputsPresent( + remoteActionExecutionContext, merkleTree, additionalInputs); + // subtract network time consumed here to ensure wall clock during upload is not + // double + // counted, and metrics time computation does not exceed total time + spawnMetrics.setUploadTime( + uploadTime + .elapsed() + .minus( + remoteActionExecutionContext + .getNetworkTime() + .getDuration() + .minus(networkTimeStart))); + } + + ExecutingStatusReporter reporter = new ExecutingStatusReporter(context); + ExecuteResponse reply; + try (SilentCloseable c = prof.profile(REMOTE_EXECUTION, "execute remotely")) { + reply = + remoteExecutor.executeRemotely(remoteActionExecutionContext, request, reporter); + } + // In case of replies from server contains metadata, but none of them has EXECUTING + // status. + // It's already late at this stage, but we should at least report once. + reporter.reportExecutingIfNot(); + + FileOutErr outErr = context.getFileOutErr(); + String message = reply.getMessage(); + ActionResult actionResult = reply.getResult(); + if ((actionResult.getExitCode() != 0 || reply.getStatus().getCode() != Code.OK.value()) + && !message.isEmpty()) { + outErr.printErr(message + "\n"); + } + + spawnMetricsAccounting(spawnMetrics, actionResult.getExecutionMetadata()); + + try (SilentCloseable c = prof.profile(REMOTE_DOWNLOAD, "download server logs")) { + maybeDownloadServerLogs(remoteActionExecutionContext, reply, actionKey); + } + try { return downloadAndFinalizeSpawnResult( remoteActionExecutionContext, actionKey.getDigest().getHash(), - cachedResult, - /* cacheHit= */ true, + actionResult, + reply.getCachedResult(), spawn, context, remoteOutputsMode, totalTime, - networkTime::getDuration, + () -> remoteActionExecutionContext.getNetworkTime().getDuration(), spawnMetrics); } catch (BulkTransferException e) { - if (!e.onlyCausedByCacheNotFoundException()) { - throw e; + if (e.onlyCausedByCacheNotFoundException()) { + // No cache hit, so if we retry this execution, we must no longer accept + // cached results, it must be reexecuted + requestBuilder.setSkipCacheLookup(true); } - // No cache hit, so we fall through to local or remote execution. - // We set acceptCachedResult to false in order to force the action re-execution. - acceptCachedResult = false; + throw e; } - } - } - } catch (IOException e) { - return execLocallyAndUploadOrFail( - remoteActionExecutionContext, - spawn, - context, - inputMap, - actionKey, - action, - command, - uploadLocalResults, - e); - } - - ExecuteRequest.Builder requestBuilder = - ExecuteRequest.newBuilder() - .setInstanceName(remoteOptions.remoteInstanceName) - .setActionDigest(actionKey.getDigest()) - .setSkipCacheLookup(!acceptCachedResult); - if (remoteOptions.remoteResultCachePriority != 0) { - requestBuilder - .getResultsCachePolicyBuilder() - .setPriority(remoteOptions.remoteResultCachePriority); - } - if (remoteOptions.remoteExecutionPriority != 0) { - requestBuilder - .getExecutionPolicyBuilder() - .setPriority(remoteOptions.remoteExecutionPriority); - } - try { - return retrier.execute( - () -> { - ExecuteRequest request = requestBuilder.build(); - - // Upload the command and all the inputs into the remote cache. - try (SilentCloseable c = prof.profile(UPLOAD_TIME, "upload missing inputs")) { - Map additionalInputs = Maps.newHashMapWithExpectedSize(2); - additionalInputs.put(actionKey.getDigest(), action); - additionalInputs.put(commandHash, command); - Duration networkTimeStart = networkTime.getDuration(); - Stopwatch uploadTime = Stopwatch.createStarted(); - remoteCache.ensureInputsPresent( - remoteActionExecutionContext, merkleTree, additionalInputs); - // subtract network time consumed here to ensure wall clock during upload is not - // double - // counted, and metrics time computation does not exceed total time - spawnMetrics.setUploadTime( - uploadTime.elapsed().minus(networkTime.getDuration().minus(networkTimeStart))); - } - - ExecutingStatusReporter reporter = new ExecutingStatusReporter(context); - ExecuteResponse reply; - try (SilentCloseable c = prof.profile(REMOTE_EXECUTION, "execute remotely")) { - reply = remoteExecutor.executeRemotely(request, reporter); - } - // In case of replies from server contains metadata, but none of them has EXECUTING - // status. - // It's already late at this stage, but we should at least report once. - reporter.reportExecutingIfNot(); - - FileOutErr outErr = context.getFileOutErr(); - String message = reply.getMessage(); - ActionResult actionResult = reply.getResult(); - if ((actionResult.getExitCode() != 0 - || reply.getStatus().getCode() != Code.OK.value()) - && !message.isEmpty()) { - outErr.printErr(message + "\n"); - } - - spawnMetricsAccounting(spawnMetrics, actionResult.getExecutionMetadata()); - - try (SilentCloseable c = prof.profile(REMOTE_DOWNLOAD, "download server logs")) { - maybeDownloadServerLogs(remoteActionExecutionContext, reply, actionKey); - } - - try { - return downloadAndFinalizeSpawnResult( - remoteActionExecutionContext, - actionKey.getDigest().getHash(), - actionResult, - reply.getCachedResult(), - spawn, - context, - remoteOutputsMode, - totalTime, - networkTime::getDuration, - spawnMetrics); - } catch (BulkTransferException e) { - if (e.onlyCausedByCacheNotFoundException()) { - // No cache hit, so if we retry this execution, we must no longer accept - // cached results, it must be reexecuted - requestBuilder.setSkipCacheLookup(true); - } - throw e; - } - }); - } catch (IOException e) { - return execLocallyAndUploadOrFail( - remoteActionExecutionContext, - spawn, - context, - inputMap, - actionKey, - action, - command, - uploadLocalResults, - e); - } - } finally { - withMetadata.detach(previous); + }); + } catch (IOException e) { + return execLocallyAndUploadOrFail( + remoteActionExecutionContext, + spawn, + context, + inputMap, + actionKey, + action, + command, + uploadLocalResults, + e); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/MissingDigestsFinder.java b/src/main/java/com/google/devtools/build/lib/remote/common/MissingDigestsFinder.java index c9c1d402ba4335..f682e93ca4d197 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/MissingDigestsFinder.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/MissingDigestsFinder.java @@ -26,5 +26,6 @@ public interface MissingDigestsFinder { * * @param digests The list of digests to look for. */ - ListenableFuture> findMissingDigests(Iterable digests); + ListenableFuture> findMissingDigests( + RemoteActionExecutionContext context, Iterable digests); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContext.java index 6d61cd0dbdb5e1..bc3d76634c50a5 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContext.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContext.java @@ -26,4 +26,9 @@ public interface RemoteActionExecutionContext { * execution. */ NetworkTime getNetworkTime(); + + /** Creates a {@link SimpleRemoteActionExecutionContext} with given {@link RequestMetadata}. */ + static RemoteActionExecutionContext create(RequestMetadata metadata) { + return new SimpleRemoteActionExecutionContext(metadata, new NetworkTime()); + } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteExecutionClient.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteExecutionClient.java index 1db02ea062f12e..ac2c283ee8ff2d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteExecutionClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteExecutionClient.java @@ -25,7 +25,8 @@ public interface RemoteExecutionClient { /** Execute an action remotely using Remote Execution API. */ - ExecuteResponse executeRemotely(ExecuteRequest request, OperationObserver observer) + ExecuteResponse executeRemotely( + RemoteActionExecutionContext context, ExecuteRequest request, OperationObserver observer) throws IOException, InterruptedException; /** Close resources associated with the remote execution client. */ diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContextImpl.java b/src/main/java/com/google/devtools/build/lib/remote/common/SimpleRemoteActionExecutionContext.java similarity index 89% rename from src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContextImpl.java rename to src/main/java/com/google/devtools/build/lib/remote/common/SimpleRemoteActionExecutionContext.java index f4894e31740778..a0d82c798bce24 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContextImpl.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/SimpleRemoteActionExecutionContext.java @@ -16,12 +16,12 @@ import build.bazel.remote.execution.v2.RequestMetadata; /** A {@link RemoteActionExecutionContext} implementation */ -public class RemoteActionExecutionContextImpl implements RemoteActionExecutionContext { +public class SimpleRemoteActionExecutionContext implements RemoteActionExecutionContext { private final RequestMetadata requestMetadata; private final NetworkTime networkTime; - public RemoteActionExecutionContextImpl( + public SimpleRemoteActionExecutionContext( RequestMetadata requestMetadata, NetworkTime networkTime) { this.requestMetadata = requestMetadata; this.networkTime = networkTime; diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java index 60c501677d0cd9..b34098556a8c7b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java @@ -97,9 +97,12 @@ public ListenableFuture uploadBlob( } @Override - public ListenableFuture> findMissingDigests(Iterable digests) { - ListenableFuture> remoteQuery = remoteCache.findMissingDigests(digests); - ListenableFuture> diskQuery = diskCache.findMissingDigests(digests); + public ListenableFuture> findMissingDigests( + RemoteActionExecutionContext context, Iterable digests) { + ListenableFuture> remoteQuery = + remoteCache.findMissingDigests(context, digests); + ListenableFuture> diskQuery = + diskCache.findMissingDigests(context, digests); return Futures.whenAllSucceed(remoteQuery, diskQuery) .call( () -> diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java index 6778525ccca254..26ce20ad35867f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java @@ -143,7 +143,8 @@ public ListenableFuture uploadBlob( } @Override - public ListenableFuture> findMissingDigests(Iterable digests) { + public ListenableFuture> findMissingDigests( + RemoteActionExecutionContext context, Iterable digests) { // Both upload and download check if the file exists before doing I/O. So we don't // have to do it here. return Futures.immediateFuture(ImmutableSet.copyOf(digests)); 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 672207bb68e71a..71b22c7782e3c6 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 @@ -29,9 +29,7 @@ import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.remote.ReferenceCountedChannel; import com.google.devtools.build.lib.remote.RemoteRetrier; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; @@ -118,7 +116,7 @@ public void download( RequestMetadata metadata = TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "remote_downloader"); RemoteActionExecutionContext remoteActionExecutionContext = - new RemoteActionExecutionContextImpl(metadata, new NetworkTime()); + RemoteActionExecutionContext.create(metadata); final FetchBlobRequest request = newFetchBlobRequest(options.remoteInstanceName, urls, authHeaders, checksum, canonicalId); diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java index 1d8a922299abab..1efecd3bb160ae 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java @@ -690,7 +690,8 @@ public ListenableFuture uploadBlob( } @Override - public ListenableFuture> findMissingDigests(Iterable digests) { + public ListenableFuture> findMissingDigests( + RemoteActionExecutionContext context, Iterable digests) { return Futures.immediateFuture(ImmutableSet.copyOf(digests)); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/TracingMetadataUtils.java b/src/main/java/com/google/devtools/build/lib/remote/util/TracingMetadataUtils.java index cb880ce9525ac0..93cf8a36a5ff9d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/TracingMetadataUtils.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/TracingMetadataUtils.java @@ -18,7 +18,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; -import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.options.RemoteOptions; import io.grpc.ClientInterceptor; import io.grpc.Context; @@ -46,18 +45,6 @@ private TracingMetadataUtils() {} public static final Metadata.Key METADATA_KEY = ProtoUtils.keyForProto(RequestMetadata.getDefaultInstance()); - /** - * Returns a new gRPC context derived from the current context, with {@link RequestMetadata} - * accessible by the {@link fromCurrentContext()} method. - * - *

The {@link RequestMetadata} is constructed using the provided arguments and the current tool - * version. - */ - public static Context contextWithMetadata( - String buildRequestId, String commandId, ActionKey actionKey) { - return contextWithMetadata(buildRequestId, commandId, actionKey.getDigest().getHash()); - } - public static RequestMetadata buildMetadata( String buildRequestId, String commandId, String actionId) { Preconditions.checkNotNull(buildRequestId); @@ -74,27 +61,10 @@ public static RequestMetadata buildMetadata( .build(); } - /** - * Returns a new gRPC context derived from the current context, with {@link RequestMetadata} - * accessible by the {@link fromCurrentContext()} method. - * - *

The {@link RequestMetadata} is constructed using the provided arguments and the current tool - * version. - */ - public static Context contextWithMetadata( - String buildRequestId, String commandId, String actionId) { - RequestMetadata metadata = buildMetadata(buildRequestId, commandId, actionId); - return contextWithMetadata(metadata); - } - - public static Context contextWithMetadata(RequestMetadata metadata) { - return Context.current().withValue(CONTEXT_KEY, metadata); - } - /** * Fetches a {@link RequestMetadata} defined on the current context. * - * @throws {@link IllegalStateException} when the metadata is not defined in the current context. + * @throws IllegalStateException when the metadata is not defined in the current context. */ public static RequestMetadata fromCurrentContext() { RequestMetadata metadata = CONTEXT_KEY.get(); @@ -104,16 +74,6 @@ public static RequestMetadata fromCurrentContext() { return metadata; } - /** - * Creates a {@link Metadata} containing the {@link RequestMetadata} defined on the current - * context. - * - * @throws {@link IllegalStateException} when the metadata is not defined in the current context. - */ - public static Metadata headersFromCurrentContext() { - return headersFromRequestMetadata(fromCurrentContext()); - } - /** Creates a {@link Metadata} containing the {@link RequestMetadata}. */ public static Metadata headersFromRequestMetadata(RequestMetadata requestMetadata) { Metadata headers = new Metadata(); @@ -129,10 +89,6 @@ public static Metadata headersFromRequestMetadata(RequestMetadata requestMetadat return headers.get(METADATA_KEY); } - public static ClientInterceptor attachMetadataFromContextInterceptor() { - return MetadataUtils.newAttachHeadersInterceptor(headersFromCurrentContext()); - } - public static ClientInterceptor attachMetadataInterceptor(RequestMetadata requestMetadata) { return MetadataUtils.newAttachHeadersInterceptor(headersFromRequestMetadata(requestMetadata)); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java index e633f2c701b05d..79badf17285142 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ByteStreamBuildEventArtifactUploaderTest.java @@ -48,15 +48,14 @@ import com.google.devtools.build.lib.remote.ByteStreamUploaderTest.FixedBackoff; import com.google.devtools.build.lib.remote.ByteStreamUploaderTest.MaybeFailOnceUploadService; import com.google.devtools.build.lib.remote.common.MissingDigestsFinder; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TestUtils; -import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; -import io.grpc.Context; import io.grpc.ManagedChannel; import io.grpc.Server; import io.grpc.Status; @@ -93,8 +92,6 @@ public class ByteStreamBuildEventArtifactUploaderTest { private Server server; private ManagedChannel channel; - private Context withEmptyMetadata; - private Context prevContext; private final FileSystem fs = new InMemoryFileSystem(new JavaClock(), DigestHashFunction.SHA256); private final Path execRoot = fs.getPath("/execroot"); @@ -111,12 +108,6 @@ public final void setUp() throws Exception { .build() .start(); channel = InProcessChannelBuilder.forName(serverName).build(); - withEmptyMetadata = - TracingMetadataUtils.contextWithMetadata( - "none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance())); - // Needs to be repeated in every test that uses the timeout setting, since the tests run - // on different threads than the setUp. - prevContext = withEmptyMetadata.attach(); outputRoot = ArtifactRoot.asDerivedRoot(execRoot, "out"); outputRoot.getRoot().asPath().createDirectoryAndParents(); @@ -126,9 +117,6 @@ public final void setUp() throws Exception { @After public void tearDown() throws Exception { - // Needs to be repeated in every test that uses the timeout setting, since the tests run - // on different threads than the tearDown. - withEmptyMetadata.detach(prevContext); retryService.shutdownNow(); retryService.awaitTermination( @@ -348,7 +336,7 @@ public void remoteFileShouldNotBeUploaded_findMissingDigests() throws Exception PathConverter pathConverter = artifactUploader.upload(files).get(); // assert - verify(digestQuerier).findMissingDigests(any()); + verify(digestQuerier).findMissingDigests(any(), any()); verify(uploader).uploadBlobAsync(any(), eq(localDigest), any(), anyBoolean()); assertThat(pathConverter.apply(remoteFile)).contains(remoteDigest.getHash()); assertThat(pathConverter.apply(localFile)).contains(localDigest.getHash()); @@ -392,7 +380,8 @@ public StaticMissingDigestsFinder(ImmutableSet knownDigests) { } @Override - public ListenableFuture> findMissingDigests(Iterable digests) { + public ListenableFuture> findMissingDigests( + RemoteActionExecutionContext context, Iterable digests) { ImmutableSet.Builder missingDigests = ImmutableSet.builder(); for (Digest digest : digests) { if (!knownDigests.contains(digest)) { @@ -408,7 +397,8 @@ private static class AllMissingDigestsFinder implements MissingDigestsFinder { public static final AllMissingDigestsFinder INSTANCE = new AllMissingDigestsFinder(); @Override - public ListenableFuture> findMissingDigests(Iterable digests) { + public ListenableFuture> findMissingDigests( + RemoteActionExecutionContext context, Iterable digests) { return Futures.immediateFuture(ImmutableSet.copyOf(digests)); } } 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 29cd41eb4ef88f..a53278765492d7 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 @@ -36,9 +36,7 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.analysis.BlazeVersionInfo; import com.google.devtools.build.lib.authandtls.CallCredentialsProvider; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TestUtils; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; @@ -46,7 +44,6 @@ import com.google.protobuf.ByteString; import io.grpc.BindableService; import io.grpc.CallCredentials; -import io.grpc.Context; import io.grpc.ManagedChannel; import io.grpc.Metadata; import io.grpc.Server; @@ -104,8 +101,6 @@ public class ByteStreamUploaderTest { private Server server; private ManagedChannel channel; private RemoteActionExecutionContext context; - private Context withEmptyMetadata; - private Context prevContext; @Mock private Retrier.Backoff mockBackoff; @@ -125,22 +120,13 @@ public final void setUp() throws Exception { "none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance()).getDigest().getHash()); - context = new RemoteActionExecutionContextImpl(metadata, new NetworkTime()); - withEmptyMetadata = TracingMetadataUtils.contextWithMetadata(metadata); + context = RemoteActionExecutionContext.create(metadata); retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1)); - - // Needs to be repeated in every test that uses the timeout setting, since the tests run - // on different threads than the setUp. - prevContext = withEmptyMetadata.attach(); } @After public void tearDown() throws Exception { - // Needs to be repeated in every test that uses the timeout setting, since the tests run - // on different threads than the tearDown. - withEmptyMetadata.detach(prevContext); - retryService.shutdownNow(); retryService.awaitTermination( com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS); @@ -153,7 +139,6 @@ public void tearDown() throws Exception { @Test public void singleBlobUploadShouldWork() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> mockBackoff, (e) -> true, retryService); ByteStreamUploader uploader = @@ -226,13 +211,10 @@ public void onCompleted() { Mockito.verifyZeroInteractions(mockBackoff); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void progressiveUploadShouldWork() throws Exception { - Context prevContext = withEmptyMetadata.attach(); Mockito.when(mockBackoff.getRetryAttempts()).thenReturn(0); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> mockBackoff, (e) -> true, retryService); @@ -345,15 +327,12 @@ public void queryWriteStatus( Mockito.verify(mockBackoff, Mockito.times(1)).getRetryAttempts(); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void concurrentlyCompletedUploadIsNotRetried() throws Exception { // Test that after an upload has failed and the QueryWriteStatus call returns // that the upload has completed that we'll not retry the upload. - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> new FixedBackoff(1, 0), (e) -> true, retryService); ByteStreamUploader uploader = @@ -408,13 +387,10 @@ public void queryWriteStatus( assertThat(numWriteCalls.get()).isEqualTo(1); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void unimplementedQueryShouldRestartUpload() throws Exception { - Context prevContext = withEmptyMetadata.attach(); Mockito.when(mockBackoff.getRetryAttempts()).thenReturn(0); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> mockBackoff, (e) -> true, retryService); @@ -483,13 +459,10 @@ public void queryWriteStatus( Mockito.verify(mockBackoff, Mockito.times(1)).nextDelayMillis(any(Exception.class)); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void earlyWriteResponseShouldCompleteUpload() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> mockBackoff, (e) -> true, retryService); ByteStreamUploader uploader = @@ -524,13 +497,10 @@ public StreamObserver write(StreamObserver streamOb Mockito.verifyZeroInteractions(mockBackoff); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void incorrectCommittedSizeFailsUpload() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> mockBackoff, (e) -> true, retryService); ByteStreamUploader uploader = @@ -569,13 +539,10 @@ public StreamObserver write(StreamObserver streamOb Mockito.verifyZeroInteractions(mockBackoff); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void multipleBlobsUploadShouldWork() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> new FixedBackoff(1, 0), (e) -> true, retryService); ByteStreamUploader uploader = @@ -605,13 +572,10 @@ public void multipleBlobsUploadShouldWork() throws Exception { uploader.uploadBlobs(context, chunkers, true); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void contextShouldBePreservedUponRetries() throws Exception { - Context prevContext = withEmptyMetadata.attach(); // We upload blobs with different context, and retry 3 times for each upload. // We verify that the correct metadata is passed to the server with every blob. RemoteRetrier retrier = @@ -706,7 +670,7 @@ public void queryWriteStatus( "command-id", DIGEST_UTIL.asActionKey(actionDigest).getDigest().getHash()); RemoteActionExecutionContext remoteActionExecutionContext = - new RemoteActionExecutionContextImpl(metadata, new NetworkTime()); + RemoteActionExecutionContext.create(metadata); uploads.add( uploader.uploadBlobAsync( remoteActionExecutionContext, @@ -720,8 +684,6 @@ public void queryWriteStatus( } blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test @@ -795,8 +757,6 @@ public ServerCall.Listener interceptCall( @Test public void sameBlobShouldNotBeUploadedTwice() throws Exception { // Test that uploading the same file concurrently triggers only one file upload. - - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> mockBackoff, (e) -> true, retryService); ByteStreamUploader uploader = @@ -859,13 +819,10 @@ public void onCompleted() { upload1.get(); assertThat(numWriteCalls.get()).isEqualTo(1); - - withEmptyMetadata.detach(prevContext); } @Test public void errorsShouldBeReported() throws IOException, InterruptedException { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> new FixedBackoff(1, 10), (e) -> true, retryService); ByteStreamUploader uploader = @@ -895,13 +852,10 @@ public StreamObserver write(StreamObserver response } catch (IOException e) { assertThat(RemoteRetrierUtils.causedByStatus(e, Code.INTERNAL)).isTrue(); } - - withEmptyMetadata.detach(prevContext); } @Test public void shutdownShouldCancelOngoingUploads() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> new FixedBackoff(1, 10), (e) -> true, retryService); ByteStreamUploader uploader = @@ -960,13 +914,10 @@ public void onCancel() { assertThat(f2.isCancelled()).isTrue(); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void failureInRetryExecutorShouldBeHandled() throws Exception { - Context prevContext = withEmptyMetadata.attach(); ListeningScheduledExecutorService retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1)); RemoteRetrier retrier = @@ -1003,13 +954,10 @@ public StreamObserver write(StreamObserver response } catch (IOException e) { assertThat(e).hasCauseThat().isInstanceOf(RejectedExecutionException.class); } - - withEmptyMetadata.detach(prevContext); } @Test public void resourceNameWithoutInstanceName() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> mockBackoff, (e) -> true, retryService); ByteStreamUploader uploader = @@ -1048,13 +996,10 @@ public void onCompleted() { HashCode hash = HashCode.fromString(DIGEST_UTIL.compute(blob).getHash()); uploader.uploadBlob(context, hash, chunker, true); - - withEmptyMetadata.detach(prevContext); } @Test public void nonRetryableStatusShouldNotBeRetried() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier( () -> new FixedBackoff(1, 0), /* No Status is retriable. */ (e) -> false, retryService); @@ -1088,13 +1033,10 @@ public StreamObserver write(StreamObserver response } catch (IOException e) { assertThat(numCalls.get()).isEqualTo(1); } - - withEmptyMetadata.detach(prevContext); } @Test public void failedUploadsShouldNotDeduplicate() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> Retrier.RETRIES_DISABLED, (e) -> false, retryService); ByteStreamUploader uploader = @@ -1169,13 +1111,10 @@ public void onCompleted() { assertThat(numUploads.get()).isEqualTo(2); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void deduplicationOfUploadsShouldWork() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier(() -> mockBackoff, (e) -> true, retryService); ByteStreamUploader uploader = @@ -1237,13 +1176,10 @@ public void onCompleted() { Mockito.verifyZeroInteractions(mockBackoff); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void unauthenticatedErrorShouldNotBeRetried() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier( () -> mockBackoff, RemoteRetrier.RETRIABLE_GRPC_ERRORS, retryService); @@ -1297,13 +1233,10 @@ public StreamObserver write(StreamObserver streamOb Mockito.verifyZeroInteractions(mockBackoff); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } @Test public void shouldRefreshCredentialsOnAuthenticationError() throws Exception { - Context prevContext = withEmptyMetadata.attach(); RemoteRetrier retrier = TestUtils.newRemoteRetrier( () -> mockBackoff, RemoteRetrier.RETRIABLE_GRPC_ERRORS, retryService); @@ -1385,8 +1318,6 @@ public void onCompleted() { Mockito.verifyZeroInteractions(mockBackoff); blockUntilInternalStateConsistent(uploader); - - withEmptyMetadata.detach(prevContext); } private static class NoopStreamObserver implements StreamObserver { diff --git a/src/test/java/com/google/devtools/build/lib/remote/ExperimentalGrpcRemoteExecutorTest.java b/src/test/java/com/google/devtools/build/lib/remote/ExperimentalGrpcRemoteExecutorTest.java index 95f32bee4b130d..44e915e7345da6 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ExperimentalGrpcRemoteExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ExperimentalGrpcRemoteExecutorTest.java @@ -28,13 +28,13 @@ import com.google.devtools.build.lib.authandtls.CallCredentialsProvider; import com.google.devtools.build.lib.remote.RemoteRetrier.ExponentialBackoff; import com.google.devtools.build.lib.remote.common.OperationObserver; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.TestUtils; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.common.options.Options; import com.google.longrunning.Operation; import com.google.rpc.Code; -import io.grpc.Context; import io.grpc.Server; import io.grpc.Status; import io.grpc.inprocess.InProcessChannelBuilder; @@ -53,12 +53,11 @@ @RunWith(JUnit4.class) public class ExperimentalGrpcRemoteExecutorTest { + private RemoteActionExecutionContext context; private FakeExecutionService executionService; private RemoteOptions remoteOptions; private Server fakeServer; private ListeningScheduledExecutorService retryService; - private Context context; - private Context prevContext; ExperimentalGrpcRemoteExecutor executor; private static final int MAX_RETRY_ATTEMPTS = 5; @@ -90,6 +89,8 @@ public class ExperimentalGrpcRemoteExecutorTest { @Before public final void setUp() throws Exception { + context = RemoteActionExecutionContext.create(RequestMetadata.getDefaultInstance()); + executionService = new FakeExecutionService(); String fakeServerName = "fake server for " + getClass(); @@ -117,9 +118,6 @@ public final void setUp() throws Exception { .directExecutor() .build()); - context = TracingMetadataUtils.contextWithMetadata(RequestMetadata.getDefaultInstance()); - prevContext = context.attach(); - executor = new ExperimentalGrpcRemoteExecutor( remoteOptions, channel.retain(), CallCredentialsProvider.NO_CREDENTIALS, retrier); @@ -129,9 +127,6 @@ public final void setUp() throws Exception { @After public void tearDown() throws Exception { - executor.close(); - context.detach(prevContext); - retryService.shutdownNow(); retryService.awaitTermination( com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, SECONDS); @@ -144,7 +139,8 @@ public void tearDown() throws Exception { public void executeRemotely_smoke() throws Exception { executionService.whenExecute(DUMMY_REQUEST).thenAck().thenAck().thenDone(DUMMY_RESPONSE); - ExecuteResponse response = executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + ExecuteResponse response = + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); assertThat(response).isEqualTo(DUMMY_RESPONSE); assertThat(executionService.getExecTimes()).isEqualTo(1); @@ -156,7 +152,8 @@ public void executeRemotely_errorInOperation_retryExecute() throws Exception { executionService.whenExecute(DUMMY_REQUEST).thenError(Code.UNAVAILABLE); executionService.whenExecute(DUMMY_REQUEST).thenAck().thenDone(DUMMY_RESPONSE); - ExecuteResponse response = executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + ExecuteResponse response = + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); assertThat(executionService.getExecTimes()).isEqualTo(3); assertThat(response).isEqualTo(DUMMY_RESPONSE); @@ -172,7 +169,8 @@ public void executeRemotely_errorInResponse_retryExecute() throws Exception { .build()); executionService.whenExecute(DUMMY_REQUEST).thenAck().thenDone(DUMMY_RESPONSE); - ExecuteResponse response = executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + ExecuteResponse response = + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); assertThat(executionService.getExecTimes()).isEqualTo(2); assertThat(response).isEqualTo(DUMMY_RESPONSE); @@ -192,7 +190,7 @@ public void executeRemotely_unretriableErrorInResponse_reportError() { assertThrows( IOException.class, () -> { - executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); }); assertThat(e).hasMessageThat().contains("INVALID_ARGUMENT"); @@ -209,7 +207,7 @@ public void executeRemotely_retryExecuteAndFail() { assertThrows( IOException.class, () -> { - executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); }); assertThat(executionService.getExecTimes()).isEqualTo(MAX_RETRY_ATTEMPTS + 1); @@ -221,7 +219,8 @@ public void executeRemotely_executeAndWait() throws Exception { executionService.whenExecute(DUMMY_REQUEST).thenAck().thenError(Code.UNAVAILABLE); executionService.whenWaitExecution(DUMMY_REQUEST).thenDone(DUMMY_RESPONSE); - ExecuteResponse response = executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + ExecuteResponse response = + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); assertThat(executionService.getExecTimes()).isEqualTo(1); assertThat(executionService.getWaitTimes()).isEqualTo(1); @@ -233,7 +232,8 @@ public void executeRemotely_executeAndRetryWait() throws Exception { executionService.whenExecute(DUMMY_REQUEST).thenAck().thenError(Code.UNAVAILABLE); executionService.whenWaitExecution(DUMMY_REQUEST).thenDone(DUMMY_RESPONSE); - ExecuteResponse response = executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + ExecuteResponse response = + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); assertThat(executionService.getExecTimes()).isEqualTo(1); assertThat(executionService.getWaitTimes()).isEqualTo(1); @@ -249,7 +249,8 @@ public void executeRemotely_executeAndRetryWait_forever() throws Exception { } executionService.whenWaitExecution(DUMMY_REQUEST).thenDone(DUMMY_RESPONSE); - ExecuteResponse response = executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + ExecuteResponse response = + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); assertThat(executionService.getExecTimes()).isEqualTo(1); assertThat(executionService.getWaitTimes()).isEqualTo(errorTimes + 1); @@ -266,7 +267,7 @@ public void executeRemotely_executeAndRetryWait_failForConsecutiveErrors() { assertThrows( IOException.class, () -> { - executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); }); assertThat(executionService.getExecTimes()).isEqualTo(1); @@ -288,7 +289,7 @@ public void executeRemotely_operationWithoutResult_shouldNotCrash() { assertThrows( IOException.class, () -> { - executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); }); assertThat(e).hasCauseThat().isInstanceOf(ExecutionStatusException.class); @@ -306,7 +307,7 @@ public void executeRemotely_responseWithoutResult_shouldNotCrash() { assertThrows( IOException.class, () -> { - executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); }); assertThat(e).hasCauseThat().isInstanceOf(ExecutionStatusException.class); @@ -322,7 +323,8 @@ public void executeRemotely_retryExecuteWhenUnauthenticated() executionService.whenExecute(DUMMY_REQUEST).thenError(Code.UNAUTHENTICATED); executionService.whenExecute(DUMMY_REQUEST).thenAck().thenDone(DUMMY_RESPONSE); - ExecuteResponse response = executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + ExecuteResponse response = + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); assertThat(executionService.getExecTimes()).isEqualTo(2); assertThat(response).isEqualTo(DUMMY_RESPONSE); @@ -335,7 +337,8 @@ public void executeRemotely_retryWaitExecutionWhenUnauthenticated() executionService.whenWaitExecution(DUMMY_REQUEST).thenAck().thenError(Code.UNAUTHENTICATED); executionService.whenWaitExecution(DUMMY_REQUEST).thenAck().thenDone(DUMMY_RESPONSE); - ExecuteResponse response = executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + ExecuteResponse response = + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); assertThat(executionService.getExecTimes()).isEqualTo(1); assertThat(executionService.getWaitTimes()).isEqualTo(2); @@ -349,7 +352,8 @@ public void executeRemotely_retryExecuteIfNotFound() throws IOException, Interru executionService.whenExecute(DUMMY_REQUEST).thenAck().thenError(Code.UNAVAILABLE); executionService.whenWaitExecution(DUMMY_REQUEST).thenDone(DUMMY_RESPONSE); - ExecuteResponse response = executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + ExecuteResponse response = + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); assertThat(executionService.getExecTimes()).isEqualTo(2); assertThat(executionService.getWaitTimes()).isEqualTo(2); @@ -367,7 +371,7 @@ public void executeRemotely_notFoundLoop_reportError() { assertThrows( IOException.class, () -> { - executor.executeRemotely(DUMMY_REQUEST, OperationObserver.NO_OP); + executor.executeRemotely(context, DUMMY_REQUEST, OperationObserver.NO_OP); }); assertThat(e).hasCauseThat().isInstanceOf(ExecutionStatusException.class); @@ -382,7 +386,7 @@ public void executeRemotely_notifyObserver() throws IOException, InterruptedExce executionService.whenExecute(DUMMY_REQUEST).thenAck().thenDone(DUMMY_RESPONSE); List notified = new ArrayList<>(); - executor.executeRemotely(DUMMY_REQUEST, notified::add); + executor.executeRemotely(context, DUMMY_REQUEST, notified::add); assertThat(notified) .containsExactly( diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java index c1f010429a152b..61e7b65925e3bf 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java @@ -35,6 +35,7 @@ import build.bazel.remote.execution.v2.FindMissingBlobsRequest; import build.bazel.remote.execution.v2.FindMissingBlobsResponse; import build.bazel.remote.execution.v2.GetActionResultRequest; +import build.bazel.remote.execution.v2.RequestMetadata; import build.bazel.remote.execution.v2.Tree; import build.bazel.remote.execution.v2.UpdateActionResultRequest; import com.google.api.client.json.GenericJson; @@ -61,9 +62,7 @@ import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.remote.RemoteRetrier.ExponentialBackoff; import com.google.devtools.build.lib.remote.Retrier.Backoff; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.merkletree.MerkleTree; import com.google.devtools.build.lib.remote.options.RemoteOptions; @@ -86,7 +85,6 @@ import io.grpc.Channel; import io.grpc.ClientCall; import io.grpc.ClientInterceptor; -import io.grpc.Context; import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Server; @@ -131,9 +129,7 @@ public class GrpcCacheClientTest { private final MutableHandlerRegistry serviceRegistry = new MutableHandlerRegistry(); private final String fakeServerName = "fake server for " + getClass(); private Server fakeServer; - private Context withEmptyMetadata; - private RemoteActionExecutionContext remoteActionExecutionContext; - private Context prevContext; + private RemoteActionExecutionContext context; private ListeningScheduledExecutorService retryService; @Before @@ -156,22 +152,14 @@ public final void setUp() throws Exception { FileSystemUtils.createDirectoryAndParents(stdout.getParentDirectory()); FileSystemUtils.createDirectoryAndParents(stderr.getParentDirectory()); outErr = new FileOutErr(stdout, stderr); - remoteActionExecutionContext = - new RemoteActionExecutionContextImpl( - TracingMetadataUtils.buildMetadata( - "none", "none", Digest.getDefaultInstance().getHash()), - new NetworkTime()); - withEmptyMetadata = - TracingMetadataUtils.contextWithMetadata(remoteActionExecutionContext.getRequestMetadata()); + RequestMetadata metadata = + TracingMetadataUtils.buildMetadata("none", "none", Digest.getDefaultInstance().getHash()); + context = RemoteActionExecutionContext.create(metadata); retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1)); - - prevContext = withEmptyMetadata.attach(); } @After public void tearDown() throws Exception { - withEmptyMetadata.detach(prevContext); - retryService.shutdownNow(); retryService.awaitTermination( com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS); @@ -317,7 +305,7 @@ public void onError(Throwable t) { }); // Upload all missing inputs (that is, the virtual action input from above) - client.ensureInputsPresent(remoteActionExecutionContext, merkleTree, ImmutableMap.of()); + client.ensureInputsPresent(context, merkleTree, ImmutableMap.of()); } @Test @@ -325,7 +313,7 @@ public void testDownloadEmptyBlob() throws Exception { GrpcCacheClient client = newClient(); Digest emptyDigest = DIGEST_UTIL.compute(new byte[0]); // Will not call the mock Bytestream interface at all. - assertThat(downloadBlob(remoteActionExecutionContext, client, emptyDigest)).isEmpty(); + assertThat(downloadBlob(context, client, emptyDigest)).isEmpty(); } @Test @@ -342,8 +330,7 @@ public void read(ReadRequest request, StreamObserver responseObser responseObserver.onCompleted(); } }); - assertThat(new String(downloadBlob(remoteActionExecutionContext, client, digest), UTF_8)) - .isEqualTo("abcdefg"); + assertThat(new String(downloadBlob(context, client, digest), UTF_8)).isEqualTo("abcdefg"); } @Test @@ -364,8 +351,7 @@ public void read(ReadRequest request, StreamObserver responseObser responseObserver.onCompleted(); } }); - assertThat(new String(downloadBlob(remoteActionExecutionContext, client, digest), UTF_8)) - .isEqualTo("abcdefg"); + assertThat(new String(downloadBlob(context, client, digest), UTF_8)).isEqualTo("abcdefg"); } @Test @@ -385,11 +371,7 @@ public void testDownloadAllResults() throws Exception { result.addOutputFilesBuilder().setPath("b/empty").setDigest(emptyDigest); result.addOutputFilesBuilder().setPath("a/bar").setDigest(barDigest).setIsExecutable(true); remoteCache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - /* outputFilesLocker= */ () -> {}); + context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); assertThat(DIGEST_UTIL.compute(execRoot.getRelative("b/empty"))).isEqualTo(emptyDigest); assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/bar"))).isEqualTo(barDigest); @@ -426,11 +408,7 @@ public void testDownloadDirectory() throws Exception { result.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest); result.addOutputDirectoriesBuilder().setPath("a/bar").setTreeDigest(barTreeDigest); remoteCache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - /* outputFilesLocker= */ () -> {}); + context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/bar/qux"))).isEqualTo(quxDigest); @@ -452,11 +430,7 @@ public void testDownloadDirectoryEmpty() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); result.addOutputDirectoriesBuilder().setPath("a/bar").setTreeDigest(barTreeDigest); remoteCache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - /* outputFilesLocker= */ () -> {}); + context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); assertThat(execRoot.getRelative("a/bar").isDirectory()).isTrue(); } @@ -499,11 +473,7 @@ public void testDownloadDirectoryNested() throws Exception { result.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest); result.addOutputDirectoriesBuilder().setPath("a/bar").setTreeDigest(barTreeDigest); remoteCache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - /* outputFilesLocker= */ () -> {}); + context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); assertThat(DIGEST_UTIL.compute(execRoot.getRelative("a/bar/wobble/qux"))).isEqualTo(quxDigest); @@ -723,8 +693,7 @@ private ActionResult uploadDirectory(RemoteCache remoteCache, List outputs Action action = Action.getDefaultInstance(); ActionKey actionKey = DIGEST_UTIL.computeActionKey(action); Command cmd = Command.getDefaultInstance(); - return remoteCache.upload( - remoteActionExecutionContext, actionKey, action, cmd, execRoot, outputs, outErr); + return remoteCache.upload(context, actionKey, action, cmd, execRoot, outputs, outErr); } @Test @@ -793,7 +762,7 @@ public void getActionResult( GrpcCacheClient client = newClient(remoteOptions); RemoteCache remoteCache = new RemoteCache(client, remoteOptions, DIGEST_UTIL); remoteCache.downloadActionResult( - remoteActionExecutionContext, + context, DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")), /* inlineOutErr= */ false); } @@ -850,7 +819,7 @@ public void updateActionResult( ActionResult result = remoteCache.upload( - remoteActionExecutionContext, + context, DIGEST_UTIL.asActionKey(actionDigest), action, command, @@ -913,7 +882,7 @@ public void updateActionResult( ActionResult result = remoteCache.upload( - remoteActionExecutionContext, + context, DIGEST_UTIL.asActionKey(actionDigest), action, command, @@ -1062,7 +1031,7 @@ public void onError(Throwable t) { .when(mockByteStreamImpl) .queryWriteStatus(any(), any()); remoteCache.upload( - remoteActionExecutionContext, + context, actionKey, Action.getDefaultInstance(), Command.getDefaultInstance(), @@ -1091,8 +1060,7 @@ public void getActionResult( }); assertThat( getFromFuture( - client.downloadActionResult( - remoteActionExecutionContext, actionKey, /* inlineOutErr= */ false))) + client.downloadActionResult(context, actionKey, /* inlineOutErr= */ false))) .isNull(); } @@ -1122,8 +1090,7 @@ public void read(ReadRequest request, StreamObserver responseObser } } }); - assertThat(new String(downloadBlob(remoteActionExecutionContext, client, digest), UTF_8)) - .isEqualTo("abcdefg"); + assertThat(new String(downloadBlob(context, client, digest), UTF_8)).isEqualTo("abcdefg"); Mockito.verify(mockBackoff, Mockito.never()).nextDelayMillis(any(Exception.class)); } @@ -1147,9 +1114,7 @@ public void read(ReadRequest request, StreamObserver responseObser responseObserver.onError(Status.DEADLINE_EXCEEDED.asException()); } }); - IOException e = - assertThrows( - IOException.class, () -> downloadBlob(remoteActionExecutionContext, client, digest)); + IOException e = assertThrows(IOException.class, () -> downloadBlob(context, client, digest)); Status st = Status.fromThrowable(e); assertThat(st.getCode()).isEqualTo(Status.Code.DEADLINE_EXCEEDED); Mockito.verify(mockBackoff, Mockito.times(1)).nextDelayMillis(any(Exception.class)); @@ -1170,9 +1135,7 @@ public void read(ReadRequest request, StreamObserver responseObser responseObserver.onCompleted(); } }); - IOException e = - assertThrows( - IOException.class, () -> downloadBlob(remoteActionExecutionContext, client, digest)); + IOException e = assertThrows(IOException.class, () -> downloadBlob(context, client, digest)); assertThat(e).hasMessageThat().contains(digest.getHash()); assertThat(e).hasMessageThat().contains(DIGEST_UTIL.computeAsUtf8("bar").getHash()); } @@ -1196,8 +1159,7 @@ public void read(ReadRequest request, StreamObserver responseObser } }); - assertThat(downloadBlob(remoteActionExecutionContext, client, digest)) - .isEqualTo(downloadContents.toByteArray()); + assertThat(downloadBlob(context, client, digest)).isEqualTo(downloadContents.toByteArray()); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java index cd9906bcc5761c..38cbfd93e2a73f 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTests.java @@ -33,6 +33,7 @@ import build.bazel.remote.execution.v2.FileNode; import build.bazel.remote.execution.v2.OutputDirectory; import build.bazel.remote.execution.v2.OutputFile; +import build.bazel.remote.execution.v2.RequestMetadata; import build.bazel.remote.execution.v2.SymlinkNode; import build.bazel.remote.execution.v2.Tree; import com.google.common.base.Throwables; @@ -54,9 +55,7 @@ import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.remote.RemoteCache.OutputFilesLocker; import com.google.devtools.build.lib.remote.RemoteCache.UploadManifest; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestUtil; @@ -102,7 +101,7 @@ public class RemoteCacheTests { @Mock private OutputFilesLocker outputFilesLocker; - private RemoteActionExecutionContext remoteActionExecutionContext; + private RemoteActionExecutionContext context; private FileSystem fs; private Path execRoot; ArtifactRoot artifactRoot; @@ -114,11 +113,9 @@ public class RemoteCacheTests { @Before public void setUp() throws Exception { MockitoAnnotations.initMocks(this); - remoteActionExecutionContext = - new RemoteActionExecutionContextImpl( - TracingMetadataUtils.buildMetadata( - "none", "none", Digest.getDefaultInstance().getHash()), - new NetworkTime()); + RequestMetadata metadata = + TracingMetadataUtils.buildMetadata("none", "none", Digest.getDefaultInstance().getHash()); + context = RemoteActionExecutionContext.create(metadata); fs = new InMemoryFileSystem(new JavaClock(), DigestHashFunction.SHA256); execRoot = fs.getPath("/execroot"); execRoot.createDirectoryAndParents(); @@ -547,7 +544,7 @@ public void downloadRelativeFileSymlink() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); result.addOutputFileSymlinksBuilder().setPath("a/b/link").setTarget("../../foo"); // Doesn't check for dangling links, hence download succeeds. - cache.download(remoteActionExecutionContext, result.build(), execRoot, null, outputFilesLocker); + cache.download(context, result.build(), execRoot, null, outputFilesLocker); Path path = execRoot.getRelative("a/b/link"); assertThat(path.isSymbolicLink()).isTrue(); assertThat(path.readSymbolicLink()).isEqualTo(PathFragment.create("../../foo")); @@ -560,7 +557,7 @@ public void downloadRelativeDirectorySymlink() throws Exception { ActionResult.Builder result = ActionResult.newBuilder(); result.addOutputDirectorySymlinksBuilder().setPath("a/b/link").setTarget("foo"); // Doesn't check for dangling links, hence download succeeds. - cache.download(remoteActionExecutionContext, result.build(), execRoot, null, outputFilesLocker); + cache.download(context, result.build(), execRoot, null, outputFilesLocker); Path path = execRoot.getRelative("a/b/link"); assertThat(path.isSymbolicLink()).isTrue(); assertThat(path.readSymbolicLink()).isEqualTo(PathFragment.create("foo")); @@ -576,11 +573,11 @@ public void downloadRelativeSymlinkInDirectory() throws Exception { Directory.newBuilder() .addSymlinks(SymlinkNode.newBuilder().setName("link").setTarget("../foo"))) .build(); - Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray()); + Digest treeDigest = cache.addContents(context, tree.toByteArray()); ActionResult.Builder result = ActionResult.newBuilder(); result.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest); // Doesn't check for dangling links, hence download succeeds. - cache.download(remoteActionExecutionContext, result.build(), execRoot, null, outputFilesLocker); + cache.download(context, result.build(), execRoot, null, outputFilesLocker); Path path = execRoot.getRelative("dir/link"); assertThat(path.isSymbolicLink()).isTrue(); assertThat(path.readSymbolicLink()).isEqualTo(PathFragment.create("../foo")); @@ -595,13 +592,7 @@ public void downloadAbsoluteDirectorySymlinkError() throws Exception { IOException expected = assertThrows( IOException.class, - () -> - cache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - outputFilesLocker)); + () -> cache.download(context, result.build(), execRoot, null, outputFilesLocker)); assertThat(expected).hasMessageThat().contains("/abs/link"); assertThat(expected).hasMessageThat().contains("absolute path"); verify(outputFilesLocker).lock(); @@ -615,13 +606,7 @@ public void downloadAbsoluteFileSymlinkError() throws Exception { IOException expected = assertThrows( IOException.class, - () -> - cache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - outputFilesLocker)); + () -> cache.download(context, result.build(), execRoot, null, outputFilesLocker)); assertThat(expected).hasMessageThat().contains("/abs/link"); assertThat(expected).hasMessageThat().contains("absolute path"); verify(outputFilesLocker).lock(); @@ -636,19 +621,13 @@ public void downloadAbsoluteSymlinkInDirectoryError() throws Exception { Directory.newBuilder() .addSymlinks(SymlinkNode.newBuilder().setName("link").setTarget("/foo"))) .build(); - Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray()); + Digest treeDigest = cache.addContents(context, tree.toByteArray()); ActionResult.Builder result = ActionResult.newBuilder(); result.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest); IOException expected = assertThrows( IOException.class, - () -> - cache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - outputFilesLocker)); + () -> cache.download(context, result.build(), execRoot, null, outputFilesLocker)); assertThat(expected.getSuppressed()).isEmpty(); assertThat(expected).hasMessageThat().contains("dir/link"); assertThat(expected).hasMessageThat().contains("/foo"); @@ -660,10 +639,10 @@ public void downloadAbsoluteSymlinkInDirectoryError() throws Exception { public void downloadFailureMaintainsDirectories() throws Exception { InMemoryRemoteCache cache = newRemoteCache(); Tree tree = Tree.newBuilder().setRoot(Directory.newBuilder()).build(); - Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray()); + Digest treeDigest = cache.addContents(context, tree.toByteArray()); Digest outputFileDigest = cache.addException("outputdir/outputfile", new IOException("download failed")); - Digest otherFileDigest = cache.addContents(remoteActionExecutionContext, "otherfile"); + Digest otherFileDigest = cache.addContents(context, "otherfile"); ActionResult.Builder result = ActionResult.newBuilder(); result.addOutputDirectoriesBuilder().setPath("outputdir").setTreeDigest(treeDigest); @@ -672,9 +651,7 @@ public void downloadFailureMaintainsDirectories() throws Exception { result.addOutputFiles(OutputFile.newBuilder().setPath("otherfile").setDigest(otherFileDigest)); assertThrows( BulkTransferException.class, - () -> - cache.download( - remoteActionExecutionContext, result.build(), execRoot, null, outputFilesLocker)); + () -> cache.download(context, result.build(), execRoot, null, outputFilesLocker)); assertThat(cache.getNumFailedDownloads()).isEqualTo(1); assertThat(execRoot.getRelative("outputdir").exists()).isTrue(); assertThat(execRoot.getRelative("outputdir/outputfile").exists()).isFalse(); @@ -692,9 +669,9 @@ public void onErrorWaitForRemainingDownloadsToComplete() throws Exception { Path stderr = fs.getPath("/execroot/stderr"); InMemoryRemoteCache cache = newRemoteCache(); - Digest digest1 = cache.addContents(remoteActionExecutionContext, "file1"); + Digest digest1 = cache.addContents(context, "file1"); Digest digest2 = cache.addException("file2", new IOException("download failed")); - Digest digest3 = cache.addContents(remoteActionExecutionContext, "file3"); + Digest digest3 = cache.addContents(context, "file3"); ActionResult result = ActionResult.newBuilder() @@ -708,11 +685,7 @@ public void onErrorWaitForRemainingDownloadsToComplete() throws Exception { BulkTransferException.class, () -> cache.download( - remoteActionExecutionContext, - result, - execRoot, - new FileOutErr(stdout, stderr), - outputFilesLocker)); + context, result, execRoot, new FileOutErr(stdout, stderr), outputFilesLocker)); assertThat(downloadException.getSuppressed()).hasLength(1); assertThat(cache.getNumSuccessfulDownloads()).isEqualTo(2); assertThat(cache.getNumFailedDownloads()).isEqualTo(1); @@ -728,7 +701,7 @@ public void downloadWithMultipleErrorsAddsThemAsSuppressed() throws Exception { Path stderr = fs.getPath("/execroot/stderr"); InMemoryRemoteCache cache = newRemoteCache(); - Digest digest1 = cache.addContents(remoteActionExecutionContext, "file1"); + Digest digest1 = cache.addContents(context, "file1"); Digest digest2 = cache.addException("file2", new IOException("file2 failed")); Digest digest3 = cache.addException("file3", new IOException("file3 failed")); @@ -744,11 +717,7 @@ public void downloadWithMultipleErrorsAddsThemAsSuppressed() throws Exception { BulkTransferException.class, () -> cache.download( - remoteActionExecutionContext, - result, - execRoot, - new FileOutErr(stdout, stderr), - outputFilesLocker)); + context, result, execRoot, new FileOutErr(stdout, stderr), outputFilesLocker)); assertThat(e.getSuppressed()).hasLength(2); assertThat(e.getSuppressed()[0]).isInstanceOf(IOException.class); @@ -763,7 +732,7 @@ public void downloadWithDuplicateIOErrorsDoesNotSuppress() throws Exception { Path stderr = fs.getPath("/execroot/stderr"); InMemoryRemoteCache cache = newRemoteCache(); - Digest digest1 = cache.addContents(remoteActionExecutionContext, "file1"); + Digest digest1 = cache.addContents(context, "file1"); IOException reusedException = new IOException("reused io exception"); Digest digest2 = cache.addException("file2", reusedException); Digest digest3 = cache.addException("file3", reusedException); @@ -780,11 +749,7 @@ public void downloadWithDuplicateIOErrorsDoesNotSuppress() throws Exception { BulkTransferException.class, () -> cache.download( - remoteActionExecutionContext, - result, - execRoot, - new FileOutErr(stdout, stderr), - outputFilesLocker)); + context, result, execRoot, new FileOutErr(stdout, stderr), outputFilesLocker)); for (Throwable t : downloadException.getSuppressed()) { assertThat(t).isInstanceOf(IOException.class); @@ -799,7 +764,7 @@ public void downloadWithDuplicateInterruptionsDoesNotSuppress() throws Exception Path stderr = fs.getPath("/execroot/stderr"); InMemoryRemoteCache cache = newRemoteCache(); - Digest digest1 = cache.addContents(remoteActionExecutionContext, "file1"); + Digest digest1 = cache.addContents(context, "file1"); InterruptedException reusedInterruption = new InterruptedException("reused interruption"); Digest digest2 = cache.addException("file2", reusedInterruption); Digest digest3 = cache.addException("file3", reusedInterruption); @@ -816,11 +781,7 @@ public void downloadWithDuplicateInterruptionsDoesNotSuppress() throws Exception InterruptedException.class, () -> cache.download( - remoteActionExecutionContext, - result, - execRoot, - new FileOutErr(stdout, stderr), - outputFilesLocker)); + context, result, execRoot, new FileOutErr(stdout, stderr), outputFilesLocker)); assertThat(e.getSuppressed()).isEmpty(); assertThat(Throwables.getRootCause(e)).hasMessageThat().isEqualTo("reused interruption"); @@ -839,8 +800,8 @@ public void testDownloadWithStdoutStderrOnSuccess() throws Exception { when(spyOutErr.childOutErr()).thenReturn(spyChildOutErr); InMemoryRemoteCache cache = newRemoteCache(); - Digest digestStdout = cache.addContents(remoteActionExecutionContext, "stdout"); - Digest digestStderr = cache.addContents(remoteActionExecutionContext, "stderr"); + Digest digestStdout = cache.addContents(context, "stdout"); + Digest digestStderr = cache.addContents(context, "stderr"); ActionResult result = ActionResult.newBuilder() @@ -849,7 +810,7 @@ public void testDownloadWithStdoutStderrOnSuccess() throws Exception { .setStderrDigest(digestStderr) .build(); - cache.download(remoteActionExecutionContext, result, execRoot, spyOutErr, outputFilesLocker); + cache.download(context, result, execRoot, spyOutErr, outputFilesLocker); verify(spyOutErr, Mockito.times(2)).childOutErr(); verify(spyChildOutErr).clearOut(); @@ -892,9 +853,7 @@ public void testDownloadWithStdoutStderrOnFailure() throws Exception { .build(); assertThrows( BulkTransferException.class, - () -> - cache.download( - remoteActionExecutionContext, result, execRoot, spyOutErr, outputFilesLocker)); + () -> cache.download(context, result, execRoot, spyOutErr, outputFilesLocker)); verify(spyOutErr, Mockito.times(2)).childOutErr(); verify(spyChildOutErr).clearOut(); verify(spyChildOutErr).clearErr(); @@ -917,8 +876,8 @@ public void testDownloadClashes() throws Exception { // arrange InMemoryRemoteCache remoteCache = newRemoteCache(); - Digest d1 = remoteCache.addContents(remoteActionExecutionContext, "content1"); - Digest d2 = remoteCache.addContents(remoteActionExecutionContext, "content2"); + Digest d1 = remoteCache.addContents(context, "content1"); + Digest d2 = remoteCache.addContents(context, "content2"); ActionResult r = ActionResult.newBuilder() .setExitCode(0) @@ -931,8 +890,7 @@ public void testDownloadClashes() throws Exception { // act - remoteCache.download( - remoteActionExecutionContext, r, execRoot, new FileOutErr(), outputFilesLocker); + remoteCache.download(context, r, execRoot, new FileOutErr(), outputFilesLocker); // assert @@ -949,8 +907,8 @@ public void testDownloadMinimalFiles() throws Exception { // arrange InMemoryRemoteCache remoteCache = newRemoteCache(); - Digest d1 = remoteCache.addContents(remoteActionExecutionContext, "content1"); - Digest d2 = remoteCache.addContents(remoteActionExecutionContext, "content2"); + Digest d1 = remoteCache.addContents(context, "content1"); + Digest d2 = remoteCache.addContents(context, "content2"); ActionResult r = ActionResult.newBuilder() .setExitCode(0) @@ -966,7 +924,7 @@ public void testDownloadMinimalFiles() throws Exception { // act InMemoryOutput inMemoryOutput = remoteCache.downloadMinimal( - remoteActionExecutionContext, + context, "action-id", r, ImmutableList.of(a1, a2), @@ -996,19 +954,19 @@ public void testDownloadMinimalDirectory() throws Exception { // Output Directory: // dir/file1 // dir/a/file2 - Digest d1 = remoteCache.addContents(remoteActionExecutionContext, "content1"); - Digest d2 = remoteCache.addContents(remoteActionExecutionContext, "content2"); + Digest d1 = remoteCache.addContents(context, "content1"); + Digest d2 = remoteCache.addContents(context, "content2"); FileNode file1 = FileNode.newBuilder().setName("file1").setDigest(d1).build(); FileNode file2 = FileNode.newBuilder().setName("file2").setDigest(d2).build(); Directory a = Directory.newBuilder().addFiles(file2).build(); - Digest da = remoteCache.addContents(remoteActionExecutionContext, a); + Digest da = remoteCache.addContents(context, a); Directory root = Directory.newBuilder() .addFiles(file1) .addDirectories(DirectoryNode.newBuilder().setName("a").setDigest(da)) .build(); Tree t = Tree.newBuilder().setRoot(root).addChildren(a).build(); - Digest dt = remoteCache.addContents(remoteActionExecutionContext, t); + Digest dt = remoteCache.addContents(context, t); ActionResult r = ActionResult.newBuilder() .setExitCode(0) @@ -1029,7 +987,7 @@ public void testDownloadMinimalDirectory() throws Exception { // act InMemoryOutput inMemoryOutput = remoteCache.downloadMinimal( - remoteActionExecutionContext, + context, "action-id", r, ImmutableList.of(dir), @@ -1069,12 +1027,12 @@ public void testDownloadMinimalDirectoryFails() throws Exception { // Output Directory: // dir/file1 // dir/a/file2 - Digest d1 = remoteCache.addContents(remoteActionExecutionContext, "content1"); - Digest d2 = remoteCache.addContents(remoteActionExecutionContext, "content2"); + Digest d1 = remoteCache.addContents(context, "content1"); + Digest d2 = remoteCache.addContents(context, "content2"); FileNode file1 = FileNode.newBuilder().setName("file1").setDigest(d1).build(); FileNode file2 = FileNode.newBuilder().setName("file2").setDigest(d2).build(); Directory a = Directory.newBuilder().addFiles(file2).build(); - Digest da = remoteCache.addContents(remoteActionExecutionContext, a); + Digest da = remoteCache.addContents(context, a); Directory root = Directory.newBuilder() .addFiles(file1) @@ -1105,7 +1063,7 @@ public void testDownloadMinimalDirectoryFails() throws Exception { BulkTransferException.class, () -> remoteCache.downloadMinimal( - remoteActionExecutionContext, + context, "action-id", r, ImmutableList.of(dir), @@ -1126,8 +1084,8 @@ public void testDownloadMinimalWithStdoutStderr() throws Exception { // arrange InMemoryRemoteCache remoteCache = newRemoteCache(); - Digest dOut = remoteCache.addContents(remoteActionExecutionContext, "stdout"); - Digest dErr = remoteCache.addContents(remoteActionExecutionContext, "stderr"); + Digest dOut = remoteCache.addContents(context, "stdout"); + Digest dErr = remoteCache.addContents(context, "stderr"); ActionResult r = ActionResult.newBuilder() .setExitCode(0) @@ -1140,7 +1098,7 @@ public void testDownloadMinimalWithStdoutStderr() throws Exception { // act InMemoryOutput inMemoryOutput = remoteCache.downloadMinimal( - remoteActionExecutionContext, + context, "action-id", r, ImmutableList.of(), @@ -1167,8 +1125,8 @@ public void testDownloadMinimalWithInMemoryOutput() throws Exception { // arrange InMemoryRemoteCache remoteCache = newRemoteCache(); - Digest d1 = remoteCache.addContents(remoteActionExecutionContext, "content1"); - Digest d2 = remoteCache.addContents(remoteActionExecutionContext, "content2"); + Digest d1 = remoteCache.addContents(context, "content1"); + Digest d2 = remoteCache.addContents(context, "content2"); ActionResult r = ActionResult.newBuilder() .setExitCode(0) @@ -1184,7 +1142,7 @@ public void testDownloadMinimalWithInMemoryOutput() throws Exception { // act InMemoryOutput inMemoryOutput = remoteCache.downloadMinimal( - remoteActionExecutionContext, + context, "action-id", r, ImmutableList.of(a1, a2), @@ -1215,7 +1173,7 @@ public void testDownloadMinimalWithMissingInMemoryOutput() throws Exception { // arrange InMemoryRemoteCache remoteCache = newRemoteCache(); - Digest d1 = remoteCache.addContents(remoteActionExecutionContext, "in-memory output"); + Digest d1 = remoteCache.addContents(context, "in-memory output"); ActionResult r = ActionResult.newBuilder().setExitCode(0).build(); Artifact a1 = ActionsTestUtil.createArtifact(artifactRoot, "file1"); MetadataInjector injector = mock(MetadataInjector.class); @@ -1225,7 +1183,7 @@ public void testDownloadMinimalWithMissingInMemoryOutput() throws Exception { // act InMemoryOutput inMemoryOutput = remoteCache.downloadMinimal( - remoteActionExecutionContext, + context, "action-id", r, ImmutableList.of(a1), @@ -1251,14 +1209,10 @@ public void testDownloadEmptyBlobAndFile() throws Exception { Digest emptyDigest = digestUtil.compute(new byte[0]); // act and assert - assertThat( - Utils.getFromFuture( - remoteCache.downloadBlob(remoteActionExecutionContext, emptyDigest))) - .isEmpty(); + assertThat(Utils.getFromFuture(remoteCache.downloadBlob(context, emptyDigest))).isEmpty(); try (OutputStream out = file.getOutputStream()) { - Utils.getFromFuture( - remoteCache.downloadFile(remoteActionExecutionContext, file, emptyDigest)); + Utils.getFromFuture(remoteCache.downloadFile(context, file, emptyDigest)); } assertThat(file.exists()).isTrue(); assertThat(file.getFileSize()).isEqualTo(0); @@ -1283,7 +1237,7 @@ public void testDownloadFileWithSymlinkTemplate() throws Exception { RemoteCache remoteCache = new InMemoryRemoteCache(cas, options, digestUtil); // act - Utils.getFromFuture(remoteCache.downloadFile(remoteActionExecutionContext, file, helloDigest)); + Utils.getFromFuture(remoteCache.downloadFile(context, file, helloDigest)); // assert assertThat(file.isSymbolicLink()).isTrue(); @@ -1325,11 +1279,7 @@ public void testDownloadDirectory() throws Exception { // act RemoteCache remoteCache = newRemoteCache(cas); remoteCache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - /* outputFilesLocker= */ () -> {}); + context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); // assert assertThat(digestUtil.compute(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); @@ -1354,11 +1304,7 @@ public void testDownloadEmptyDirectory() throws Exception { // act RemoteCache remoteCache = newRemoteCache(map); remoteCache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - /* outputFilesLocker= */ () -> {}); + context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); // assert assertThat(execRoot.getRelative("a/bar").isDirectory()).isTrue(); @@ -1403,11 +1349,7 @@ public void testDownloadNestedDirectory() throws Exception { // act RemoteCache remoteCache = newRemoteCache(map); remoteCache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - /* outputFilesLocker= */ () -> {}); + context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); // assert assertThat(digestUtil.compute(execRoot.getRelative("a/foo"))).isEqualTo(fooDigest); @@ -1458,11 +1400,7 @@ public void testDownloadDirectoryWithSameHash() throws Exception { // act RemoteCache remoteCache = newRemoteCache(map); remoteCache.download( - remoteActionExecutionContext, - result.build(), - execRoot, - null, - /* outputFilesLocker= */ () -> {}); + context, result.build(), execRoot, null, /* outputFilesLocker= */ () -> {}); // assert assertThat(digestUtil.compute(execRoot.getRelative("a/bar/foo/file"))).isEqualTo(fileDigest); @@ -1504,7 +1442,7 @@ public void testUploadDirectory() throws Exception { InMemoryRemoteCache remoteCache = newRemoteCache(); ActionResult result = remoteCache.upload( - remoteActionExecutionContext, + context, digestUtil.asActionKey(actionDigest), action, cmd, @@ -1520,7 +1458,7 @@ public void testUploadDirectory() throws Exception { ImmutableList toQuery = ImmutableList.of(fooDigest, quxDigest, barDigest, cmdDigest, actionDigest); - assertThat(remoteCache.findMissingDigests(toQuery)).isEmpty(); + assertThat(remoteCache.findMissingDigests(context, toQuery)).isEmpty(); } @Test @@ -1541,7 +1479,7 @@ public void testUploadEmptyDirectory() throws Exception { InMemoryRemoteCache remoteCache = newRemoteCache(); ActionResult result = remoteCache.upload( - remoteActionExecutionContext, + context, actionDigest, action, cmd, @@ -1553,7 +1491,7 @@ public void testUploadEmptyDirectory() throws Exception { ActionResult.Builder expectedResult = ActionResult.newBuilder(); expectedResult.addOutputDirectoriesBuilder().setPath("bar").setTreeDigest(barDigest); assertThat(result).isEqualTo(expectedResult.build()); - assertThat(remoteCache.findMissingDigests(ImmutableList.of(barDigest))).isEmpty(); + assertThat(remoteCache.findMissingDigests(context, ImmutableList.of(barDigest))).isEmpty(); } @Test @@ -1598,7 +1536,7 @@ public void testUploadNestedDirectory() throws Exception { InMemoryRemoteCache remoteCache = newRemoteCache(); ActionResult result = remoteCache.upload( - remoteActionExecutionContext, + context, actionDigest, action, cmd, @@ -1612,7 +1550,7 @@ public void testUploadNestedDirectory() throws Exception { assertThat(result).isEqualTo(expectedResult.build()); ImmutableList toQuery = ImmutableList.of(wobbleDigest, quxDigest, barDigest); - assertThat(remoteCache.findMissingDigests(toQuery)).isEmpty(); + assertThat(remoteCache.findMissingDigests(context, toQuery)).isEmpty(); } private static RemoteFileArtifactValue remoteFileMatchingDigest(Digest expectedDigest) { @@ -1680,9 +1618,10 @@ int getNumFailedDownloads() { return ((InMemoryCacheClient) cacheProtocol).getNumFailedDownloads(); } - ImmutableSet findMissingDigests(Iterable digests) + ImmutableSet findMissingDigests( + RemoteActionExecutionContext context, Iterable digests) throws IOException, InterruptedException { - return Utils.getFromFuture(cacheProtocol.findMissingDigests(digests)); + return Utils.getFromFuture(cacheProtocol.findMissingDigests(context, digests)); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java index f313ec91cf235d..8133d5c940571e 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java @@ -89,7 +89,7 @@ public void testZeroExitCodeFromCache() throws IOException, InterruptedException // Assert verify(remoteCache).downloadActionResult(any(), any(), anyBoolean()); // Don't fallback to execution - verify(remoteExecutor, never()).executeRemotely(any(), any()); + verify(remoteExecutor, never()).executeRemotely(any(), any(), any()); assertThat(executionResult.exitCode()).isEqualTo(0); } @@ -104,7 +104,7 @@ public void testNoneZeroExitCodeFromCache() throws IOException, InterruptedExcep .thenReturn(cachedResult); ExecuteResponse response = ExecuteResponse.newBuilder().setResult(cachedResult).build(); - when(remoteExecutor.executeRemotely(any(), any())).thenReturn(response); + when(remoteExecutor.executeRemotely(any(), any(), any())).thenReturn(response); // Act ExecutionResult executionResult = @@ -119,7 +119,7 @@ public void testNoneZeroExitCodeFromCache() throws IOException, InterruptedExcep // Assert verify(remoteCache).downloadActionResult(any(), any(), anyBoolean()); // Fallback to execution - verify(remoteExecutor).executeRemotely(any(), any()); + verify(remoteExecutor).executeRemotely(any(), any(), any()); assertThat(executionResult.exitCode()).isEqualTo(1); } @@ -141,7 +141,7 @@ public void testInlineStdoutStderr() throws IOException, InterruptedException { .thenReturn(cachedResult); ExecuteResponse response = ExecuteResponse.newBuilder().setResult(cachedResult).build(); - when(remoteExecutor.executeRemotely(any(), any())).thenReturn(response); + when(remoteExecutor.executeRemotely(any(), any(), any())).thenReturn(response); // Act ExecutionResult executionResult = diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index d4104756e21d6d..cff754e1686999 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -63,7 +63,6 @@ import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.options.RemoteOutputsMode; import com.google.devtools.build.lib.remote.util.DigestUtil; -import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; import com.google.devtools.build.lib.server.FailureDetails.Spawn.Code; @@ -98,6 +97,9 @@ public class RemoteSpawnCacheTest { private static final ArtifactExpander SIMPLE_ARTIFACT_EXPANDER = (artifact, output) -> output.add(artifact); + private static final String BUILD_REQUEST_ID = "build-req-id"; + private static final String COMMAND_ID = "command-id"; + private FileSystem fs; private DigestUtil digestUtil; private Path execRoot; @@ -201,8 +203,8 @@ private RemoteSpawnCache remoteSpawnCacheWithOptions(RemoteOptions options) { options, /* verboseFailures=*/ true, remoteCache, - "build-req-id", - "command-id", + BUILD_REQUEST_ID, + COMMAND_ID, reporter, digestUtil, /* filesToDownload= */ ImmutableSet.of()); @@ -244,9 +246,10 @@ public void cacheHit() throws Exception { new Answer() { @Override public ActionResult answer(InvocationOnMock invocation) { - RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - assertThat(meta.getCorrelatedInvocationsId()).isEqualTo("build-req-id"); - assertThat(meta.getToolInvocationId()).isEqualTo("command-id"); + RemoteActionExecutionContext context = invocation.getArgument(0); + RequestMetadata meta = context.getRequestMetadata(); + assertThat(meta.getCorrelatedInvocationsId()).isEqualTo(BUILD_REQUEST_ID); + assertThat(meta.getToolInvocationId()).isEqualTo(COMMAND_ID); return actionResult; } }); @@ -254,9 +257,10 @@ public ActionResult answer(InvocationOnMock invocation) { new Answer() { @Override public Void answer(InvocationOnMock invocation) { - RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - assertThat(meta.getCorrelatedInvocationsId()).isEqualTo("build-req-id"); - assertThat(meta.getToolInvocationId()).isEqualTo("command-id"); + RemoteActionExecutionContext context = invocation.getArgument(0); + RequestMetadata meta = context.getRequestMetadata(); + assertThat(meta.getCorrelatedInvocationsId()).isEqualTo(BUILD_REQUEST_ID); + assertThat(meta.getToolInvocationId()).isEqualTo(COMMAND_ID); return null; } }) @@ -302,9 +306,10 @@ public void cacheMiss() throws Exception { new Answer() { @Override public Void answer(InvocationOnMock invocation) { - RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - assertThat(meta.getCorrelatedInvocationsId()).isEqualTo("build-req-id"); - assertThat(meta.getToolInvocationId()).isEqualTo("command-id"); + RemoteActionExecutionContext context = invocation.getArgument(0); + RequestMetadata meta = context.getRequestMetadata(); + assertThat(meta.getCorrelatedInvocationsId()).isEqualTo(BUILD_REQUEST_ID); + assertThat(meta.getToolInvocationId()).isEqualTo(COMMAND_ID); return null; } }) @@ -564,9 +569,10 @@ public void printWarningIfDownloadFails() throws Exception { new Answer() { @Override public Void answer(InvocationOnMock invocation) { - RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - assertThat(meta.getCorrelatedInvocationsId()).isEqualTo("build-req-id"); - assertThat(meta.getToolInvocationId()).isEqualTo("command-id"); + RemoteActionExecutionContext context = invocation.getArgument(0); + RequestMetadata meta = context.getRequestMetadata(); + assertThat(meta.getCorrelatedInvocationsId()).isEqualTo(BUILD_REQUEST_ID); + assertThat(meta.getToolInvocationId()).isEqualTo(COMMAND_ID); return null; } }) @@ -613,9 +619,10 @@ public void orphanedCachedResultIgnored() throws Exception { new Answer() { @Override public ActionResult answer(InvocationOnMock invocation) { - RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - assertThat(meta.getCorrelatedInvocationsId()).isEqualTo("build-req-id"); - assertThat(meta.getToolInvocationId()).isEqualTo("command-id"); + RemoteActionExecutionContext context = invocation.getArgument(0); + RequestMetadata meta = context.getRequestMetadata(); + assertThat(meta.getCorrelatedInvocationsId()).isEqualTo(BUILD_REQUEST_ID); + assertThat(meta.getToolInvocationId()).isEqualTo(COMMAND_ID); return actionResult; } }); @@ -637,9 +644,10 @@ public ActionResult answer(InvocationOnMock invocation) { new Answer() { @Override public Void answer(InvocationOnMock invocation) { - RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - assertThat(meta.getCorrelatedInvocationsId()).isEqualTo("build-req-id"); - assertThat(meta.getToolInvocationId()).isEqualTo("command-id"); + RemoteActionExecutionContext context = invocation.getArgument(0); + RequestMetadata meta = context.getRequestMetadata(); + assertThat(meta.getCorrelatedInvocationsId()).isEqualTo(BUILD_REQUEST_ID); + assertThat(meta.getToolInvocationId()).isEqualTo(COMMAND_ID); return null; } }) diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index d811da1244b80d..39a82f20ffaa84 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -191,7 +191,10 @@ public void nonCachableSpawnsShouldNotBeCached_remote() throws Exception { ExecuteResponse.newBuilder() .setResult(ActionResult.newBuilder().setExitCode(0).build()) .build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn(succeeded); Spawn spawn = simpleSpawnWithExecutionInfo(NO_CACHE); @@ -200,7 +203,11 @@ public void nonCachableSpawnsShouldNotBeCached_remote() throws Exception { runner.exec(spawn, policy); ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(ExecuteRequest.class); - verify(executor).executeRemotely(requestCaptor.capture(), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + requestCaptor.capture(), + any(OperationObserver.class)); assertThat(requestCaptor.getValue().getSkipCacheLookup()).isTrue(); assertThat(requestCaptor.getValue().getResultsCachePolicy().getPriority()).isEqualTo(1); assertThat(requestCaptor.getValue().getExecutionPolicy().getPriority()).isEqualTo(2); @@ -236,7 +243,10 @@ public void nonCachableSpawnsShouldNotBeCached_localFallback() throws Exception RemoteSpawnRunner runner = newSpawnRunner(); // Throw an IOException to trigger the local fallback. - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(IOException.class); Spawn spawn = simpleSpawnWithExecutionInfo(NO_CACHE); @@ -261,7 +271,10 @@ public void cachableSpawnsShouldBeCached_localFallback() throws Exception { RemoteSpawnRunner runner = spy(newSpawnRunner()); // Throw an IOException to trigger the local fallback. - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(IOException.class); SpawnResult res = @@ -302,7 +315,10 @@ public void failedLocalActionShouldNotBeUploaded() throws Exception { RemoteSpawnRunner runner = spy(newSpawnRunner()); // Throw an IOException to trigger the local fallback. - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(IOException.class); Spawn spawn = newSimpleSpawn(); @@ -346,7 +362,10 @@ public void treatFailedCachedActionAsCacheMiss_local() throws Exception { RemoteSpawnRunner runner = spy(newSpawnRunner()); // Throw an IOException to trigger the local fallback. - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(IOException.class); Spawn spawn = newSimpleSpawn(); @@ -401,7 +420,10 @@ public void treatFailedCachedActionAsCacheMiss_remote() throws Exception { ExecuteResponse.newBuilder() .setResult(ActionResult.newBuilder().setExitCode(0).build()) .build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn(succeeded); Spawn spawn = newSimpleSpawn(); SpawnExecutionContext policy = getSpawnContext(spawn); @@ -409,7 +431,11 @@ public void treatFailedCachedActionAsCacheMiss_remote() throws Exception { runner.exec(spawn, policy); ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(ExecuteRequest.class); - verify(executor).executeRemotely(requestCaptor.capture(), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + requestCaptor.capture(), + any(OperationObserver.class)); assertThat(requestCaptor.getValue().getSkipCacheLookup()).isTrue(); } @@ -426,7 +452,10 @@ public void printWarningIfCacheIsDown() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(reporter); // Trigger local fallback - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(new IOException()); Spawn spawn = newSimpleSpawn(); @@ -471,7 +500,10 @@ public void noRemoteExecutorFallbackFails() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(); // Trigger local fallback - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(new IOException()); Spawn spawn = newSimpleSpawn(); @@ -501,7 +533,10 @@ public void remoteCacheErrorFallbackFails() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(); // Trigger local fallback - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(new IOException()); Spawn spawn = newSimpleSpawn(); @@ -533,7 +568,10 @@ public void testLocalFallbackFailureRemoteExecutorFailure() throws Exception { any(ActionKey.class), /* inlineOutErr= */ eq(false))) .thenReturn(null); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(new IOException()); Spawn spawn = newSimpleSpawn(); @@ -553,7 +591,10 @@ public void testHumanReadableServerLogsSavedForFailingAction() throws Exception RemoteSpawnRunner runner = newSpawnRunner(); Digest logDigest = digestUtil.computeAsUtf8("bla"); Path logPath = logDir.getRelative(simpleActionId).getRelative("logname"); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn( ExecuteResponse.newBuilder() .putServerLogs( @@ -572,7 +613,11 @@ public void testHumanReadableServerLogsSavedForFailingAction() throws Exception SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); verify(cache).downloadFile(any(RemoteActionExecutionContext.class), eq(logPath), eq(logDigest)); } @@ -589,7 +634,10 @@ public void testHumanReadableServerLogsSavedForFailingActionWithStatus() throws "logname", LogFile.newBuilder().setHumanReadable(true).setDigest(logDigest).build()) .setStatus(timeoutStatus) .build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(new IOException(new ExecutionStatusException(resp.getStatus(), resp))); SettableFuture completed = SettableFuture.create(); completed.set(null); @@ -602,7 +650,11 @@ public void testHumanReadableServerLogsSavedForFailingActionWithStatus() throws SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.TIMEOUT); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); verify(cache).downloadFile(any(RemoteActionExecutionContext.class), eq(logPath), eq(logDigest)); } @@ -612,7 +664,10 @@ public void testNonHumanReadableServerLogsNotSaved() throws Exception { Digest logDigest = digestUtil.computeAsUtf8("bla"); ActionResult result = ActionResult.newBuilder().setExitCode(31).build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn( ExecuteResponse.newBuilder() .putServerLogs("logname", LogFile.newBuilder().setDigest(logDigest).build()) @@ -624,7 +679,11 @@ public void testNonHumanReadableServerLogsNotSaved() throws Exception { SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); verify(cache) .download( any(RemoteActionExecutionContext.class), @@ -642,7 +701,10 @@ public void testServerLogsNotSavedForSuccessfulAction() throws Exception { Digest logDigest = digestUtil.computeAsUtf8("bla"); ActionResult result = ActionResult.newBuilder().setExitCode(0).build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn( ExecuteResponse.newBuilder() .putServerLogs( @@ -657,7 +719,11 @@ public void testServerLogsNotSavedForSuccessfulAction() throws Exception { SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.SUCCESS); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); verify(cache) .download( any(RemoteActionExecutionContext.class), @@ -693,7 +759,10 @@ public void cacheDownloadFailureTriggersRemoteExecution() throws Exception { any()); ActionResult execResult = ActionResult.newBuilder().setExitCode(31).build(); ExecuteResponse succeeded = ExecuteResponse.newBuilder().setResult(execResult).build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn(succeeded); doNothing() .when(cache) @@ -712,7 +781,11 @@ public void cacheDownloadFailureTriggersRemoteExecution() throws Exception { assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT); assertThat(res.exitCode()).isEqualTo(31); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); } @Test @@ -732,7 +805,10 @@ public void resultsDownloadFailureTriggersRemoteExecutionWithSkipCacheLookup() t ExecuteResponse cachedResponse = ExecuteResponse.newBuilder().setResult(cachedResult).setCachedResult(true).build(); ExecuteResponse executedResponse = ExecuteResponse.newBuilder().setResult(execResult).build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn(cachedResponse) .thenReturn(executedResponse); Exception downloadFailure = @@ -764,7 +840,10 @@ public void resultsDownloadFailureTriggersRemoteExecutionWithSkipCacheLookup() t ArgumentCaptor requestCaptor = ArgumentCaptor.forClass(ExecuteRequest.class); verify(executor, times(2)) - .executeRemotely(requestCaptor.capture(), any(OperationObserver.class)); + .executeRemotely( + any(RemoteActionExecutionContext.class), + requestCaptor.capture(), + any(OperationObserver.class)); List requests = requestCaptor.getAllValues(); // first request should have been executed without skip cache lookup assertThat(requests.get(0).getSkipCacheLookup()).isFalse(); @@ -794,7 +873,10 @@ public void testRemoteExecutionTimeout() throws Exception { .setCode(Code.DEADLINE_EXCEEDED.getNumber()) .build()) .build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(new IOException(new ExecutionStatusException(resp.getStatus(), resp))); Spawn spawn = newSimpleSpawn(); @@ -804,7 +886,11 @@ public void testRemoteExecutionTimeout() throws Exception { SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.TIMEOUT); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); verify(cache) .download( any(RemoteActionExecutionContext.class), @@ -837,7 +923,10 @@ public void testRemoteExecutionTimeoutDoesNotTriggerFallback() throws Exception .setCode(Code.DEADLINE_EXCEEDED.getNumber()) .build()) .build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(new IOException(new ExecutionStatusException(resp.getStatus(), resp))); Spawn spawn = newSimpleSpawn(); @@ -847,7 +936,11 @@ public void testRemoteExecutionTimeoutDoesNotTriggerFallback() throws Exception SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.TIMEOUT); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); verify(cache) .download( any(RemoteActionExecutionContext.class), @@ -874,7 +967,10 @@ public void testRemoteExecutionCommandFailureDoesNotTriggerFallback() throws Exc ExecuteResponse.newBuilder() .setResult(ActionResult.newBuilder().setExitCode(33).build()) .build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn(failed); Spawn spawn = newSimpleSpawn(); @@ -885,7 +981,11 @@ public void testRemoteExecutionCommandFailureDoesNotTriggerFallback() throws Exc assertThat(res.status()).isEqualTo(Status.NON_ZERO_EXIT); assertThat(res.exitCode()).isEqualTo(33); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); verify(cache, never()) .download( any(RemoteActionExecutionContext.class), @@ -910,7 +1010,10 @@ public void testExitCode_executorfailure() throws Exception { any(ActionKey.class), /* inlineOutErr= */ eq(false))) .thenReturn(null); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenThrow(new IOException("reasons")); Spawn spawn = newSimpleSpawn(); @@ -977,7 +1080,10 @@ private void testParamFilesAreMaterializedForFlag(String flag) throws Exception ExecuteResponse.newBuilder() .setResult(ActionResult.newBuilder().setExitCode(0).build()) .build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn(succeeded); ImmutableList args = ImmutableList.of("--foo", "--bar"); @@ -1056,7 +1162,10 @@ public void testDownloadMinimalOnCacheMiss() throws Exception { ActionResult succeededAction = ActionResult.newBuilder().setExitCode(0).build(); ExecuteResponse succeeded = ExecuteResponse.newBuilder().setResult(succeededAction).build(); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn(succeeded); RemoteSpawnRunner runner = newSpawnRunner(); @@ -1070,7 +1179,9 @@ public void testDownloadMinimalOnCacheMiss() throws Exception { assertThat(result.status()).isEqualTo(Status.SUCCESS); // assert - verify(executor).executeRemotely(any(), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), any(), any(OperationObserver.class)); verify(cache) .downloadMinimal( any(RemoteActionExecutionContext.class), @@ -1236,10 +1347,13 @@ public void shouldReportExecutingStatusWithoutMetadata() throws Exception { SpawnExecutionContext policy = mock(SpawnExecutionContext.class); when(policy.getTimeout()).thenReturn(Duration.ZERO); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenAnswer( invocationOnMock -> { - OperationObserver receiver = invocationOnMock.getArgument(1); + OperationObserver receiver = invocationOnMock.getArgument(2); verify(policy, never()).report(eq(ProgressStatus.EXECUTING), any(String.class)); receiver.onNext(Operation.getDefaultInstance()); return succeeded; @@ -1248,7 +1362,11 @@ public void shouldReportExecutingStatusWithoutMetadata() throws Exception { SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.SUCCESS); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); InOrder reportOrder = inOrder(policy); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.SCHEDULING), any(String.class)); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.EXECUTING), any(String.class)); @@ -1266,10 +1384,13 @@ public void shouldReportExecutingStatusAfterGotExecutingStageFromMetadata() thro SpawnExecutionContext policy = mock(SpawnExecutionContext.class); when(policy.getTimeout()).thenReturn(Duration.ZERO); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenAnswer( invocationOnMock -> { - OperationObserver receiver = invocationOnMock.getArgument(1); + OperationObserver receiver = invocationOnMock.getArgument(2); Operation queued = Operation.newBuilder() .setMetadata( @@ -1295,7 +1416,11 @@ public void shouldReportExecutingStatusAfterGotExecutingStageFromMetadata() thro SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.SUCCESS); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); InOrder reportOrder = inOrder(policy); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.SCHEDULING), any(String.class)); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.EXECUTING), any(String.class)); @@ -1313,10 +1438,13 @@ public void shouldIgnoreInvalidMetadata() throws Exception { SpawnExecutionContext policy = mock(SpawnExecutionContext.class); when(policy.getTimeout()).thenReturn(Duration.ZERO); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenAnswer( invocationOnMock -> { - OperationObserver receiver = invocationOnMock.getArgument(1); + OperationObserver receiver = invocationOnMock.getArgument(2); Operation operation = Operation.newBuilder() .setMetadata( @@ -1330,7 +1458,11 @@ public void shouldIgnoreInvalidMetadata() throws Exception { SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.SUCCESS); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); InOrder reportOrder = inOrder(policy); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.SCHEDULING), any(String.class)); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.EXECUTING), any(String.class)); @@ -1348,10 +1480,13 @@ public void shouldReportExecutingStatusIfNoExecutingStatusFromMetadata() throws SpawnExecutionContext policy = mock(SpawnExecutionContext.class); when(policy.getTimeout()).thenReturn(Duration.ZERO); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenAnswer( invocationOnMock -> { - OperationObserver receiver = invocationOnMock.getArgument(1); + OperationObserver receiver = invocationOnMock.getArgument(2); Operation completed = Operation.newBuilder() .setMetadata( @@ -1367,7 +1502,11 @@ public void shouldReportExecutingStatusIfNoExecutingStatusFromMetadata() throws SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.SUCCESS); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); InOrder reportOrder = inOrder(policy); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.SCHEDULING), any(String.class)); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.EXECUTING), any(String.class)); @@ -1385,13 +1524,20 @@ public void shouldReportExecutingStatusEvenNoOperationFromServer() throws Except SpawnExecutionContext policy = mock(SpawnExecutionContext.class); when(policy.getTimeout()).thenReturn(Duration.ZERO); - when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) + when(executor.executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class))) .thenReturn(succeeded); SpawnResult res = runner.exec(spawn, policy); assertThat(res.status()).isEqualTo(Status.SUCCESS); - verify(executor).executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class)); + verify(executor) + .executeRemotely( + any(RemoteActionExecutionContext.class), + any(ExecuteRequest.class), + any(OperationObserver.class)); InOrder reportOrder = inOrder(policy); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.SCHEDULING), any(String.class)); reportOrder.verify(policy, times(1)).report(eq(ProgressStatus.EXECUTING), any(String.class)); diff --git a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java index 2e13a3717e9d34..202bffd69d0e96 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/downloader/GrpcRemoteDownloaderTest.java @@ -38,9 +38,7 @@ import com.google.devtools.build.lib.remote.ReferenceCountedChannel; import com.google.devtools.build.lib.remote.RemoteRetrier; import com.google.devtools.build.lib.remote.RemoteRetrier.ExponentialBackoff; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestUtil; @@ -53,7 +51,6 @@ import com.google.devtools.common.options.Options; import com.google.protobuf.ByteString; import io.grpc.CallCredentials; -import io.grpc.Context; import io.grpc.Server; import io.grpc.inprocess.InProcessChannelBuilder; import io.grpc.inprocess.InProcessServerBuilder; @@ -84,8 +81,6 @@ public class GrpcRemoteDownloaderTest { private final String fakeServerName = "fake server for " + getClass(); private Server fakeServer; private RemoteActionExecutionContext context; - private Context withEmptyMetadata; - private Context prevContext; private ListeningScheduledExecutorService retryService; @Before @@ -102,18 +97,13 @@ public final void setUp() throws Exception { "none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance()).getDigest().getHash()); - context = new RemoteActionExecutionContextImpl(metadata, new NetworkTime()); - withEmptyMetadata = TracingMetadataUtils.contextWithMetadata(metadata); + context = RemoteActionExecutionContext.create(metadata); retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1)); - - prevContext = withEmptyMetadata.attach(); } @After public void tearDown() throws Exception { - withEmptyMetadata.detach(prevContext); - retryService.shutdownNow(); retryService.awaitTermination( com.google.devtools.build.lib.testutil.TestUtils.WAIT_TIMEOUT_SECONDS, TimeUnit.SECONDS); diff --git a/src/test/java/com/google/devtools/build/lib/remote/http/HttpCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/http/HttpCacheClientTest.java index 5c091c29f758a8..5ef61527910e81 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/http/HttpCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/http/HttpCacheClientTest.java @@ -32,9 +32,7 @@ import com.google.common.base.Charsets; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.vfs.DigestHashFunction; @@ -301,10 +299,9 @@ private HttpCacheClient createHttpBlobStore( @Before public void setUp() throws Exception { remoteActionExecutionContext = - new RemoteActionExecutionContextImpl( + RemoteActionExecutionContext.create( TracingMetadataUtils.buildMetadata( - "none", "none", Digest.getDefaultInstance().getHash()), - new NetworkTime()); + "none", "none", Digest.getDefaultInstance().getHash())); } @Test diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java index 9894f836eb8a43..b5ce3ee7b1946f 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java @@ -129,7 +129,8 @@ public ListenableFuture uploadBlob( } @Override - public ListenableFuture> findMissingDigests(Iterable digests) { + public ListenableFuture> findMissingDigests( + RemoteActionExecutionContext context, Iterable digests) { ImmutableSet.Builder missingBuilder = ImmutableSet.builder(); for (Digest digest : digests) { if (!cas.containsKey(digest)) { diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java index a077c8d19aaf8b..51972dd5024267 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java @@ -21,9 +21,7 @@ import build.bazel.remote.execution.v2.RequestMetadata; import build.bazel.remote.execution.v2.UpdateActionResultRequest; import com.google.common.flogger.GoogleLogger; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; @@ -46,8 +44,7 @@ public void getActionResult( GetActionResultRequest request, StreamObserver responseObserver) { try { RequestMetadata requestMetadata = TracingMetadataUtils.fromCurrentContext(); - RemoteActionExecutionContext context = - new RemoteActionExecutionContextImpl(requestMetadata, new NetworkTime()); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata); ActionKey actionKey = digestUtil.asActionKey(request.getActionDigest()); ActionResult result = @@ -71,8 +68,7 @@ public void updateActionResult( UpdateActionResultRequest request, StreamObserver responseObserver) { try { RequestMetadata requestMetadata = TracingMetadataUtils.fromCurrentContext(); - RemoteActionExecutionContext context = - new RemoteActionExecutionContextImpl(requestMetadata, new NetworkTime()); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata); ActionKey actionKey = digestUtil.asActionKey(request.getActionDigest()); cache.uploadActionResult(context, actionKey, request.getActionResult()); diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java index 2ed32e5dcbc531..b9e483bf24a86f 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ByteStreamServer.java @@ -26,9 +26,7 @@ import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.remote.Chunker; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -72,8 +70,7 @@ public ByteStreamServer(OnDiskBlobStoreCache cache, Path workPath, DigestUtil di @Override public void read(ReadRequest request, StreamObserver responseObserver) { RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - RemoteActionExecutionContext context = - new RemoteActionExecutionContextImpl(meta, new NetworkTime()); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(meta); Digest digest = parseDigestFromResourceName(request.getResourceName()); if (digest == null) { @@ -104,8 +101,7 @@ public void read(ReadRequest request, StreamObserver responseObser @Override public StreamObserver write(final StreamObserver responseObserver) { RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - RemoteActionExecutionContext context = - new RemoteActionExecutionContextImpl(meta, new NetworkTime()); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(meta); Path temp = workPath.getRelative("upload").getRelative(UUID.randomUUID().toString()); try { diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/CasServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/CasServer.java index e862939094cf82..d874b879af5ca6 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/CasServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/CasServer.java @@ -29,9 +29,7 @@ import build.bazel.remote.execution.v2.RequestMetadata; import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.protobuf.InvalidProtocolBufferException; import com.google.rpc.Code; @@ -69,8 +67,7 @@ public void findMissingBlobs( public void batchUpdateBlobs( BatchUpdateBlobsRequest request, StreamObserver responseObserver) { RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - RemoteActionExecutionContext context = - new RemoteActionExecutionContextImpl(meta, new NetworkTime()); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(meta); BatchUpdateBlobsResponse.Builder batchResponse = BatchUpdateBlobsResponse.newBuilder(); for (BatchUpdateBlobsRequest.Request r : request.getRequestsList()) { @@ -96,8 +93,7 @@ public void batchUpdateBlobs( @Override public void getTree(GetTreeRequest request, StreamObserver responseObserver) { RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); - RemoteActionExecutionContext context = - new RemoteActionExecutionContextImpl(meta, new NetworkTime()); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(meta); // Directories are returned in depth-first order. We store all previously-traversed digests so // identical subtrees having the same digest will only be traversed and returned once. diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java index e693ccc3051b97..c3eaa4f1e49c90 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java @@ -37,9 +37,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.remote.ExecutionStatusException; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; -import com.google.devtools.build.lib.remote.common.NetworkTime; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContextImpl; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; @@ -56,7 +54,6 @@ import com.google.protobuf.util.Durations; import com.google.rpc.Code; import com.google.rpc.Status; -import io.grpc.Context; import io.grpc.StatusException; import io.grpc.protobuf.StatusProto; import io.grpc.stub.StreamObserver; @@ -202,9 +199,12 @@ private void waitExecution( @Override public void execute(ExecuteRequest request, StreamObserver responseObserver) { + RequestMetadata metadata = TracingMetadataUtils.fromCurrentContext(); + RemoteActionExecutionContext context = RemoteActionExecutionContext.create(metadata); + final String opName = UUID.randomUUID().toString(); ListenableFuture future = - executorService.submit(Context.current().wrap(() -> execute(request, opName))); + executorService.submit(() -> execute(context, request, opName)); operationsCache.put(opName, future); // Send the first operation. responseObserver.onNext(Operation.newBuilder().setName(opName).build()); @@ -213,20 +213,19 @@ public void execute(ExecuteRequest request, StreamObserver responseOb } @SuppressWarnings("LogAndThrow") - private ActionResult execute(ExecuteRequest request, String id) + private ActionResult execute( + RemoteActionExecutionContext context, ExecuteRequest request, String id) throws IOException, InterruptedException, StatusException { Path tempRoot = workPath.getRelative("build-" + id); String workDetails = ""; try { tempRoot.createDirectory(); - RequestMetadata meta = TracingMetadataUtils.fromCurrentContext(); + RequestMetadata meta = context.getRequestMetadata(); workDetails = String.format( "build-request-id: %s command-id: %s action-id: %s", meta.getCorrelatedInvocationsId(), meta.getToolInvocationId(), meta.getActionId()); logger.atFine().log("Received work for: %s", workDetails); - RemoteActionExecutionContext context = - new RemoteActionExecutionContextImpl(meta, new NetworkTime()); ActionResult result = execute(context, request.getActionDigest(), tempRoot); logger.atFine().log("Completed %s", workDetails); return result;