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 6d142bf5df60cb..3b3e1051dce7b0 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 @@ -48,6 +48,7 @@ import com.google.common.util.concurrent.SettableFuture; import com.google.devtools.build.lib.authandtls.CallCredentialsProvider; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent; 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; @@ -81,6 +82,9 @@ public class GrpcCacheClient implements RemoteCacheClient, MissingDigestsFinder { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT = + SpawnCheckingCacheEvent.create("remote-cache"); + private final CallCredentialsProvider callCredentialsProvider; private final ReferenceCountedChannel channel; private final RemoteOptions options; @@ -274,6 +278,10 @@ public ListenableFuture getAuthority() { @Override public ListenableFuture downloadActionResult( RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { + if (context.getSpawnExecutionContext() != null) { + context.getSpawnExecutionContext().report(SPAWN_CHECKING_CACHE_EVENT); + } + GetActionResultRequest request = GetActionResultRequest.newBuilder() .setInstanceName(options.remoteInstanceName) diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index e6890842321789..7a6dcb47253d78 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -605,7 +605,7 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context buildRequestId, commandId, actionKey.getDigest().getHash(), spawn.getResourceOwner()); RemoteActionExecutionContext remoteActionExecutionContext = RemoteActionExecutionContext.create( - spawn, metadata, getWriteCachePolicy(spawn), getReadCachePolicy(spawn)); + spawn, context, metadata, getWriteCachePolicy(spawn), getReadCachePolicy(spawn)); return new RemoteAction( spawn, 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 bee863a5b3b4ec..ad0c51c65e37c9 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 @@ -31,7 +31,6 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.exec.SpawnCache; -import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; @@ -51,9 +50,6 @@ @ThreadSafe // If the RemoteActionCache implementation is thread-safe. final class RemoteSpawnCache implements SpawnCache { - private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT = - SpawnCheckingCacheEvent.create("remote-cache"); - private final Path execRoot; private final RemoteOptions options; private final RemoteExecutionService remoteExecutionService; @@ -101,7 +97,6 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) Profiler prof = Profiler.instance(); if (shouldAcceptCachedResult) { - context.report(SPAWN_CHECKING_CACHE_EVENT); // Metadata will be available in context.current() until we detach. // This is done via a thread-local variable. try { 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 46a58e5927391b..fda0782ea15586 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 @@ -16,6 +16,8 @@ import build.bazel.remote.execution.v2.RequestMetadata; import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.Spawn; +import com.google.devtools.build.lib.exec.SpawnRunner; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import javax.annotation.Nullable; /** A context that provide remote execution related information for executing an action remotely. */ @@ -61,27 +63,35 @@ public CachePolicy addRemoteCache() { } @Nullable private final Spawn spawn; + @Nullable private final SpawnExecutionContext spawnExecutionContext; private final RequestMetadata requestMetadata; private final NetworkTime networkTime; private final CachePolicy writeCachePolicy; private final CachePolicy readCachePolicy; private RemoteActionExecutionContext( - @Nullable Spawn spawn, RequestMetadata requestMetadata, NetworkTime networkTime) { - this.spawn = spawn; - this.requestMetadata = requestMetadata; - this.networkTime = networkTime; - this.writeCachePolicy = CachePolicy.ANY_CACHE; - this.readCachePolicy = CachePolicy.ANY_CACHE; + @Nullable Spawn spawn, + @Nullable SpawnRunner.SpawnExecutionContext spawnExecutionContext, + RequestMetadata requestMetadata, + NetworkTime networkTime) { + this( + spawn, + spawnExecutionContext, + requestMetadata, + networkTime, + CachePolicy.ANY_CACHE, + CachePolicy.ANY_CACHE); } private RemoteActionExecutionContext( @Nullable Spawn spawn, + @Nullable SpawnExecutionContext spawnExecutionContext, RequestMetadata requestMetadata, NetworkTime networkTime, CachePolicy writeCachePolicy, CachePolicy readCachePolicy) { this.spawn = spawn; + this.spawnExecutionContext = spawnExecutionContext; this.requestMetadata = requestMetadata; this.networkTime = networkTime; this.writeCachePolicy = writeCachePolicy; @@ -90,20 +100,42 @@ private RemoteActionExecutionContext( public RemoteActionExecutionContext withWriteCachePolicy(CachePolicy writeCachePolicy) { return new RemoteActionExecutionContext( - spawn, requestMetadata, networkTime, writeCachePolicy, readCachePolicy); + spawn, + spawnExecutionContext, + requestMetadata, + networkTime, + writeCachePolicy, + readCachePolicy); } public RemoteActionExecutionContext withReadCachePolicy(CachePolicy readCachePolicy) { return new RemoteActionExecutionContext( - spawn, requestMetadata, networkTime, writeCachePolicy, readCachePolicy); + spawn, + spawnExecutionContext, + requestMetadata, + networkTime, + writeCachePolicy, + readCachePolicy); } - /** Returns the {@link Spawn} of the action being executed or {@code null}. */ + /** + * Returns the {@link Spawn} of the {@link RemoteAction} being executed, or {@code null} if it has + * no associated {@link Spawn}. + */ @Nullable public Spawn getSpawn() { return spawn; } + /** + * Returns the {@link SpawnExecutionContext} of the {@link RemoteAction} being executed, or {@code + * null} if it has no associated {@link Spawn}. + */ + @Nullable + public SpawnExecutionContext getSpawnExecutionContext() { + return spawnExecutionContext; + } + /** Returns the {@link RequestMetadata} for the action being executed. */ public RequestMetadata getRequestMetadata() { return requestMetadata; @@ -137,7 +169,8 @@ public CachePolicy getReadCachePolicy() { /** Creates a {@link RemoteActionExecutionContext} with given {@link RequestMetadata}. */ public static RemoteActionExecutionContext create(RequestMetadata metadata) { - return new RemoteActionExecutionContext(/*spawn=*/ null, metadata, new NetworkTime()); + return new RemoteActionExecutionContext( + /* spawn= */ null, /* spawnExecutionContext= */ null, metadata, new NetworkTime()); } /** @@ -145,16 +178,23 @@ public static RemoteActionExecutionContext create(RequestMetadata metadata) { * RequestMetadata}. */ public static RemoteActionExecutionContext create( - @Nullable Spawn spawn, RequestMetadata metadata) { - return new RemoteActionExecutionContext(spawn, metadata, new NetworkTime()); + Spawn spawn, SpawnExecutionContext spawnExecutionContext, RequestMetadata metadata) { + return new RemoteActionExecutionContext( + spawn, spawnExecutionContext, metadata, new NetworkTime()); } public static RemoteActionExecutionContext create( - @Nullable Spawn spawn, + Spawn spawn, + SpawnExecutionContext spawnExecutionContext, RequestMetadata requestMetadata, CachePolicy writeCachePolicy, CachePolicy readCachePolicy) { return new RemoteActionExecutionContext( - spawn, requestMetadata, new NetworkTime(), writeCachePolicy, readCachePolicy); + spawn, + spawnExecutionContext, + requestMetadata, + new NetworkTime(), + writeCachePolicy, + readCachePolicy); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD b/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD index 4647fbb694d1a7..57271f3b0a45e2 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/disk/BUILD @@ -15,6 +15,7 @@ java_library( name = "disk", srcs = glob(["*.java"]), deps = [ + "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/remote:store", "//src/main/java/com/google/devtools/build/lib/remote/common", "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", 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 dd3de658bb8ce9..cc54df8ae6783e 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 @@ -30,6 +30,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.exec.SpawnCheckingCacheEvent; import com.google.devtools.build.lib.remote.Store; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; @@ -60,6 +61,10 @@ * safe to trim the cache at the same time a Bazel process is accessing it. */ public class DiskCacheClient implements RemoteCacheClient { + + private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT = + SpawnCheckingCacheEvent.create("disk-cache"); + private final Path root; private final boolean verifyDownloads; private final DigestUtil digestUtil; @@ -222,6 +227,10 @@ public ListenableFuture getAuthority() { @Override public ListenableFuture downloadActionResult( RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { + if (context.getSpawnExecutionContext() != null) { + context.getSpawnExecutionContext().report(SPAWN_CHECKING_CACHE_EVENT); + } + return Futures.transformAsync( Utils.downloadAsActionResult(actionKey, (digest, out) -> download(digest, out, Store.AC)), actionResult -> { diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/BUILD b/src/main/java/com/google/devtools/build/lib/remote/http/BUILD index 18409a937c5c38..525889da598839 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/http/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/http/BUILD @@ -21,6 +21,7 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info", "//src/main/java/com/google/devtools/build/lib/authandtls", + "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/remote:Retrier", "//src/main/java/com/google/devtools/build/lib/remote/common", "//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception", 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 608eae019e3d7e..ba9a8865d0e130 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 @@ -28,6 +28,7 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.SettableFuture; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; +import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent; import com.google.devtools.build.lib.remote.RemoteRetrier; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; @@ -122,6 +123,9 @@ public final class HttpCacheClient implements RemoteCacheClient { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT = + SpawnCheckingCacheEvent.create("remote-cache"); + public static final String AC_PREFIX = "ac/"; public static final String CAS_PREFIX = "cas/"; private static final Pattern INVALID_TOKEN_ERROR = @@ -617,6 +621,10 @@ public ListenableFuture getAuthority() { @Override public ListenableFuture downloadActionResult( RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) { + if (context.getSpawnExecutionContext() != null) { + context.getSpawnExecutionContext().report(SPAWN_CHECKING_CACHE_EVENT); + } + return Futures.transform( retrier.executeAsync( () -> diff --git a/src/test/java/com/google/devtools/build/lib/remote/BUILD b/src/test/java/com/google/devtools/build/lib/remote/BUILD index 9d52a5b9410c90..cfa1be5178d7d4 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/BUILD @@ -11,6 +11,7 @@ filegroup( testonly = 0, srcs = glob(["**"]) + [ "//src/test/java/com/google/devtools/build/lib/remote/circuitbreaker:srcs", + "//src/test/java/com/google/devtools/build/lib/remote/disk:srcs", "//src/test/java/com/google/devtools/build/lib/remote/downloader:srcs", "//src/test/java/com/google/devtools/build/lib/remote/grpc:srcs", "//src/test/java/com/google/devtools/build/lib/remote/http:srcs", 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 299224b1b38973..bbaad1bd15ac1a 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 @@ -22,7 +22,9 @@ import static org.mockito.AdditionalAnswers.answerVoid; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import build.bazel.remote.execution.v2.Action; import build.bazel.remote.execution.v2.ActionCacheGrpc.ActionCacheImplBase; @@ -58,6 +60,7 @@ import com.google.common.util.concurrent.MoreExecutors; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.ArtifactPathResolver; +import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; @@ -65,6 +68,8 @@ import com.google.devtools.build.lib.authandtls.GoogleAuthUtils; import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.events.NullEventHandler; +import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; 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.RemoteActionExecutionContext; @@ -257,7 +262,9 @@ public final void setUp() throws Exception { RequestMetadata metadata = TracingMetadataUtils.buildMetadata( "none", "none", Digest.getDefaultInstance().getHash(), null); - context = RemoteActionExecutionContext.create(metadata); + context = + RemoteActionExecutionContext.create( + mock(Spawn.class), mock(SpawnExecutionContext.class), metadata); retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1)); } @@ -272,6 +279,30 @@ public void tearDown() throws Exception { fakeServer.awaitTermination(); } + @Test + public void testSpawnCheckingCacheEvent() throws Exception { + GrpcCacheClient client = newClient(); + + serviceRegistry.addService( + new ActionCacheImplBase() { + @Override + public void getActionResult( + GetActionResultRequest request, StreamObserver responseObserver) { + responseObserver.onError(Status.NOT_FOUND.asRuntimeException()); + } + }); + + var unused = + getFromFuture( + client.downloadActionResult( + context, + DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")), + /* inlineOutErr= */ false)); + + verify(context.getSpawnExecutionContext()) + .report(SpawnCheckingCacheEvent.create("remote-cache")); + } + @Test public void testVirtualActionInputSupport() throws Exception { RemoteOptions options = Options.getDefaults(RemoteOptions.class); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java index 9293e38164aac7..5a724a7f4a0296 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteCacheTest.java @@ -43,6 +43,7 @@ import com.google.devtools.build.lib.clock.JavaClock; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; +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.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.common.RemoteCacheClient; @@ -92,7 +93,7 @@ public class RemoteCacheTest { @Rule public final RxNoGlobalErrorsRule rxNoGlobalErrorsRule = new RxNoGlobalErrorsRule(); - private RemoteActionExecutionContext context; + private RemoteActionExecutionContext remoteActionExecutionContext; private FileSystem fs; private Path execRoot; ArtifactRoot artifactRoot; @@ -116,7 +117,9 @@ public void setUp() throws Exception { /* inputs= */ NestedSetBuilder.emptySet(Order.STABLE_ORDER), /* outputs= */ ImmutableSet.of(), ResourceSet.ZERO); - context = RemoteActionExecutionContext.create(spawn, metadata); + SpawnExecutionContext spawnExecutionContext = mock(SpawnExecutionContext.class); + remoteActionExecutionContext = + RemoteActionExecutionContext.create(spawn, spawnExecutionContext, metadata); fs = new InMemoryFileSystem(new JavaClock(), DigestHashFunction.SHA256); execRoot = fs.getPath("/execroot/main"); execRoot.createDirectoryAndParents(); @@ -142,10 +145,11 @@ public void testDownloadEmptyBlobAndFile() throws Exception { Digest emptyDigest = digestUtil.compute(new byte[0]); // act and assert - assertThat(getFromFuture(remoteCache.downloadBlob(context, emptyDigest))).isEmpty(); + assertThat(getFromFuture(remoteCache.downloadBlob(remoteActionExecutionContext, emptyDigest))) + .isEmpty(); try (OutputStream out = file.getOutputStream()) { - getFromFuture(remoteCache.downloadFile(context, file, emptyDigest)); + getFromFuture(remoteCache.downloadFile(remoteActionExecutionContext, file, emptyDigest)); } assertThat(file.exists()).isTrue(); assertThat(file.getFileSize()).isEqualTo(0); @@ -165,7 +169,8 @@ public void downloadFile_cancelled_cancelDownload() throws Exception { Path file = execRoot.getRelative("file"); // act - ListenableFuture download = remoteCache.downloadFile(context, file, digest); + ListenableFuture download = + remoteCache.downloadFile(remoteActionExecutionContext, file, digest); download.cancel(/* mayInterruptIfRunning= */ true); // assert @@ -184,7 +189,7 @@ public void downloadOutErr_empty_doNotPerformDownload() throws Exception { waitForBulkTransfer( remoteCache.downloadOutErr( - context, + remoteActionExecutionContext, result.build(), new FileOutErr(execRoot.getRelative("stdout"), execRoot.getRelative("stderr"))), true); @@ -212,7 +217,7 @@ public void testDownloadFileWithSymlinkTemplate() throws Exception { RemoteCache remoteCache = new InMemoryRemoteCache(cas, options, digestUtil); // act - getFromFuture(remoteCache.downloadFile(context, file, helloDigest)); + getFromFuture(remoteCache.downloadFile(remoteActionExecutionContext, file, helloDigest)); // assert assertThat(file.isSymbolicLink()).isTrue(); @@ -229,12 +234,19 @@ public void upload_emptyBlobAndFile_doNotPerformUpload() throws Exception { Digest emptyDigest = fakeFileCache.createScratchInput(ActionInputHelper.fromPath("file"), ""); Path file = execRoot.getRelative("file"); - getFromFuture(remoteCache.uploadBlob(context, emptyDigest, ByteString.EMPTY)); - assertThat(getFromFuture(remoteCache.findMissingDigests(context, ImmutableSet.of(emptyDigest)))) + getFromFuture( + remoteCache.uploadBlob(remoteActionExecutionContext, emptyDigest, ByteString.EMPTY)); + assertThat( + getFromFuture( + remoteCache.findMissingDigests( + remoteActionExecutionContext, ImmutableSet.of(emptyDigest)))) .containsExactly(emptyDigest); - getFromFuture(remoteCache.uploadFile(context, emptyDigest, file)); - assertThat(getFromFuture(remoteCache.findMissingDigests(context, ImmutableSet.of(emptyDigest)))) + getFromFuture(remoteCache.uploadFile(remoteActionExecutionContext, emptyDigest, file)); + assertThat( + getFromFuture( + remoteCache.findMissingDigests( + remoteActionExecutionContext, ImmutableSet.of(emptyDigest)))) .containsExactly(emptyDigest); } @@ -254,8 +266,8 @@ public void upload_deduplicationWorks() throws IOException { Digest digest = fakeFileCache.createScratchInput(ActionInputHelper.fromPath("file"), "content"); Path file = execRoot.getRelative("file"); - remoteCache.uploadFile(context, digest, file); - remoteCache.uploadFile(context, digest, file); + remoteCache.uploadFile(remoteActionExecutionContext, digest, file); + remoteCache.uploadFile(remoteActionExecutionContext, digest, file); assertThat(times.get()).isEqualTo(1); } @@ -286,20 +298,26 @@ public void upload_failedUploads_doNotDeduplicate() throws Exception { RemoteCache remoteCache = newRemoteCache(remoteCacheClient); Digest digest = fakeFileCache.createScratchInput(ActionInputHelper.fromPath("file"), "content"); Path file = execRoot.getRelative("file"); - assertThat(getFromFuture(remoteCache.findMissingDigests(context, ImmutableList.of(digest)))) + assertThat( + getFromFuture( + remoteCache.findMissingDigests( + remoteActionExecutionContext, ImmutableList.of(digest)))) .containsExactly(digest); Exception thrown = null; try { - getFromFuture(remoteCache.uploadFile(context, digest, file)); + getFromFuture(remoteCache.uploadFile(remoteActionExecutionContext, digest, file)); } catch (IOException e) { thrown = e; } assertThat(thrown).isNotNull(); assertThat(thrown).isInstanceOf(IOException.class); - getFromFuture(remoteCache.uploadFile(context, digest, file)); + getFromFuture(remoteCache.uploadFile(remoteActionExecutionContext, digest, file)); - assertThat(getFromFuture(remoteCache.findMissingDigests(context, ImmutableList.of(digest)))) + assertThat( + getFromFuture( + remoteCache.findMissingDigests( + remoteActionExecutionContext, ImmutableList.of(digest)))) .isEmpty(); } @@ -342,7 +360,8 @@ public void ensureInputsPresent_interruptedDuringUploadBlobs_cancelInProgressUpl new Thread( () -> { try { - remoteCache.ensureInputsPresent(context, merkleTree, ImmutableMap.of(), false); + remoteCache.ensureInputsPresent( + remoteActionExecutionContext, merkleTree, ImmutableMap.of(), false); } catch (IOException | InterruptedException ignored) { // ignored } finally { @@ -416,7 +435,8 @@ public void ensureInputsPresent_interruptedDuringUploadBlobs_cancelInProgressUpl Runnable work = () -> { try { - remoteCache.ensureInputsPresent(context, merkleTree, ImmutableMap.of(), false); + remoteCache.ensureInputsPresent( + remoteActionExecutionContext, merkleTree, ImmutableMap.of(), false); } catch (IOException ignored) { // ignored } catch (InterruptedException e) { @@ -509,7 +529,8 @@ public void ensureInputsPresent_interruptedDuringUploadBlobs_cancelInProgressUpl new Thread( () -> { try { - remoteCache.ensureInputsPresent(context, merkleTree1, ImmutableMap.of(), false); + remoteCache.ensureInputsPresent( + remoteActionExecutionContext, merkleTree1, ImmutableMap.of(), false); } catch (IOException ignored) { // ignored } catch (InterruptedException e) { @@ -522,7 +543,8 @@ public void ensureInputsPresent_interruptedDuringUploadBlobs_cancelInProgressUpl new Thread( () -> { try { - remoteCache.ensureInputsPresent(context, merkleTree2, ImmutableMap.of(), false); + remoteCache.ensureInputsPresent( + remoteActionExecutionContext, merkleTree2, ImmutableMap.of(), false); } catch (InterruptedException | IOException ignored) { // ignored } finally { @@ -584,7 +606,9 @@ public void ensureInputsPresent_uploadFailed_propagateErrors() throws Exception IOException e = Assert.assertThrows( IOException.class, - () -> remoteCache.ensureInputsPresent(context, merkleTree, ImmutableMap.of(), false)); + () -> + remoteCache.ensureInputsPresent( + remoteActionExecutionContext, merkleTree, ImmutableMap.of(), false)); assertThat(e).hasMessageThat().contains("upload failed"); } @@ -600,7 +624,8 @@ public void shutdownNow_cancelInProgressUploads() throws Exception { Digest digest = fakeFileCache.createScratchInput(ActionInputHelper.fromPath("file"), "content"); Path file = execRoot.getRelative("file"); - ListenableFuture upload = remoteCache.uploadFile(context, digest, file); + ListenableFuture upload = + remoteCache.uploadFile(remoteActionExecutionContext, digest, file); assertThat(remoteCache.casUploadCache.getInProgressTasks()).contains(digest); remoteCache.shutdownNow(); 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 e1d8d2187162bc..33c00c53c637ab 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 @@ -60,7 +60,6 @@ import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.exec.SpawnCache.CacheHandle; -import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent; import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; @@ -88,8 +87,6 @@ import com.google.devtools.common.options.Options; import java.io.IOException; import java.time.Duration; -import java.util.ArrayList; -import java.util.List; import java.util.SortedMap; import javax.annotation.Nullable; import org.junit.Before; @@ -122,7 +119,6 @@ public class RemoteSpawnCacheTest { private FakeActionInputFileCache fakeFileCache; @Mock private RemoteCache remoteCache; private FileOutErr outErr; - private final List progressUpdates = new ArrayList<>(); private StoredEventHandler eventHandler = new StoredEventHandler(); @@ -203,7 +199,6 @@ public SortedMap getInputMapping( @Override public void report(ProgressStatus progress) { - progressUpdates.add(progress); } @Override @@ -347,7 +342,6 @@ public CachedActionResult answer(InvocationOnMock invocation) { // We expect the CachedLocalSpawnRunner to _not_ write to outErr at all. assertThat(outErr.hasRecordedOutput()).isFalse(); assertThat(outErr.hasRecordedStderr()).isFalse(); - assertThat(progressUpdates).containsExactly(SpawnCheckingCacheEvent.create("remote-cache")); } @Test @@ -373,7 +367,6 @@ public void cacheMiss() throws Exception { doNothing().when(service).uploadOutputs(any(), any()); entry.store(result); verify(service).uploadOutputs(any(), any()); - assertThat(progressUpdates).containsExactly(SpawnCheckingCacheEvent.create("remote-cache")); } @Test @@ -401,7 +394,6 @@ public void noCacheSpawns() throws Exception { .build(); entry.store(result); verifyNoMoreInteractions(remoteCache); - assertThat(progressUpdates).isEmpty(); } } } @@ -435,7 +427,6 @@ public void noRemoteCacheSpawns_remoteCache() throws Exception { .build(); entry.store(result); verifyNoMoreInteractions(remoteCache); - assertThat(progressUpdates).isEmpty(); } } @@ -469,7 +460,6 @@ public void noRemoteCacheSpawns_combinedCache() throws Exception { .build(); entry.store(result); verifyNoMoreInteractions(remoteCache); - assertThat(progressUpdates).isEmpty(); } } @@ -541,7 +531,6 @@ public void failedActionsAreNotUploaded() throws Exception { .build(); entry.store(result); verify(service, never()).uploadOutputs(any(), any()); - assertThat(progressUpdates).containsExactly(SpawnCheckingCacheEvent.create("remote-cache")); } @Test @@ -572,7 +561,6 @@ public void printWarningIfDownloadFails() throws Exception { Event evt = eventHandler.getEvents().get(0); assertThat(evt.getKind()).isEqualTo(EventKind.WARNING); assertThat(evt.getMessage()).contains("UNAVAILABLE"); - assertThat(progressUpdates).containsExactly(SpawnCheckingCacheEvent.create("remote-cache")); } @Test @@ -616,7 +604,6 @@ public CachedActionResult answer(InvocationOnMock invocation) { doNothing().when(service).uploadOutputs(any(), any()); entry.store(result); verify(service).uploadOutputs(any(), eq(result)); - assertThat(progressUpdates).containsExactly(SpawnCheckingCacheEvent.create("remote-cache")); assertThat(eventHandler.getEvents()).isEmpty(); // no warning is printed. } diff --git a/src/test/java/com/google/devtools/build/lib/remote/disk/BUILD b/src/test/java/com/google/devtools/build/lib/remote/disk/BUILD new file mode 100644 index 00000000000000..a154a394bfd136 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/remote/disk/BUILD @@ -0,0 +1,33 @@ +load("@rules_java//java:defs.bzl", "java_test") + +package( + default_applicable_licenses = ["//:license"], + default_testonly = 1, + default_visibility = ["//src:__subpackages__"], +) + +filegroup( + name = "srcs", + testonly = 0, + srcs = glob(["**"]), + visibility = ["//src:__subpackages__"], +) + +java_test( + name = "disk", + srcs = glob(["*.java"]), + test_class = "com.google.devtools.build.lib.AllTests", + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", + "//src/main/java/com/google/devtools/build/lib/remote/common", + "//src/main/java/com/google/devtools/build/lib/remote/disk", + "//src/main/java/com/google/devtools/build/lib/remote/util", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", + "//src/test/java/com/google/devtools/build/lib:test_runner", + "//third_party:junit4", + "//third_party:mockito", + "@remoteapis//:build_bazel_remote_execution_v2_remote_execution_java_proto", + ], +) diff --git a/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheClientTest.java new file mode 100644 index 00000000000000..4e0b48a2e69b53 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheClientTest.java @@ -0,0 +1,68 @@ +// Copyright 2024 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.disk; + +import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import build.bazel.remote.execution.v2.RequestMetadata; +import com.google.devtools.build.lib.actions.Spawn; +import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.util.DigestUtil; +import com.google.devtools.build.lib.vfs.DigestHashFunction; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.SyscallCache; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link DiskCacheClient}. */ +@RunWith(JUnit4.class) +public class DiskCacheClientTest { + private static final DigestUtil DIGEST_UTIL = + new DigestUtil(SyscallCache.NO_CACHE, DigestHashFunction.SHA256); + + private final FileSystem fs = new InMemoryFileSystem(DigestHashFunction.SHA256); + private final Path root = fs.getPath("/"); + private DiskCacheClient client; + private RemoteActionExecutionContext context; + + @Before + public void setUp() throws Exception { + client = new DiskCacheClient(root, /* verifyDownloads= */ true, DIGEST_UTIL); + context = + RemoteActionExecutionContext.create( + mock(Spawn.class), + mock(SpawnExecutionContext.class), + RequestMetadata.getDefaultInstance()); + } + + @Test + public void testSpawnCheckingCacheEvent() throws Exception { + var unused = + getFromFuture( + client.downloadActionResult( + context, + DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")), + /* inlineOutErr= */ false)); + + verify(context.getSpawnExecutionContext()).report(SpawnCheckingCacheEvent.create("disk-cache")); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/remote/http/BUILD b/src/test/java/com/google/devtools/build/lib/remote/http/BUILD index 0193b9ebe5782d..e232778809bf35 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/http/BUILD +++ b/src/test/java/com/google/devtools/build/lib/remote/http/BUILD @@ -21,7 +21,9 @@ java_test( ], test_class = "com.google.devtools.build.lib.AllTests", deps = [ + "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/authandtls", + "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/remote:Retrier", "//src/main/java/com/google/devtools/build/lib/remote/common", "//src/main/java/com/google/devtools/build/lib/remote/http", 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 cfac498e07b677..2b850826fee926 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 @@ -34,7 +34,10 @@ import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.common.util.concurrent.MoreExecutors; +import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions; +import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.remote.RemoteRetrier; import com.google.devtools.build.lib.remote.Retrier; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; @@ -345,10 +348,38 @@ private HttpCacheClient createHttpBlobStore( public void setUp() throws Exception { remoteActionExecutionContext = RemoteActionExecutionContext.create( + mock(Spawn.class), + mock(SpawnExecutionContext.class), TracingMetadataUtils.buildMetadata( "none", "none", Digest.getDefaultInstance().getHash(), null)); } + @Test + public void testSpawnCheckingCacheEvent() throws Exception { + ServerChannel server = null; + try { + ConcurrentHashMap cacheContents = new ConcurrentHashMap<>(); + server = testServer.start(new HttpCacheServerHandler(cacheContents)); + + HttpCacheClient blobStore = + createHttpBlobStore( + server, /* timeoutSeconds= */ 1, /* creds= */ null, new AuthAndTLSOptions()); + + var unused = + getFromFuture( + blobStore.downloadActionResult( + remoteActionExecutionContext, + DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")), + /* inlineOutErr= */ false)); + + verify(remoteActionExecutionContext.getSpawnExecutionContext()) + .report(SpawnCheckingCacheEvent.create("remote-cache")); + + } finally { + testServer.stop(server); + } + } + @Test public void testUpload() throws Exception { ServerChannel server = null;