diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java index 16205eddb30194..8b65a8a5eec19e 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java @@ -34,6 +34,7 @@ public interface MetadataSupplier { new ActionInputPrefetcher() { @Override public ListenableFuture prefetchFiles( + ActionExecutionMetadata action, Iterable inputs, MetadataSupplier metadataSupplier, Priority priority) { @@ -79,7 +80,10 @@ public enum Priority { * @return future success if prefetch is finished or {@link IOException}. */ ListenableFuture prefetchFiles( - Iterable inputs, MetadataSupplier metadataSupplier, Priority priority); + ActionExecutionMetadata action, + Iterable inputs, + MetadataSupplier metadataSupplier, + Priority priority); /** * Whether the prefetcher requires the metadata for a tree artifact to be available whenever one diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index 907039fd857481..2b0030fd187b7e 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -252,6 +252,7 @@ public ListenableFuture prefetchInputs() actionExecutionContext .getActionInputPrefetcher() .prefetchFiles( + spawn.getResourceOwner(), getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true) .values(), getInputMetadataProvider()::getInputMetadata, diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java index 86a5ec7ad31a2f..d4533595848f76 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java @@ -29,6 +29,7 @@ import com.google.common.flogger.GoogleLogger; import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputPrefetcher; import com.google.devtools.build.lib.actions.Artifact; @@ -263,6 +264,7 @@ private boolean shouldDownloadFile(Path path, FileArtifactValue metadata) { * @param tempPath the temporary path which the input should be written to. */ protected abstract ListenableFuture doDownloadFile( + ActionExecutionMetadata action, Reporter reporter, Path tempPath, PathFragment execPath, @@ -285,6 +287,7 @@ protected void prefetchVirtualActionInput(VirtualActionInput input) throws IOExc */ @Override public ListenableFuture prefetchFiles( + ActionExecutionMetadata action, Iterable inputs, MetadataSupplier metadataSupplier, Priority priority) { @@ -308,7 +311,8 @@ public ListenableFuture prefetchFiles( Flowable transfers = Flowable.fromIterable(files) - .flatMapSingle(input -> prefetchFile(dirCtx, metadataSupplier, input, priority)); + .flatMapSingle( + input -> prefetchFile(action, dirCtx, metadataSupplier, input, priority)); Completable prefetch = Completable.using( @@ -318,6 +322,7 @@ public ListenableFuture prefetchFiles( } private Single prefetchFile( + ActionExecutionMetadata action, DirectoryContext dirCtx, MetadataSupplier metadataSupplier, ActionInput input, @@ -347,6 +352,7 @@ private Single prefetchFile( Completable result = downloadFileNoCheckRx( + action, dirCtx, execRoot.getRelative(execPath), treeRootExecPath != null ? execRoot.getRelative(treeRootExecPath) : null, @@ -421,6 +427,7 @@ private Symlink maybeGetSymlink( } private Completable downloadFileNoCheckRx( + ActionExecutionMetadata action, DirectoryContext dirCtx, Path path, @Nullable Path treeRoot, @@ -445,6 +452,7 @@ private Completable downloadFileNoCheckRx( toCompletable( () -> doDownloadFile( + action, reporter, tempPath, finalPath.relativeTo(execRoot), @@ -603,7 +611,7 @@ public void finalizeAction(Action action, OutputMetadataStore outputMetadataStor try (var s = Profiler.instance().profile(ProfilerTask.REMOTE_DOWNLOAD, "Download outputs")) { getFromFuture( prefetchFiles( - outputsToDownload, outputMetadataStore::getOutputMetadata, Priority.HIGH)); + action, outputsToDownload, outputMetadataStore::getOutputMetadata, Priority.HIGH)); } } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index 0dae5c58245144..aeb757d255da8b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -25,6 +25,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.ActionInputMap; @@ -81,6 +82,7 @@ public class RemoteActionFileSystem extends DelegateFileSystem { private final RemoteActionInputFetcher inputFetcher; private final RemoteInMemoryFileSystem remoteOutputTree; + @Nullable private ActionExecutionMetadata action = null; @Nullable private MetadataInjector metadataInjector = null; RemoteActionFileSystem( @@ -121,7 +123,8 @@ private boolean isRemote(PathFragment path) { return getRemoteMetadata(path) != null; } - public void updateContext(MetadataInjector metadataInjector) { + public void updateContext(ActionExecutionMetadata action, MetadataInjector metadataInjector) { + this.action = action; this.metadataInjector = metadataInjector; } @@ -625,7 +628,7 @@ private void downloadFileIfRemote(PathFragment path) throws IOException { } getFromFuture( inputFetcher.prefetchFiles( - ImmutableList.of(input), this::getInputMetadata, Priority.CRITICAL)); + action, ImmutableList.of(input), this::getInputMetadata, Priority.CRITICAL)); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new IOException(String.format("Received interrupt while fetching file '%s'", path), e); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java index 2ce36bc8859f12..f560cd60e69bfb 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java @@ -14,18 +14,15 @@ package com.google.devtools.build.lib.remote; import static com.google.common.base.Preconditions.checkArgument; -import static com.google.devtools.build.lib.remote.common.ProgressStatusListener.NO_ACTION; import build.bazel.remote.execution.v2.Digest; import build.bazel.remote.execution.v2.RequestMetadata; import com.google.common.base.Preconditions; import com.google.common.util.concurrent.ListenableFuture; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; -import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress; import com.google.devtools.build.lib.events.Reporter; -import com.google.devtools.build.lib.exec.SpawnProgressEvent; -import com.google.devtools.build.lib.remote.RemoteCache.DownloadProgressReporter; import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.remote.util.TempPathGenerator; @@ -79,6 +76,7 @@ protected boolean canDownloadFile(Path path, FileArtifactValue metadata) { @Override protected ListenableFuture doDownloadFile( + ActionExecutionMetadata action, Reporter reporter, Path tempPath, PathFragment execPath, @@ -87,47 +85,18 @@ protected ListenableFuture doDownloadFile( throws IOException { checkArgument(metadata.isRemote(), "Cannot download file that is not a remote file."); RequestMetadata requestMetadata = - TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "prefetcher", null); + TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "prefetcher", action); RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata); Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize()); - DownloadProgressReporter downloadProgressReporter; - if (priority == Priority.LOW) { - // Only report download progress for toplevel outputs - downloadProgressReporter = - new DownloadProgressReporter( - /* includeFile= */ false, - progress -> reporter.post(new DownloadProgress(progress)), - execPath.toString(), - metadata.getSize()); - } else { - downloadProgressReporter = new DownloadProgressReporter(NO_ACTION, "", 0); - } - - return remoteCache.downloadFile(context, tempPath, digest, downloadProgressReporter); - } - - public static class DownloadProgress implements FetchProgress { - private final SpawnProgressEvent progress; - - public DownloadProgress(SpawnProgressEvent progress) { - this.progress = progress; - } - - @Override - public String getResourceIdentifier() { - return progress.progressId(); - } - - @Override - public String getProgress() { - return progress.progress(); - } - - @Override - public boolean isFinished() { - return progress.finished(); - } + return remoteCache.downloadFile( + context, + tempPath, + digest, + new RemoteCache.DownloadProgressReporter( + progress -> progress.postTo(reporter, action), + execPath.toString(), + digest.getSizeBytes())); } } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java index 996d1ca65616bc..9fcdb59e356a2d 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.eventbus.Subscribe; import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionInputMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; @@ -99,11 +100,12 @@ public FileSystem createActionFileSystem( @Override public void updateActionFileSystemContext( + ActionExecutionMetadata action, FileSystem actionFileSystem, Environment env, MetadataInjector injector, ImmutableMap> filesets) { - ((RemoteActionFileSystem) actionFileSystem).updateContext(injector); + ((RemoteActionFileSystem) actionFileSystem).updateContext(action, injector); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 2c5bf381e34519..5d70ff315aad35 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -382,11 +382,13 @@ FileSystem createActionFileSystem( } private void updateActionFileSystemContext( + Action action, FileSystem actionFileSystem, Environment env, MetadataInjector metadataInjector, ImmutableMap> filesets) { - outputService.updateActionFileSystemContext(actionFileSystem, env, metadataInjector, filesets); + outputService.updateActionFileSystemContext( + action, actionFileSystem, env, metadataInjector, filesets); } void executionOver() { @@ -489,7 +491,8 @@ ActionExecutionValue executeAction( boolean hasDiscoveredInputs) throws ActionExecutionException, InterruptedException { if (actionFileSystem != null) { - updateActionFileSystemContext(actionFileSystem, env, metadataHandler, expandedFilesets); + updateActionFileSystemContext( + action, actionFileSystem, env, metadataHandler, expandedFilesets); } ActionExecutionContext actionExecutionContext = @@ -834,6 +837,7 @@ NestedSet discoverInputs( threadStateReceiverFactory.apply(actionLookupData)); if (actionFileSystem != null) { updateActionFileSystemContext( + action, actionFileSystem, env, THROWING_METADATA_INJECTOR_FOR_ACTIONFS, diff --git a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java index 840a6aa142b4b8..b432033fbcac20 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java +++ b/src/main/java/com/google/devtools/build/lib/vfs/OutputService.java @@ -18,6 +18,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.Action; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionInputMap; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactPathResolver; @@ -196,6 +197,7 @@ default FileSystem createActionFileSystem( * @param filesets The Fileset symlinks known for this action. */ default void updateActionFileSystemContext( + ActionExecutionMetadata action, FileSystem actionFileSystem, Environment env, MetadataInjector injector, diff --git a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java index 9f636e4af0bc68..22aba982d61078 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java @@ -17,6 +17,7 @@ import static com.google.common.base.Throwables.throwIfInstanceOf; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.NULL_ACTION_OWNER; import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.createTreeArtifactWithGeneratingAction; import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture; import static java.nio.charset.StandardCharsets.UTF_8; @@ -24,10 +25,12 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -35,6 +38,7 @@ import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; import com.google.common.util.concurrent.SettableFuture; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputPrefetcher.MetadataSupplier; import com.google.devtools.build.lib.actions.ActionInputPrefetcher.Priority; @@ -78,8 +82,14 @@ public abstract class ActionInputPrefetcherTestBase { protected ArtifactRoot artifactRoot; protected TempPathGenerator tempPathGenerator; + protected ActionExecutionMetadata action; + @Before public void setUp() throws IOException { + action = mock(ActionExecutionMetadata.class); + when(action.getMnemonic()).thenReturn("DummyAction"); + when(action.getOwner()).thenReturn(NULL_ACTION_OWNER); + fs = SpiedFileSystem.createInMemorySpy(); execRoot = fs.getPath("/exec"); execRoot.createDirectoryAndParents(); @@ -201,9 +211,9 @@ public void prefetchFiles_fileExists_doNotDownload() FileSystemUtils.writeContent(a.getPath(), "hello world".getBytes(UTF_8)); AbstractActionInputPrefetcher prefetcher = spy(createPrefetcher(cas)); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(action, metadata.keySet(), metadata::get, Priority.MEDIUM)); - verify(prefetcher, never()).doDownloadFile(any(), any(), any(), any(), any()); + verify(prefetcher, never()).doDownloadFile(eq(action), any(), any(), any(), any(), any()); assertThat(prefetcher.downloadedFiles()).containsExactly(a.getPath()); assertThat(prefetcher.downloadsInProgress()).isEmpty(); } @@ -217,9 +227,9 @@ public void prefetchFiles_fileExistsButContentMismatches_download() FileSystemUtils.writeContent(a.getPath(), "hello world local".getBytes(UTF_8)); AbstractActionInputPrefetcher prefetcher = spy(createPrefetcher(cas)); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(action, metadata.keySet(), metadata::get, Priority.MEDIUM)); - verify(prefetcher).doDownloadFile(any(), any(), eq(a.getExecPath()), any(), any()); + verify(prefetcher).doDownloadFile(eq(action), any(), any(), eq(a.getExecPath()), any(), any()); assertThat(prefetcher.downloadedFiles()).containsExactly(a.getPath()); assertThat(prefetcher.downloadsInProgress()).isEmpty(); assertThat(FileSystemUtils.readContent(a.getPath(), UTF_8)).isEqualTo("hello world remote"); @@ -233,7 +243,7 @@ public void prefetchFiles_downloadRemoteFiles() throws Exception { Artifact a2 = createRemoteArtifact("file2", "fizz buzz", metadata, cas); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(action, metadata.keySet(), metadata::get, Priority.MEDIUM)); assertThat(FileSystemUtils.readContent(a1.getPath(), UTF_8)).isEqualTo("hello world"); assertReadableNonWritableAndExecutable(a1.getPath()); @@ -251,7 +261,7 @@ public void prefetchFiles_downloadRemoteFiles_withMaterializationExecPath() thro Artifact a = createRemoteArtifact("file", "hello world", targetExecPath, metadata, cas); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(action, metadata.keySet(), metadata::get, Priority.MEDIUM)); assertThat(a.getPath().isSymbolicLink()).isTrue(); assertThat(a.getPath().readSymbolicLink()) @@ -281,7 +291,7 @@ public void prefetchFiles_downloadRemoteTrees() throws Exception { AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(children, metadata::get, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(action, children, metadata::get, Priority.MEDIUM)); assertThat(FileSystemUtils.readContent(firstChild.getPath(), UTF_8)).isEqualTo("content1"); assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); @@ -313,7 +323,7 @@ public void prefetchFiles_downloadRemoteTrees_partial() throws Exception { wait( prefetcher.prefetchFiles( - ImmutableList.of(firstChild, secondChild), metadata::get, Priority.MEDIUM)); + action, ImmutableList.of(firstChild, secondChild), metadata::get, Priority.MEDIUM)); assertThat(firstChild.getPath().exists()).isFalse(); assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); @@ -342,7 +352,7 @@ public void prefetchFiles_downloadRemoteTrees_withMaterializationExecPath() thro AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(children, metadata::get, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(action, children, metadata::get, Priority.MEDIUM)); assertThat(tree.getPath().isSymbolicLink()).isTrue(); assertThat(tree.getPath().readSymbolicLink()) @@ -368,7 +378,10 @@ public void prefetchFiles_missingFiles_fails() throws Exception { assertThrows( Exception.class, - () -> wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadata::get, Priority.MEDIUM))); + () -> + wait( + prefetcher.prefetchFiles( + action, ImmutableList.of(a), metadata::get, Priority.MEDIUM))); assertThat(prefetcher.downloadedFiles()).isEmpty(); assertThat(prefetcher.downloadsInProgress()).isEmpty(); @@ -385,7 +398,7 @@ public void prefetchFiles_ignoreNonRemoteFiles() throws Exception { ImmutableMap metadata = ImmutableMap.of(a, f); AbstractActionInputPrefetcher prefetcher = createPrefetcher(new HashMap<>()); - wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadata::get, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(action, ImmutableList.of(a), metadata::get, Priority.MEDIUM)); assertThat(prefetcher.downloadedFiles()).isEmpty(); assertThat(prefetcher.downloadsInProgress()).isEmpty(); @@ -411,7 +424,7 @@ public void prefetchFiles_ignoreNonRemoteFiles_tree() throws Exception { AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(children, metadata::get, Priority.MEDIUM)); + wait(prefetcher.prefetchFiles(action, children, metadata::get, Priority.MEDIUM)); assertThat(firstChild.getPath().exists()).isFalse(); assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); @@ -439,7 +452,7 @@ public void prefetchFiles_treeFiles_minimizeFilesystemOperations() throws Except wait( prefetcher.prefetchFiles( - ImmutableList.of(firstChild, secondChild), metadata::get, Priority.MEDIUM)); + action, ImmutableList.of(firstChild, secondChild), metadata::get, Priority.MEDIUM)); verify(fs, times(1)).createWritableDirectory(tree.getPath().asFragment()); verify(fs, times(1)).createWritableDirectory(tree.getPath().getChild("subdir").asFragment()); @@ -464,7 +477,7 @@ public void prefetchFiles_multipleThreads_downloadIsCancelled() throws Exception try { wait( prefetcher.prefetchFiles( - ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); + action, ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } @@ -476,7 +489,7 @@ public void prefetchFiles_multipleThreads_downloadIsCancelled() throws Exception try { wait( prefetcher.prefetchFiles( - ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); + action, ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } @@ -514,7 +527,7 @@ public void prefetchFiles_multipleThreads_downloadIsNotCancelledByOtherThreads() try { wait( prefetcher.prefetchFiles( - ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); + action, ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } @@ -527,7 +540,7 @@ public void prefetchFiles_multipleThreads_downloadIsNotCancelledByOtherThreads() try { wait( prefetcher.prefetchFiles( - ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); + action, ImmutableList.of(artifact), metadata::get, Priority.MEDIUM)); successful.set(true); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing @@ -571,7 +584,7 @@ public void prefetchFile_interruptingMetadataSupplier_interruptsDownload() throw ListenableFuture future = prefetcher.prefetchFiles( - ImmutableList.of(a1), interruptedMetadataSupplier, Priority.MEDIUM); + action, ImmutableList.of(a1), interruptedMetadataSupplier, Priority.MEDIUM); assertThrows(CancellationException.class, future::get); } @@ -598,7 +611,8 @@ public void prefetchFiles_onInterrupt_deletePartialDownloadedFile() throws Excep () -> { try { getFromFuture( - prefetcher.prefetchFiles(ImmutableList.of(a1), metadata::get, Priority.MEDIUM)); + prefetcher.prefetchFiles( + action, ImmutableList.of(a1), metadata::get, Priority.MEDIUM)); } catch (IOException ignored) { // Intentionally left empty } catch (InterruptedException e) { @@ -625,7 +639,10 @@ public void missingInputs_addedToList() { assertThrows( Exception.class, - () -> wait(prefetcher.prefetchFiles(metadata.keySet(), metadata::get, Priority.MEDIUM))); + () -> + wait( + prefetcher.prefetchFiles( + action, metadata.keySet(), metadata::get, Priority.MEDIUM))); assertThat(prefetcher.getMissingActionInputs()).contains(a); } @@ -656,8 +673,8 @@ protected static void mockDownload( throws IOException { doAnswer( invocation -> { - Path path = invocation.getArgument(1); - FileArtifactValue metadata = invocation.getArgument(3); + Path path = invocation.getArgument(2); + FileArtifactValue metadata = invocation.getArgument(4); byte[] content = cas.get(HashCode.fromBytes(metadata.getDigest())); if (content == null) { return Futures.immediateFailedFuture(new IOException("Not found")); @@ -666,7 +683,7 @@ protected static void mockDownload( return resultSupplier.get(); }) .when(prefetcher) - .doDownloadFile(any(), any(), any(), any(), any()); + .doDownloadFile(any(), any(), any(), any(), any(), any()); } private void assertReadableNonWritableAndExecutable(Path path) throws IOException { diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java index b1d011f15382b6..70e3c9b16cd962 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionFileSystemTest.java @@ -31,6 +31,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.hash.HashCode; import com.google.common.util.concurrent.ListenableFuture; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.ActionInputMap; @@ -105,7 +106,7 @@ protected RemoteActionFileSystem createActionFileSystem( outputs, fileCache, inputFetcher); - remoteActionFileSystem.updateContext(metadataInjector); + remoteActionFileSystem.updateContext(mock(ActionExecutionMetadata.class), metadataInjector); remoteActionFileSystem.createDirectoryAndParents(outputRoot.getRoot().asPath().asFragment()); return remoteActionFileSystem; } @@ -156,7 +157,7 @@ public void testGetInputStream_fromInputArtifactData_forRemoteArtifact() throws FileSystem actionFs = createActionFileSystem(inputs); doAnswer(mockPrefetchFile(artifact.getPath(), "remote contents")) .when(inputFetcher) - .prefetchFiles(eq(ImmutableList.of(artifact)), any(), eq(Priority.CRITICAL)); + .prefetchFiles(any(), eq(ImmutableList.of(artifact)), any(), eq(Priority.CRITICAL)); // act Path actionFsPath = actionFs.getPath(artifact.getPath().asFragment()); @@ -166,7 +167,7 @@ public void testGetInputStream_fromInputArtifactData_forRemoteArtifact() throws assertThat(actionFsPath.getFileSystem()).isSameInstanceAs(actionFs); assertThat(contents).isEqualTo("remote contents"); verify(inputFetcher) - .prefetchFiles(eq(ImmutableList.of(artifact)), any(), eq(Priority.CRITICAL)); + .prefetchFiles(any(), eq(ImmutableList.of(artifact)), any(), eq(Priority.CRITICAL)); verifyNoMoreInteractions(inputFetcher); } @@ -178,7 +179,7 @@ public void testGetInputStream_fromRemoteOutputTree_forDeclaredOutput() throws E injectRemoteFile(actionFs, artifact.getPath().asFragment(), "remote contents"); doAnswer(mockPrefetchFile(artifact.getPath(), "remote contents")) .when(inputFetcher) - .prefetchFiles(eq(ImmutableList.of(artifact)), any(), eq(Priority.CRITICAL)); + .prefetchFiles(any(), eq(ImmutableList.of(artifact)), any(), eq(Priority.CRITICAL)); // act Path actionFsPath = actionFs.getPath(artifact.getPath().asFragment()); @@ -188,7 +189,7 @@ public void testGetInputStream_fromRemoteOutputTree_forDeclaredOutput() throws E assertThat(actionFsPath.getFileSystem()).isSameInstanceAs(actionFs); assertThat(contents).isEqualTo("remote contents"); verify(inputFetcher) - .prefetchFiles(eq(ImmutableList.of(artifact)), any(), eq(Priority.CRITICAL)); + .prefetchFiles(any(), eq(ImmutableList.of(artifact)), any(), eq(Priority.CRITICAL)); verifyNoMoreInteractions(inputFetcher); } @@ -201,7 +202,7 @@ public void testGetInputStream_fromRemoteOutputTree_forUndeclaredOutput() throws injectRemoteFile(actionFs, path.asFragment(), "remote contents"); doAnswer(mockPrefetchFile(path, "remote contents")) .when(inputFetcher) - .prefetchFiles(eq(ImmutableList.of(input)), any(), eq(Priority.CRITICAL)); + .prefetchFiles(any(), eq(ImmutableList.of(input)), any(), eq(Priority.CRITICAL)); // act Path actionFsPath = actionFs.getPath(path.asFragment()); @@ -210,7 +211,8 @@ public void testGetInputStream_fromRemoteOutputTree_forUndeclaredOutput() throws // assert assertThat(actionFsPath.getFileSystem()).isSameInstanceAs(actionFs); assertThat(contents).isEqualTo("remote contents"); - verify(inputFetcher).prefetchFiles(eq(ImmutableList.of(input)), any(), eq(Priority.CRITICAL)); + verify(inputFetcher) + .prefetchFiles(any(), eq(ImmutableList.of(input)), any(), eq(Priority.CRITICAL)); verifyNoMoreInteractions(inputFetcher); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java index 1d0d2cefdc5012..1a6b4fa5e7348a 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java @@ -100,7 +100,7 @@ public void testStagingVirtualActionInput() throws Exception { // act wait( actionInputFetcher.prefetchFiles( - ImmutableList.of(a), (ActionInput unused) -> null, Priority.MEDIUM)); + action, ImmutableList.of(a), (ActionInput unused) -> null, Priority.MEDIUM)); // assert Path p = execRoot.getRelative(a.getExecPath()); @@ -128,6 +128,7 @@ public void testStagingEmptyVirtualActionInput() throws Exception { // act wait( actionInputFetcher.prefetchFiles( + action, ImmutableList.of(VirtualActionInput.EMPTY_MARKER), (ActionInput unused) -> null, Priority.MEDIUM)); @@ -148,7 +149,8 @@ public void prefetchFiles_missingFiles_failsWithSpecificMessage() throws Excepti BulkTransferException.class, () -> wait( - prefetcher.prefetchFiles(ImmutableList.of(a), metadata::get, Priority.MEDIUM))); + prefetcher.prefetchFiles( + action, ImmutableList.of(a), metadata::get, Priority.MEDIUM))); assertThat(prefetcher.downloadedFiles()).isEmpty(); assertThat(prefetcher.downloadsInProgress()).isEmpty();