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 93a5c1c7309488..a88ab82c20f8ed 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 @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.remote.RemoteRetrier.ProgressiveBackoff; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; 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.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.remote.util.DigestOutputStream; @@ -142,9 +143,11 @@ private ActionCacheBlockingStub acBlockingStub() { .withDeadlineAfter(options.remoteTimeout.getSeconds(), TimeUnit.SECONDS); } - private ActionCacheFutureStub acFutureStub() { + private ActionCacheFutureStub acFutureStub(RemoteActionExecutionContext context) { return ActionCacheGrpc.newFutureStub(channel) - .withInterceptors(TracingMetadataUtils.attachMetadataFromContextInterceptor()) + .withInterceptors( + TracingMetadataUtils.attachMetadataInterceptor(context.getRequestMetadata()), + new NetworkTimeInterceptor(context::getNetworkTime)) .withCallCredentials(callCredentialsProvider.getCallCredentials()) .withDeadlineAfter(options.remoteTimeout.getSeconds(), TimeUnit.SECONDS); } @@ -240,7 +243,7 @@ private ListenableFuture handleStatus(ListenableFuture downloadActionResult( - ActionKey actionKey, boolean inlineOutErr) { + RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { GetActionResultRequest request = GetActionResultRequest.newBuilder() .setInstanceName(options.remoteInstanceName) @@ -248,11 +251,10 @@ public ListenableFuture downloadActionResult( .setInlineStderr(inlineOutErr) .setInlineStdout(inlineOutErr) .build(); - Context ctx = Context.current(); return Utils.refreshIfUnauthenticatedAsync( () -> retrier.executeAsync( - () -> ctx.call(() -> handleStatus(acFutureStub().getActionResult(request)))), + () -> handleStatus(acFutureStub(context).getActionResult(request))), callCredentialsProvider); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/NetworkTime.java b/src/main/java/com/google/devtools/build/lib/remote/NetworkTimeInterceptor.java similarity index 60% rename from src/main/java/com/google/devtools/build/lib/remote/util/NetworkTime.java rename to src/main/java/com/google/devtools/build/lib/remote/NetworkTimeInterceptor.java index 52e11ed55e636a..ac953aa48f9d42 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/NetworkTime.java +++ b/src/main/java/com/google/devtools/build/lib/remote/NetworkTimeInterceptor.java @@ -1,4 +1,4 @@ -// Copyright 2019 The Bazel Authors. All rights reserved. +// Copyright 2020 The Bazel Authors. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -11,12 +11,10 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.remote.util; +package com.google.devtools.build.lib.remote; import build.bazel.remote.execution.v2.ExecutionGrpc; -import com.google.common.base.MoreObjects; -import com.google.common.base.Stopwatch; -import com.google.devtools.build.lib.concurrent.ThreadSafety; +import com.google.devtools.build.lib.remote.common.NetworkTime; import io.grpc.CallOptions; import io.grpc.Channel; import io.grpc.ClientCall; @@ -27,59 +25,31 @@ import io.grpc.Metadata; import io.grpc.MethodDescriptor; import io.grpc.Status; -import java.time.Duration; +import java.util.function.Supplier; -/** Reentrant wall clock stopwatch and grpc interceptor for network waits. */ -@ThreadSafety.ThreadSafe -public class NetworkTime { +/** 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; - private final Stopwatch wallTime = Stopwatch.createUnstarted(); - private int outstanding = 0; - - private synchronized void start() { - if (!wallTime.isRunning()) { - wallTime.start(); - } - outstanding++; - } - - private synchronized void stop() { - if (--outstanding == 0) { - wallTime.stop(); - } - } - - public Duration getDuration() { - return wallTime.elapsed(); + public NetworkTimeInterceptor(Supplier networkTimeSupplier) { + this.networkTimeSupplier = networkTimeSupplier; } @Override - public String toString() { - return MoreObjects.toStringHelper(this) - .add("outstanding", outstanding) - .add("wallTime", wallTime) - .add("wallTime.isRunning", wallTime.isRunning()) - .toString(); - } - - /** The ClientInterceptor used to track network time. */ - public static class Interceptor implements ClientInterceptor { - @Override - public ClientCall interceptCall( - MethodDescriptor method, CallOptions callOptions, Channel next) { - ClientCall call = next.newCall(method, callOptions); - // prevent accounting for execution wait time - if (method != ExecutionGrpc.getExecuteMethod() - && method != ExecutionGrpc.getWaitExecutionMethod()) { - NetworkTime networkTime = CONTEXT_KEY.get(); - if (networkTime != null) { - call = new NetworkTimeCall<>(call, networkTime); - } + public ClientCall interceptCall( + MethodDescriptor method, CallOptions callOptions, Channel next) { + ClientCall call = next.newCall(method, callOptions); + // prevent accounting for execution wait time + if (method != ExecutionGrpc.getExecuteMethod() + && method != ExecutionGrpc.getWaitExecutionMethod()) { + NetworkTime networkTime = networkTimeSupplier.get(); + if (networkTime != null) { + call = new NetworkTimeCall<>(call, networkTime); } - return call; } + return call; } private static class NetworkTimeCall 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 ec6e6961b6832f..59138b5a3c1c79 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 @@ -55,6 +55,7 @@ import com.google.devtools.build.lib.remote.RemoteCache.ActionResultMetadata.DirectoryMetadata; import com.google.devtools.build.lib.remote.RemoteCache.ActionResultMetadata.FileMetadata; import com.google.devtools.build.lib.remote.RemoteCache.ActionResultMetadata.SymlinkMetadata; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteActionFileArtifactValue; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; @@ -115,9 +116,10 @@ public RemoteCache( this.digestUtil = digestUtil; } - public ActionResult downloadActionResult(ActionKey actionKey, boolean inlineOutErr) + public ActionResult downloadActionResult( + RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) throws IOException, InterruptedException { - return getFromFuture(cacheProtocol.downloadActionResult(actionKey, inlineOutErr)); + return getFromFuture(cacheProtocol.downloadActionResult(context, actionKey, inlineOutErr)); } /** diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java index 360b2a5e41814f..ecb175e3992a8b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java @@ -68,7 +68,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.NetworkTime; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.remote.util.Utils; import com.google.devtools.build.lib.runtime.BlazeModule; @@ -330,7 +329,6 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { if (loggingInterceptor != null) { interceptors.add(loggingInterceptor); } - interceptors.add(new NetworkTime.Interceptor()); try { execChannel = RemoteCacheClientFactory.createGrpcChannelPool( @@ -357,7 +355,6 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { if (loggingInterceptor != null) { interceptors.add(loggingInterceptor); } - interceptors.add(new NetworkTime.Interceptor()); try { cacheChannel = RemoteCacheClientFactory.createGrpcChannelPool( @@ -551,7 +548,9 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException { remoteCache, remoteExecutor, digestUtil, - repoContext, + buildRequestId, + invocationId, + "repository_rule", remoteOptions.remoteInstanceName, remoteOptions.remoteAcceptCached)); } else { 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 852bce5ea25c8d..e66897b599ad48 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 @@ -28,11 +28,15 @@ 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; 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.runtime.RepositoryRemoteExecutor; import com.google.devtools.build.lib.vfs.Path; @@ -49,7 +53,9 @@ public class RemoteRepositoryRemoteExecutor implements RepositoryRemoteExecutor private final RemoteExecutionCache remoteCache; private final RemoteExecutionClient remoteExecutor; private final DigestUtil digestUtil; - private final Context requestCtx; + private final String buildRequestId; + private final String commandId; + private final String actionId; private final String remoteInstanceName; private final boolean acceptCached; @@ -58,13 +64,17 @@ public RemoteRepositoryRemoteExecutor( RemoteExecutionCache remoteCache, RemoteExecutionClient remoteExecutor, DigestUtil digestUtil, - Context requestCtx, + String buildRequestId, + String commandId, + String actionId, String remoteInstanceName, boolean acceptCached) { this.remoteCache = remoteCache; this.remoteExecutor = remoteExecutor; this.digestUtil = digestUtil; - this.requestCtx = requestCtx; + this.buildRequestId = buildRequestId; + this.commandId = commandId; + this.actionId = actionId; this.remoteInstanceName = remoteInstanceName; this.acceptCached = acceptCached; } @@ -100,6 +110,8 @@ public ExecutionResult execute( String workingDirectory, Duration timeout) throws IOException, InterruptedException { + Context requestCtx = + TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionId); Context prev = requestCtx.attach(); try { Platform platform = PlatformUtils.buildPlatformProto(executionProperties); @@ -117,10 +129,16 @@ public ExecutionResult execute( commandHash, merkleTree.getRootDigest(), timeout, acceptCached); Digest actionDigest = digestUtil.compute(action); ActionKey actionKey = new ActionKey(actionDigest); + RemoteActionExecutionContext remoteActionExecutionContext = + new RemoteActionExecutionContextImpl( + TracingMetadataUtils.buildMetadata(buildRequestId, commandId, actionId), + new NetworkTime()); ActionResult actionResult; try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { - actionResult = remoteCache.downloadActionResult(actionKey, /* inlineOutErr= */ true); + actionResult = + remoteCache.downloadActionResult( + remoteActionExecutionContext, actionKey, /* inlineOutErr= */ true); } if (actionResult == null || actionResult.getExitCode() != 0) { try (SilentCloseable c = diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorFactory.java index 3cf49c617f7fea..70ee4d903ca690 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorFactory.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorFactory.java @@ -17,7 +17,6 @@ import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutorFactory; -import io.grpc.Context; /** Factory for {@link RemoteRepositoryRemoteExecutor}. */ class RemoteRepositoryRemoteExecutorFactory implements RepositoryRemoteExecutorFactory { @@ -25,7 +24,9 @@ class RemoteRepositoryRemoteExecutorFactory implements RepositoryRemoteExecutorF private final RemoteExecutionCache remoteExecutionCache; private final RemoteExecutionClient remoteExecutor; private final DigestUtil digestUtil; - private final Context requestCtx; + private final String buildRequestId; + private final String commandId; + private final String actionId; private final String remoteInstanceName; private final boolean acceptCached; @@ -34,13 +35,17 @@ class RemoteRepositoryRemoteExecutorFactory implements RepositoryRemoteExecutorF RemoteExecutionCache remoteExecutionCache, RemoteExecutionClient remoteExecutor, DigestUtil digestUtil, - Context requestCtx, + String buildRequestId, + String commandId, + String actionId, String remoteInstanceName, boolean acceptCached) { this.remoteExecutionCache = remoteExecutionCache; this.remoteExecutor = remoteExecutor; this.digestUtil = digestUtil; - this.requestCtx = requestCtx; + this.buildRequestId = buildRequestId; + this.commandId = commandId; + this.actionId = actionId; this.remoteInstanceName = remoteInstanceName; this.acceptCached = acceptCached; } @@ -51,7 +56,9 @@ public RepositoryRemoteExecutor create() { remoteExecutionCache, remoteExecutor, digestUtil, - requestCtx, + buildRequestId, + commandId, + actionId, remoteInstanceName, acceptCached); } 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 51a1d659035784..5bdda3e57d8008 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 @@ -48,12 +48,14 @@ 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; 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.NetworkTime; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.remote.util.Utils; import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; @@ -151,9 +153,15 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) digestUtil.compute(command), merkleTreeRoot, context.getTimeout(), true); // Look up action cache, and reuse the action output if it is found. ActionKey actionKey = digestUtil.computeActionKey(action); + + RemoteActionExecutionContext remoteActionExecutionContext = + new RemoteActionExecutionContextImpl( + TracingMetadataUtils.buildMetadata( + buildRequestId, commandId, actionKey.getDigest().getHash()), + networkTime); Context withMetadata = TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey) - .withValue(NetworkTime.CONTEXT_KEY, networkTime); + .withValue(NetworkTimeInterceptor.CONTEXT_KEY, networkTime); Profiler prof = Profiler.instance(); if (options.remoteAcceptCached @@ -165,7 +173,9 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) try { ActionResult result; try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { - result = remoteCache.downloadActionResult(actionKey, /* inlineOutErr= */ false); + result = + remoteCache.downloadActionResult( + remoteActionExecutionContext, actionKey, /* inlineOutErr= */ false); } // In case the remote cache returned a failed action (exit code != 0) we treat it as a // cache miss 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 f1925062de5a74..c72d9c05cf7c33 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 @@ -66,14 +66,16 @@ 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; 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.NetworkTime; import com.google.devtools.build.lib.remote.util.TracingMetadataUtils; import com.google.devtools.build.lib.remote.util.Utils; import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; @@ -244,9 +246,15 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) NetworkTime networkTime = new NetworkTime(); // Look up action cache, and reuse the action output if it is found. ActionKey actionKey = digestUtil.computeActionKey(action); + + RemoteActionExecutionContext remoteActionExecutionContext = + new RemoteActionExecutionContextImpl( + TracingMetadataUtils.buildMetadata( + buildRequestId, commandId, actionKey.getDigest().getHash()), + networkTime); Context withMetadata = TracingMetadataUtils.contextWithMetadata(buildRequestId, commandId, actionKey) - .withValue(NetworkTime.CONTEXT_KEY, networkTime); + .withValue(NetworkTimeInterceptor.CONTEXT_KEY, networkTime); Context previous = withMetadata.attach(); Profiler prof = Profiler.instance(); try { @@ -256,7 +264,8 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { cachedResult = acceptCachedResult - ? remoteCache.downloadActionResult(actionKey, /* inlineOutErr= */ false) + ? remoteCache.downloadActionResult( + remoteActionExecutionContext, actionKey, /* inlineOutErr= */ false) : null; } if (cachedResult != null) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD index 024321d2695b23..515eb63205d0a2 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD @@ -15,6 +15,7 @@ java_library( srcs = glob(["*.java"]), deps = [ "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/vfs", "//third_party:guava", "//third_party/protobuf:protobuf_java", diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/NetworkTime.java b/src/main/java/com/google/devtools/build/lib/remote/common/NetworkTime.java new file mode 100644 index 00000000000000..dab14c0879befe --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/common/NetworkTime.java @@ -0,0 +1,54 @@ +// Copyright 2019 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.remote.common; + +import com.google.common.base.MoreObjects; +import com.google.common.base.Stopwatch; +import com.google.devtools.build.lib.concurrent.ThreadSafety; +import java.time.Duration; + +/** Reentrant wall clock stopwatch and grpc interceptor for network waits. */ +@ThreadSafety.ThreadSafe +public class NetworkTime { + + private final Stopwatch wallTime = Stopwatch.createUnstarted(); + private int outstanding = 0; + + public synchronized void start() { + if (!wallTime.isRunning()) { + wallTime.start(); + } + outstanding++; + } + + public synchronized void stop() { + if (--outstanding == 0) { + wallTime.stop(); + } + } + + public Duration getDuration() { + return wallTime.elapsed(); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("outstanding", outstanding) + .add("wallTime", wallTime) + .add("wallTime.isRunning", wallTime.isRunning()) + .toString(); + } + +} 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 new file mode 100644 index 00000000000000..6d61cd0dbdb5e1 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContext.java @@ -0,0 +1,29 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.remote.common; + +import build.bazel.remote.execution.v2.RequestMetadata; + +/** A context that provide remote execution related information for executing an action remotely. */ +public interface RemoteActionExecutionContext { + + /** Get the {@link RequestMetadata} for the action being executed. */ + RequestMetadata getRequestMetadata(); + + /** + * Get the {@link NetworkTime} instance used to measure the network time during the action + * execution. + */ + NetworkTime getNetworkTime(); +} 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/RemoteActionExecutionContextImpl.java new file mode 100644 index 00000000000000..f4894e31740778 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteActionExecutionContextImpl.java @@ -0,0 +1,39 @@ +// Copyright 2020 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.remote.common; + +import build.bazel.remote.execution.v2.RequestMetadata; + +/** A {@link RemoteActionExecutionContext} implementation */ +public class RemoteActionExecutionContextImpl implements RemoteActionExecutionContext { + + private final RequestMetadata requestMetadata; + private final NetworkTime networkTime; + + public RemoteActionExecutionContextImpl( + RequestMetadata requestMetadata, NetworkTime networkTime) { + this.requestMetadata = requestMetadata; + this.networkTime = networkTime; + } + + @Override + public RequestMetadata getRequestMetadata() { + return requestMetadata; + } + + @Override + public NetworkTime getNetworkTime() { + return networkTime; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java index ae6a15113f72d9..11fa078357ca8b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java @@ -72,7 +72,8 @@ public int hashCode() { * @return A Future representing pending download of an action result. If an action result for * {@code actionKey} cannot be found the result of the Future is {@code null}. */ - ListenableFuture downloadActionResult(ActionKey actionKey, boolean inlineOutErr); + ListenableFuture downloadActionResult( + RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr); /** * Uploads an action result for the {@code actionKey}. 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 4c3a65cdc86800..d188191652d12d 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 @@ -20,6 +20,7 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.vfs.Path; @@ -164,14 +165,14 @@ public ListenableFuture downloadBlob(Digest digest, OutputStream out) { @Override public ListenableFuture downloadActionResult( - ActionKey actionKey, boolean inlineOutErr) { + RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { if (diskCache.containsActionResult(actionKey)) { - return diskCache.downloadActionResult(actionKey, inlineOutErr); + return diskCache.downloadActionResult(context, actionKey, inlineOutErr); } if (!options.incompatibleRemoteResultsIgnoreDisk || options.remoteAcceptCached) { return Futures.transformAsync( - remoteCache.downloadActionResult(actionKey, inlineOutErr), + remoteCache.downloadActionResult(context, actionKey, inlineOutErr), (actionResult) -> { if (actionResult == null) { return Futures.immediateFuture(null); 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 62434d9cd16bb3..3722cc56716523 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 @@ -21,6 +21,7 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.util.DigestOutputStream; import com.google.devtools.build.lib.remote.util.DigestUtil; @@ -101,7 +102,7 @@ public ListenableFuture downloadBlob(Digest digest, OutputStream out) { @Override public ListenableFuture downloadActionResult( - ActionKey actionKey, boolean inlineOutErr) { + RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { return Utils.downloadAsActionResult( actionKey, (digest, out) -> download(digest, out, /* isActionCache= */ true)); } 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 bdfca850fd92f2..c35d9f074a88cf 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 @@ -26,6 +26,7 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.SettableFuture; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.remote.util.DigestOutputStream; import com.google.devtools.build.lib.remote.util.DigestUtil; @@ -572,7 +573,7 @@ private void getAfterCredentialRefresh(DownloadCommand cmd, SettableFuture @Override public ListenableFuture downloadActionResult( - ActionKey actionKey, boolean inlineOutErr) { + RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { return Utils.downloadAsActionResult( actionKey, (digest, out) -> get(digest, out, /* casDownload= */ false)); } 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 79a0cbb8afa3ee..cb880ce9525ac0 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 @@ -58,6 +58,22 @@ public static Context contextWithMetadata( return contextWithMetadata(buildRequestId, commandId, actionKey.getDigest().getHash()); } + public static RequestMetadata buildMetadata( + String buildRequestId, String commandId, String actionId) { + Preconditions.checkNotNull(buildRequestId); + Preconditions.checkNotNull(commandId); + Preconditions.checkNotNull(actionId); + return RequestMetadata.newBuilder() + .setCorrelatedInvocationsId(buildRequestId) + .setToolInvocationId(commandId) + .setActionId(actionId) + .setToolDetails( + ToolDetails.newBuilder() + .setToolName("bazel") + .setToolVersion(BlazeVersionInfo.instance().getVersion())) + .build(); + } + /** * Returns a new gRPC context derived from the current context, with {@link RequestMetadata} * accessible by the {@link fromCurrentContext()} method. @@ -67,19 +83,7 @@ public static Context contextWithMetadata( */ public static Context contextWithMetadata( String buildRequestId, String commandId, String actionId) { - Preconditions.checkNotNull(buildRequestId); - Preconditions.checkNotNull(commandId); - Preconditions.checkNotNull(actionId); - RequestMetadata metadata = - RequestMetadata.newBuilder() - .setCorrelatedInvocationsId(buildRequestId) - .setToolInvocationId(commandId) - .setActionId(actionId) - .setToolDetails( - ToolDetails.newBuilder() - .setToolName("bazel") - .setToolVersion(BlazeVersionInfo.instance().getVersion())) - .build(); + RequestMetadata metadata = buildMetadata(buildRequestId, commandId, actionId); return contextWithMetadata(metadata); } @@ -107,8 +111,13 @@ public static RequestMetadata fromCurrentContext() { * @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(); - headers.put(METADATA_KEY, fromCurrentContext()); + headers.put(METADATA_KEY, requestMetadata); return headers; } @@ -124,6 +133,10 @@ public static ClientInterceptor attachMetadataFromContextInterceptor() { return MetadataUtils.newAttachHeadersInterceptor(headersFromCurrentContext()); } + public static ClientInterceptor attachMetadataInterceptor(RequestMetadata requestMetadata) { + return MetadataUtils.newAttachHeadersInterceptor(headersFromRequestMetadata(requestMetadata)); + } + private static Metadata newMetadataForHeaders(List> headers) { Metadata metadata = new Metadata(); headers.forEach( @@ -169,5 +182,4 @@ public Listener interceptCall( return Contexts.interceptCall(ctx, call, headers, next); } } - } 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 aadb0eeb4eed43..6d8af4281a60f5 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 @@ -61,6 +61,9 @@ 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; @@ -129,6 +132,7 @@ public class GrpcCacheClientTest { private final String fakeServerName = "fake server for " + getClass(); private Server fakeServer; private Context withEmptyMetadata; + private RemoteActionExecutionContext remoteActionExecutionContext; private Context prevContext; private ListeningScheduledExecutorService retryService; @@ -152,9 +156,13 @@ 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( - "none", "none", DIGEST_UTIL.asActionKey(Digest.getDefaultInstance())); + TracingMetadataUtils.contextWithMetadata(remoteActionExecutionContext.getRequestMetadata()); retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1)); prevContext = withEmptyMetadata.attach(); @@ -761,7 +769,9 @@ public void getActionResult( GrpcCacheClient client = newClient(remoteOptions); RemoteCache remoteCache = new RemoteCache(client, remoteOptions, DIGEST_UTIL); remoteCache.downloadActionResult( - DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")), /* inlineOutErr= */ false); + remoteActionExecutionContext, + DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")), + /* inlineOutErr= */ false); } @Test @@ -1052,7 +1062,10 @@ public void getActionResult( (numErrors-- <= 0 ? Status.NOT_FOUND : Status.UNAVAILABLE).asRuntimeException()); } }); - assertThat(getFromFuture(client.downloadActionResult(actionKey, /* inlineOutErr= */ false))) + assertThat( + getFromFuture( + client.downloadActionResult( + remoteActionExecutionContext, actionKey, /* inlineOutErr= */ false))) .isNull(); } 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 80d36bca934639..96c87a6a163581 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 @@ -31,7 +31,6 @@ import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor.ExecutionResult; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.protobuf.ByteString; -import io.grpc.Context; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.time.Duration; @@ -62,7 +61,9 @@ public void setup() { remoteCache, remoteExecutor, DIGEST_UTIL, - Context.current(), + "none", + "none", + "repo", /* remoteInstanceName= */ "foo", /* acceptCached= */ true); } @@ -73,7 +74,7 @@ public void testZeroExitCodeFromCache() throws IOException, InterruptedException // Arrange ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); - when(remoteCache.downloadActionResult(any(), /* inlineOutErr= */ eq(true))) + when(remoteCache.downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true))) .thenReturn(cachedResult); // Act @@ -87,7 +88,7 @@ public void testZeroExitCodeFromCache() throws IOException, InterruptedException /* timeout= */ Duration.ZERO); // Assert - verify(remoteCache).downloadActionResult(any(), anyBoolean()); + verify(remoteCache).downloadActionResult(any(), any(), anyBoolean()); // Don't fallback to execution verify(remoteExecutor, never()).executeRemotely(any(), any()); @@ -100,7 +101,7 @@ public void testNoneZeroExitCodeFromCache() throws IOException, InterruptedExcep // Arrange ActionResult cachedResult = ActionResult.newBuilder().setExitCode(1).build(); - when(remoteCache.downloadActionResult(any(), /* inlineOutErr= */ eq(true))) + when(remoteCache.downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true))) .thenReturn(cachedResult); ExecuteResponse response = ExecuteResponse.newBuilder().setResult(cachedResult).build(); @@ -117,7 +118,7 @@ public void testNoneZeroExitCodeFromCache() throws IOException, InterruptedExcep /* timeout= */ Duration.ZERO); // Assert - verify(remoteCache).downloadActionResult(any(), anyBoolean()); + verify(remoteCache).downloadActionResult(any(), any(), anyBoolean()); // Fallback to execution verify(remoteExecutor).executeRemotely(any(), any()); @@ -137,7 +138,7 @@ public void testInlineStdoutStderr() throws IOException, InterruptedException { .setStdoutRaw(ByteString.copyFrom(stdout)) .setStderrRaw(ByteString.copyFrom(stderr)) .build(); - when(remoteCache.downloadActionResult(any(), /* inlineOutErr= */ eq(true))) + when(remoteCache.downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true))) .thenReturn(cachedResult); ExecuteResponse response = ExecuteResponse.newBuilder().setResult(cachedResult).build(); @@ -154,7 +155,7 @@ public void testInlineStdoutStderr() throws IOException, InterruptedException { /* timeout= */ Duration.ZERO); // Assert - verify(remoteCache).downloadActionResult(any(), /* inlineOutErr= */ eq(true)); + verify(remoteCache).downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true)); assertThat(executionResult.exitCode()).isEqualTo(0); assertThat(executionResult.stdout()).isEqualTo(stdout); 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 c385e45738fd24..fc168ac9b99667 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 @@ -58,6 +58,7 @@ import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.util.FakeOwner; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; 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.options.RemoteOutputsMode; @@ -235,7 +236,10 @@ public final void setUp() throws Exception { @Test public void cacheHit() throws Exception { ActionResult actionResult = ActionResult.getDefaultInstance(); - when(remoteCache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(remoteCache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenAnswer( new Answer() { @Override @@ -339,7 +343,10 @@ public void noCacheSpawns() throws Exception { simpleSpawnWithExecutionInfo(ImmutableMap.of(requirement, "")); CacheHandle entry = remoteSpawnCache.lookup(uncacheableSpawn, simplePolicy); verify(remoteCache, never()) - .downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false)); + .downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false)); assertThat(entry.hasResult()).isFalse(); SpawnResult result = new SpawnResult.Builder() @@ -372,7 +379,10 @@ public void noRemoteCacheSpawns_remoteCache() throws Exception { SimpleSpawn uncacheableSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of(requirement, "")); CacheHandle entry = remoteSpawnCache.lookup(uncacheableSpawn, simplePolicy); verify(remoteCache, never()) - .downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false)); + .downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false)); assertThat(entry.hasResult()).isFalse(); SpawnResult result = new SpawnResult.Builder() @@ -406,7 +416,10 @@ public void noRemoteCacheSpawns_combinedCache() throws Exception { SimpleSpawn uncacheableSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of(requirement, "")); CacheHandle entry = remoteSpawnCache.lookup(uncacheableSpawn, simplePolicy); verify(remoteCache, never()) - .downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false)); + .downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false)); assertThat(entry.hasResult()).isFalse(); SpawnResult result = new SpawnResult.Builder() @@ -429,7 +442,11 @@ public void noRemoteCacheStillUsesLocalCache() throws Exception { SimpleSpawn cacheableSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_CACHE, "")); cache.lookup(cacheableSpawn, simplePolicy); - verify(remoteCache).downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false)); + verify(remoteCache) + .downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false)); } @Test @@ -437,14 +454,22 @@ public void noRemoteExecStillUsesCache() throws Exception { SimpleSpawn cacheableSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of(ExecutionRequirements.NO_REMOTE_EXEC, "")); cache.lookup(cacheableSpawn, simplePolicy); - verify(remoteCache).downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false)); + verify(remoteCache) + .downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false)); } @Test public void failedActionsAreNotUploaded() throws Exception { // Only successful action results are uploaded to the remote cache. CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy); - verify(remoteCache).downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false)); + verify(remoteCache) + .downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false)); assertThat(entry.hasResult()).isFalse(); SpawnResult result = new SpawnResult.Builder() @@ -514,7 +539,10 @@ public void printWarningIfUploadFails() throws Exception { public void printWarningIfDownloadFails() throws Exception { doThrow(new IOException(io.grpc.Status.UNAVAILABLE.asRuntimeException())) .when(remoteCache) - .downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false)); + .downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false)); CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy); assertThat(entry.hasResult()).isFalse(); @@ -569,7 +597,10 @@ public void orphanedCachedResultIgnored() throws Exception { ActionResult.newBuilder() .addOutputFiles(OutputFile.newBuilder().setPath("/random/file").setDigest(digest)) .build(); - when(remoteCache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(remoteCache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenAnswer( new Answer() { @Override @@ -629,7 +660,10 @@ public Void answer(InvocationOnMock invocation) { @Test public void failedCacheActionAsCacheMiss() throws Exception { ActionResult actionResult = ActionResult.newBuilder().setExitCode(1).build(); - when(remoteCache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(remoteCache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(actionResult); CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy); @@ -645,7 +679,8 @@ public void testDownloadMinimal() throws Exception { cache = remoteSpawnCacheWithOptions(remoteOptions); ActionResult success = ActionResult.newBuilder().setExitCode(0).build(); - when(remoteCache.downloadActionResult(any(), /* inlineOutErr= */ eq(false))) + when(remoteCache.downloadActionResult( + any(RemoteActionExecutionContext.class), any(), /* inlineOutErr= */ eq(false))) .thenReturn(success); // act @@ -668,7 +703,8 @@ public void testDownloadMinimalIoError() throws Exception { IOException downloadFailure = new IOException("downloadMinimal failed"); ActionResult success = ActionResult.newBuilder().setExitCode(0).build(); - when(remoteCache.downloadActionResult(any(), /* inlineOutErr= */ eq(false))) + when(remoteCache.downloadActionResult( + any(RemoteActionExecutionContext.class), any(), /* inlineOutErr= */ eq(false))) .thenReturn(success); when(remoteCache.downloadMinimal( any(), any(), anyCollection(), any(), any(), any(), any(), any())) 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 ebb5d19ae9df4a..e6851d78392f2c 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 @@ -78,6 +78,7 @@ import com.google.devtools.build.lib.exec.util.FakeOwner; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; 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.RemoteCacheClient.ActionKey; import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; import com.google.devtools.build.lib.remote.options.RemoteOptions; @@ -206,7 +207,10 @@ public void nonCachableSpawnsShouldNotBeCached_remote() throws Exception { // TODO(olaola): verify that the uploaded action has the doNotCache set. verify(cache, never()) - .downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false)); + .downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false)); verify(cache, never()).upload(any(), any(), any(), any(), any(), any()); verifyNoMoreInteractions(localRunner); } @@ -320,7 +324,10 @@ public void treatFailedCachedActionAsCacheMiss_local() throws Exception { remoteOptions.remoteUploadLocalResults = true; ActionResult failedAction = ActionResult.newBuilder().setExitCode(1).build(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(failedAction); RemoteSpawnRunner runner = spy(newSpawnRunner()); @@ -355,7 +362,10 @@ public void treatFailedCachedActionAsCacheMiss_remote() throws Exception { // remotely ActionResult failedAction = ActionResult.newBuilder().setExitCode(1).build(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(failedAction); RemoteSpawnRunner runner = newSpawnRunner(); @@ -395,7 +405,10 @@ public void printWarningIfCacheIsDown() throws Exception { Spawn spawn = newSimpleSpawn(); SpawnExecutionContext policy = getSpawnContext(spawn); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenThrow(new IOException("cache down")); doThrow(new IOException("cache down")) @@ -437,7 +450,10 @@ public void noRemoteExecutorFallbackFails() throws Exception { Spawn spawn = newSimpleSpawn(); SpawnExecutionContext policy = getSpawnContext(spawn); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(null); IOException err = new IOException("local execution error"); @@ -464,7 +480,10 @@ public void remoteCacheErrorFallbackFails() throws Exception { Spawn spawn = newSimpleSpawn(); SpawnExecutionContext policy = getSpawnContext(spawn); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenThrow(new IOException()); IOException err = new IOException("local execution error"); @@ -482,7 +501,10 @@ public void testLocalFallbackFailureRemoteExecutorFailure() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(null); when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) .thenThrow(new IOException()); @@ -611,7 +633,10 @@ public void cacheDownloadFailureTriggersRemoteExecution() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(cachedResult); Exception downloadFailure = new BulkTransferException(new CacheNotFoundException(Digest.getDefaultInstance())); @@ -642,7 +667,10 @@ public void resultsDownloadFailureTriggersRemoteExecutionWithSkipCacheLookup() t RemoteSpawnRunner runner = newSpawnRunner(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(null); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); ActionResult execResult = ActionResult.newBuilder().setExitCode(31).build(); @@ -686,7 +714,10 @@ public void testRemoteExecutionTimeout() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(null); ExecuteResponse resp = ExecuteResponse.newBuilder() @@ -720,7 +751,10 @@ public void testRemoteExecutionTimeoutDoesNotTriggerFallback() throws Exception RemoteSpawnRunner runner = newSpawnRunner(); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(null); ExecuteResponse resp = ExecuteResponse.newBuilder() @@ -752,7 +786,10 @@ public void testRemoteExecutionCommandFailureDoesNotTriggerFallback() throws Exc RemoteSpawnRunner runner = newSpawnRunner(); ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(null); ExecuteResponse failed = ExecuteResponse.newBuilder() @@ -783,7 +820,10 @@ public void testExitCode_executorfailure() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(null); when(executor.executeRemotely(any(ExecuteRequest.class), any(OperationObserver.class))) .thenThrow(new IOException("reasons")); @@ -805,7 +845,10 @@ public void testExitCode_executionfailure() throws Exception { RemoteSpawnRunner runner = newSpawnRunner(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenThrow(new IOException("reasons")); Spawn spawn = newSimpleSpawn(); @@ -884,7 +927,10 @@ public void testDownloadMinimalOnCacheHit() throws Exception { remoteOptions.remoteOutputsMode = RemoteOutputsMode.MINIMAL; ActionResult succeededAction = ActionResult.newBuilder().setExitCode(0).build(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(succeededAction); RemoteSpawnRunner runner = newSpawnRunner(); @@ -938,7 +984,10 @@ public void testDownloadMinimalIoError() throws Exception { remoteOptions.remoteOutputsMode = RemoteOutputsMode.MINIMAL; ActionResult succeededAction = ActionResult.newBuilder().setExitCode(0).build(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(succeededAction); IOException downloadFailure = new IOException("downloadMinimal failed"); when(cache.downloadMinimal(any(), any(), anyCollection(), any(), any(), any(), any(), any())) @@ -971,7 +1020,10 @@ public void testDownloadTopLevel() throws Exception { ActionsTestUtil.createArtifact(outputRoot, outputRoot.getRoot().getRelative("foo.bin")); ActionResult succeededAction = ActionResult.newBuilder().setExitCode(0).build(); - when(cache.downloadActionResult(any(ActionKey.class), /* inlineOutErr= */ eq(false))) + when(cache.downloadActionResult( + any(RemoteActionExecutionContext.class), + any(ActionKey.class), + /* inlineOutErr= */ eq(false))) .thenReturn(succeededAction); RemoteSpawnRunner runner = newSpawnRunner(ImmutableSet.of(topLevelOutput)); 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 7ba42e687aeb9a..63d0d5965b6aed 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 @@ -20,6 +20,7 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; import com.google.devtools.build.lib.vfs.Path; import com.google.protobuf.ByteString; @@ -90,7 +91,7 @@ public ListenableFuture downloadBlob(Digest digest, OutputStream out) { @Override public ListenableFuture downloadActionResult( - ActionKey actionKey, boolean inlineOutErr) { + RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { ActionResult actionResult = ac.get(actionKey); if (actionResult == null) { return Futures.immediateFailedFuture(new CacheNotFoundException(actionKey.getDigest())); 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 35c67c46897f24..8eda8fc4e8dcaf 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 @@ -18,10 +18,15 @@ import build.bazel.remote.execution.v2.ActionCacheGrpc.ActionCacheImplBase; import build.bazel.remote.execution.v2.ActionResult; import build.bazel.remote.execution.v2.GetActionResultRequest; +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; import io.grpc.stub.StreamObserver; /** A basic implementation of an {@link ActionCacheImplBase} service. */ @@ -40,8 +45,13 @@ public ActionCacheServer(OnDiskBlobStoreCache cache, DigestUtil digestUtil) { public void getActionResult( GetActionResultRequest request, StreamObserver responseObserver) { try { + RequestMetadata requestMetadata = TracingMetadataUtils.fromCurrentContext(); + RemoteActionExecutionContext context = + new RemoteActionExecutionContextImpl(requestMetadata, new NetworkTime()); + ActionKey actionKey = digestUtil.asActionKey(request.getActionDigest()); - ActionResult result = cache.downloadActionResult(actionKey, /* inlineOutErr= */ false); + ActionResult result = + cache.downloadActionResult(context, actionKey, /* inlineOutErr= */ false); if (result == null) { responseObserver.onError(StatusUtils.notFoundError(request.getActionDigest()));