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 fb2b07c9ae4cde..cd43b4431402d7 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 @@ -24,7 +24,9 @@ public interface ActionInputPrefetcher { new ActionInputPrefetcher() { @Override public ListenableFuture prefetchFiles( - Iterable inputs, MetadataProvider metadataProvider) { + ActionExecutionMetadata action, + Iterable inputs, + MetadataProvider metadataProvider) { // Do nothing. return immediateVoidFuture(); } @@ -43,7 +45,9 @@ public boolean supportsPartialTreeArtifactInputs() { * @return future success if prefetch is finished or {@link IOException}. */ ListenableFuture prefetchFiles( - Iterable inputs, MetadataProvider metadataProvider); + ActionExecutionMetadata action, + Iterable inputs, + MetadataProvider metadataProvider); /** * Whether the prefetcher is able to fetch individual files in a tree artifact without fetching 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 01ea75faba166f..84b12d11efce26 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 @@ -246,6 +246,7 @@ public ListenableFuture prefetchInputs() return actionExecutionContext .getActionInputPrefetcher() .prefetchFiles( + spawn.getResourceOwner(), getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true) .values(), getMetadataProvider()); 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 3f57b1b5760ded..a037811a8e70e2 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 @@ -33,6 +33,7 @@ import com.google.common.util.concurrent.FutureCallback; 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; @@ -292,6 +293,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, @@ -317,11 +319,13 @@ protected Completable onErrorResumeNext(Throwable error) { */ @Override public ListenableFuture prefetchFiles( + ActionExecutionMetadata action, Iterable inputs, MetadataProvider metadataProvider) { - return prefetchFiles(inputs, metadataProvider, Priority.MEDIUM); + return prefetchFiles(action, inputs, metadataProvider, Priority.MEDIUM); } protected ListenableFuture prefetchFiles( + ActionExecutionMetadata action, Iterable inputs, MetadataProvider metadataProvider, Priority priority) { @@ -346,7 +350,7 @@ protected ListenableFuture prefetchFiles( Flowable transfers = Flowable.fromIterable(files) .flatMapSingle( - input -> toTransferResult(prefetchFile(dirCtx, metadataProvider, input, priority))); + input -> toTransferResult(prefetchFile(action, dirCtx, metadataProvider, input, priority))); Completable prefetch = Completable.using( @@ -357,6 +361,7 @@ protected ListenableFuture prefetchFiles( } private Completable prefetchFile( + ActionExecutionMetadata action, DirectoryContext dirCtx, MetadataProvider metadataProvider, ActionInput input, @@ -386,6 +391,7 @@ private Completable prefetchFile( Completable result = downloadFileNoCheckRx( + action, dirCtx, execRoot.getRelative(execPath), treeRootExecPath != null ? execRoot.getRelative(treeRootExecPath) : null, @@ -461,6 +467,7 @@ private Symlink maybeGetSymlink( * download finished. */ private Completable downloadFileRx( + ActionExecutionMetadata action, DirectoryContext dirCtx, Path path, @Nullable Path treeRoot, @@ -470,10 +477,11 @@ private Completable downloadFileRx( if (!canDownloadFile(path, metadata)) { return Completable.complete(); } - return downloadFileNoCheckRx(dirCtx, path, treeRoot, actionInput, metadata, priority); + return downloadFileNoCheckRx(action, dirCtx, path, treeRoot, actionInput, metadata, priority); } private Completable downloadFileNoCheckRx( + ActionExecutionMetadata action, DirectoryContext dirCtx, Path path, @Nullable Path treeRoot, @@ -498,6 +506,7 @@ private Completable downloadFileNoCheckRx( toCompletable( () -> doDownloadFile( + action, reporter, tempPath, finalPath.relativeTo(execRoot), @@ -542,12 +551,15 @@ private Completable downloadFileNoCheckRx( *

The file will be written into a temporary file and moved to the final destination after the * download finished. */ - public void downloadFile(Path path, @Nullable ActionInput actionInput, FileArtifactValue metadata) + public void downloadFile( + ActionExecutionMetadata action, + Path path, @Nullable ActionInput actionInput, FileArtifactValue metadata) throws IOException, InterruptedException { - getFromFuture(downloadFileAsync(path.asFragment(), actionInput, metadata, Priority.CRITICAL)); + getFromFuture(downloadFileAsync(action, path.asFragment(), actionInput, metadata, Priority.CRITICAL)); } protected ListenableFuture downloadFileAsync( + ActionExecutionMetadata action, PathFragment path, @Nullable ActionInput actionInput, FileArtifactValue metadata, @@ -557,6 +569,7 @@ protected ListenableFuture downloadFileAsync( DirectoryContext::new, dirCtx -> downloadFileRx( + action, dirCtx, execRoot.getFileSystem().getPath(path), /* treeRoot= */ null, @@ -695,7 +708,7 @@ public void finalizeAction(Action action, MetadataHandler metadataHandler) { } if (!inputsToDownload.isEmpty()) { - var future = prefetchFiles(inputsToDownload, metadataHandler, Priority.HIGH); + var future = prefetchFiles(action, inputsToDownload, metadataHandler, Priority.HIGH); addCallback( future, new FutureCallback() { @@ -716,7 +729,7 @@ public void onFailure(Throwable throwable) { } if (!outputsToDownload.isEmpty()) { - var future = prefetchFiles(outputsToDownload, metadataHandler, Priority.LOW); + var future = prefetchFiles(action, outputsToDownload, metadataHandler, Priority.LOW); addCallback( future, new FutureCallback() { diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index 0f2e76ed83aecf..4dd5629f1b1a49 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -9,8 +9,8 @@ filegroup( srcs = glob(["*"]) + [ "//src/main/java/com/google/devtools/build/lib/remote/circuitbreaker:srcs", "//src/main/java/com/google/devtools/build/lib/remote/common:srcs", - "//src/main/java/com/google/devtools/build/lib/remote/downloader:srcs", "//src/main/java/com/google/devtools/build/lib/remote/disk:srcs", + "//src/main/java/com/google/devtools/build/lib/remote/downloader:srcs", "//src/main/java/com/google/devtools/build/lib/remote/grpc:srcs", "//src/main/java/com/google/devtools/build/lib/remote/http:srcs", "//src/main/java/com/google/devtools/build/lib/remote/logging:srcs", @@ -206,6 +206,7 @@ java_library( srcs = ["ToplevelArtifactsDownloader.java"], deps = [ ":abstract_action_input_prefetcher", + "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", 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 dfd21d7d79e602..de9736a995d858 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 @@ -24,6 +24,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.ActionInputMap; import com.google.devtools.build.lib.actions.Artifact; @@ -77,6 +78,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( @@ -111,7 +113,8 @@ boolean isRemote(Path path) { return getRemoteMetadata(path.asFragment()) != null; } - public void updateContext(MetadataInjector metadataInjector) { + public void updateContext(ActionExecutionMetadata action, MetadataInjector metadataInjector) { + this.action = action; this.metadataInjector = metadataInjector; } @@ -579,7 +582,7 @@ private void downloadFileIfRemote(PathFragment path) throws IOException { FileArtifactValue m = getRemoteMetadata(path); if (m != null) { try { - inputFetcher.downloadFile(delegateFs.getPath(path), getActionInput(path), m); + inputFetcher.downloadFile(action, delegateFs.getPath(path), getActionInput(path), m); } catch (InterruptedException e) { Thread.currentThread().interrupt(); throw new IOException( 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 e13606c05cd3e1..04d6fb042cadf0 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 @@ -21,6 +21,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.ListenableFuture; +import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; @@ -96,6 +97,7 @@ protected boolean canDownloadFile(Path path, FileArtifactValue metadata) { @Override protected ListenableFuture doDownloadFile( + ActionExecutionMetadata action, Reporter reporter, Path tempPath, PathFragment execPath, @@ -104,7 +106,7 @@ 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()); 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 2ea7a0f2aa9219..7cf03eb399862f 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 @@ -20,6 +20,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; @@ -84,11 +85,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/remote/ToplevelArtifactsDownloader.java b/src/main/java/com/google/devtools/build/lib/remote/ToplevelArtifactsDownloader.java index c266070a881374..75ad886bb3d5f2 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/ToplevelArtifactsDownloader.java +++ b/src/main/java/com/google/devtools/build/lib/remote/ToplevelArtifactsDownloader.java @@ -25,8 +25,11 @@ import com.google.common.flogger.GoogleLogger; import com.google.common.util.concurrent.FutureCallback; 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.ActionLookupValue; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.analysis.AspectCompleteEvent; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -46,6 +49,7 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.skyframe.MemoizingEvaluator; import com.google.devtools.build.skyframe.SkyValue; +import java.util.HashMap; import javax.annotation.Nullable; /** @@ -122,10 +126,17 @@ private void downloadTestOutput(Path path) { // because test outputs are already downloaded (otherwise it cannot hit the action cache). FileArtifactValue metadata = pathToMetadataConverter.getMetadata(path); ActionInput actionInput = pathToMetadataConverter.getActionInput(path); + ActionExecutionMetadata action; + try { + action = getAction(actionInput); + } catch (InterruptedException ignored) { + Thread.currentThread().interrupt(); + action = null; + } if (metadata != null) { ListenableFuture future = actionInputPrefetcher.downloadFileAsync( - path.asFragment(), actionInput, metadata, Priority.LOW); + action, path.asFragment(), actionInput, metadata, Priority.LOW); addCallback( future, new FutureCallback() { @@ -222,7 +233,8 @@ private boolean shouldDownloadToplevelOutputs(ConfiguredTargetKey configuredTarg private void downloadTargetOutputs( ImmutableMap outputGroups, @Nullable Runfiles runfiles) { - var builder = ImmutableMap.builder(); + HashMap> builder = + new HashMap<>(); try { for (ArtifactsInOutputGroup outputs : outputGroups.values()) { if (!outputs.areImportant()) { @@ -239,27 +251,31 @@ private void downloadTargetOutputs( return; } - var outputsAndMetadata = builder.buildKeepingLast(); - ListenableFuture future = - actionInputPrefetcher.prefetchFiles( - outputsAndMetadata.keySet().stream() - .filter(ToplevelArtifactsDownloader::isNonTreeArtifact) - .collect(toImmutableSet()), - new StaticMetadataProvider(outputsAndMetadata), - Priority.LOW); - - addCallback( - future, - new FutureCallback() { - @Override - public void onSuccess(Void unused) {} - - @Override - public void onFailure(Throwable throwable) { - logger.atWarning().withCause(throwable).log("Failed to download toplevel artifacts."); - } - }, - directExecutor()); + for (var entry : builder.entrySet()) { + var action = entry.getKey(); + var outputsAndMetadata = entry.getValue(); + ListenableFuture future = + actionInputPrefetcher.prefetchFiles( + action, + outputsAndMetadata.keySet().stream() + .filter(ToplevelArtifactsDownloader::isNonTreeArtifact) + .collect(toImmutableSet()), + new StaticMetadataProvider(outputsAndMetadata), + Priority.LOW); + + addCallback( + future, + new FutureCallback() { + @Override + public void onSuccess(Void unused) {} + + @Override + public void onFailure(Throwable throwable) { + logger.atWarning().withCause(throwable).log("Failed to download toplevel artifacts."); + } + }, + directExecutor()); + } } private static boolean isNonTreeArtifact(ActionInput actionInput) { @@ -267,7 +283,8 @@ private static boolean isNonTreeArtifact(ActionInput actionInput) { } private void appendRunfiles( - @Nullable Runfiles runfiles, ImmutableMap.Builder builder) + @Nullable Runfiles runfiles, + HashMap> builder) throws InterruptedException { if (runfiles == null) { return; @@ -279,17 +296,41 @@ private void appendRunfiles( } private void appendArtifact( - Artifact artifact, ImmutableMap.Builder builder) + Artifact artifact, + HashMap> builder) throws InterruptedException { + var action = getAction(artifact); + if (action == null) { + return; + } + + var builderForAction = builder.computeIfAbsent(action, unused -> new HashMap<>()); + SkyValue value = memoizingEvaluator.getExistingValue(Artifact.key(artifact)); if (value instanceof ActionExecutionValue) { FileArtifactValue metadata = ((ActionExecutionValue) value).getAllFileValues().get(artifact); if (metadata != null) { - builder.put(artifact, metadata); + builderForAction.put(artifact, metadata); } } else if (value instanceof TreeArtifactValue) { - builder.put(artifact, ((TreeArtifactValue) value).getMetadata()); - builder.putAll(((TreeArtifactValue) value).getChildValues()); + builderForAction.put(artifact, ((TreeArtifactValue) value).getMetadata()); + builderForAction.putAll(((TreeArtifactValue) value).getChildValues()); } } + + @Nullable + private ActionExecutionMetadata getAction(ActionInput artifact) throws InterruptedException { + if (!(artifact instanceof DerivedArtifact)) { + return null; + } + var actionLookupData = ((DerivedArtifact) artifact).getGeneratingActionKey(); + var actionLookupValue = + (ActionLookupValue) + memoizingEvaluator.getExistingValue(actionLookupData.getActionLookupKey()); + if (actionLookupValue == null) { + return null; + } + + return actionLookupValue.getAction(actionLookupData.getActionIndex()); + } } 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 08e941c591142b..7c40c8dff0a32b 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 @@ -358,11 +358,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() { @@ -465,7 +467,8 @@ ActionExecutionValue executeAction( boolean hasDiscoveredInputs) throws ActionExecutionException, InterruptedException { if (actionFileSystem != null) { - updateActionFileSystemContext(actionFileSystem, env, metadataHandler, expandedFilesets); + updateActionFileSystemContext( + action, actionFileSystem, env, metadataHandler, expandedFilesets); } ActionExecutionContext actionExecutionContext = @@ -813,6 +816,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 53517ef2d16434..45ce3d779ea40f 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; @@ -199,6 +200,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 c4528bd348a5a0..de4da2d17718a7 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,16 +17,19 @@ 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 java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertThrows; 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; @@ -34,6 +37,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.Artifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; @@ -76,8 +80,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(); @@ -196,9 +206,9 @@ public void prefetchFiles_fileExists_doNotDownload() MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = spy(createPrefetcher(cas)); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider)); + wait(prefetcher.prefetchFiles(action, metadata.keySet(), metadataProvider)); - 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(); } @@ -213,9 +223,9 @@ public void prefetchFiles_fileExistsButContentMismatches_download() MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = spy(createPrefetcher(cas)); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider)); + wait(prefetcher.prefetchFiles(action, metadata.keySet(), metadataProvider)); - 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"); @@ -230,7 +240,7 @@ public void prefetchFiles_downloadRemoteFiles() throws Exception { MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider)); + wait(prefetcher.prefetchFiles(action, metadata.keySet(), metadataProvider)); assertThat(FileSystemUtils.readContent(a1.getPath(), UTF_8)).isEqualTo("hello world"); assertReadableNonWritableAndExecutable(a1.getPath()); @@ -249,7 +259,7 @@ public void prefetchFiles_downloadRemoteFiles_withMaterializationExecPath() thro MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider)); + wait(prefetcher.prefetchFiles(action, metadata.keySet(), metadataProvider)); assertThat(a.getPath().isSymbolicLink()).isTrue(); assertThat(a.getPath().readSymbolicLink()) @@ -280,7 +290,7 @@ public void prefetchFiles_downloadRemoteTrees() throws Exception { MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(children, metadataProvider)); + wait(prefetcher.prefetchFiles(action, children, metadataProvider)); assertThat(FileSystemUtils.readContent(firstChild.getPath(), UTF_8)).isEqualTo("content1"); assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); @@ -311,7 +321,9 @@ public void prefetchFiles_downloadRemoteTrees_partial() throws Exception { MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(ImmutableList.of(firstChild, secondChild), metadataProvider)); + wait( + prefetcher.prefetchFiles( + action, ImmutableList.of(firstChild, secondChild), metadataProvider)); assertThat(firstChild.getPath().exists()).isFalse(); assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); @@ -341,7 +353,7 @@ public void prefetchFiles_downloadRemoteTrees_withMaterializationExecPath() thro MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(children, metadataProvider)); + wait(prefetcher.prefetchFiles(action, children, metadataProvider)); assertThat(tree.getPath().isSymbolicLink()).isTrue(); assertThat(tree.getPath().readSymbolicLink()) @@ -368,7 +380,7 @@ public void prefetchFiles_missingFiles_fails() throws Exception { assertThrows( Exception.class, - () -> wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadataProvider))); + () -> wait(prefetcher.prefetchFiles(action, ImmutableList.of(a), metadataProvider))); assertThat(prefetcher.downloadedFiles()).isEmpty(); assertThat(prefetcher.downloadsInProgress()).isEmpty(); @@ -385,7 +397,7 @@ public void prefetchFiles_ignoreNonRemoteFiles() throws Exception { MetadataProvider metadataProvider = new StaticMetadataProvider(ImmutableMap.of(a, f)); AbstractActionInputPrefetcher prefetcher = createPrefetcher(new HashMap<>()); - wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadataProvider)); + wait(prefetcher.prefetchFiles(action, ImmutableList.of(a), metadataProvider)); assertThat(prefetcher.downloadedFiles()).isEmpty(); assertThat(prefetcher.downloadsInProgress()).isEmpty(); @@ -412,7 +424,7 @@ public void prefetchFiles_ignoreNonRemoteFiles_tree() throws Exception { MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(children, metadataProvider)); + wait(prefetcher.prefetchFiles(action, children, metadataProvider)); assertThat(firstChild.getPath().exists()).isFalse(); assertThat(FileSystemUtils.readContent(secondChild.getPath(), UTF_8)).isEqualTo("content2"); @@ -439,7 +451,9 @@ public void prefetchFiles_treeFiles_minimizeFilesystemOperations() throws Except MetadataProvider metadataProvider = new StaticMetadataProvider(metadata); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - wait(prefetcher.prefetchFiles(ImmutableList.of(firstChild, secondChild), metadataProvider)); + wait( + prefetcher.prefetchFiles( + action, ImmutableList.of(firstChild, secondChild), metadataProvider)); verify(fs, times(1)).createWritableDirectory(tree.getPath().asFragment()); verify(fs, times(1)).createWritableDirectory(tree.getPath().getChild("subdir").asFragment()); @@ -463,7 +477,8 @@ public void prefetchFiles_multipleThreads_downloadIsCancelled() throws Exception new Thread( () -> { try { - wait(prefetcher.prefetchFiles(ImmutableList.of(artifact), metadataProvider)); + wait( + prefetcher.prefetchFiles(action, ImmutableList.of(artifact), metadataProvider)); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } @@ -473,7 +488,8 @@ public void prefetchFiles_multipleThreads_downloadIsCancelled() throws Exception new Thread( () -> { try { - wait(prefetcher.prefetchFiles(ImmutableList.of(artifact), metadataProvider)); + wait( + prefetcher.prefetchFiles(action, ImmutableList.of(artifact), metadataProvider)); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } @@ -510,7 +526,8 @@ public void prefetchFiles_multipleThreads_downloadIsNotCancelledByOtherThreads() new Thread( () -> { try { - wait(prefetcher.prefetchFiles(ImmutableList.of(artifact), metadataProvider)); + wait( + prefetcher.prefetchFiles(action, ImmutableList.of(artifact), metadataProvider)); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing } @@ -521,7 +538,8 @@ public void prefetchFiles_multipleThreads_downloadIsNotCancelledByOtherThreads() new Thread( () -> { try { - wait(prefetcher.prefetchFiles(ImmutableList.of(artifact), metadataProvider)); + wait( + prefetcher.prefetchFiles(action, ImmutableList.of(artifact), metadataProvider)); successful.set(true); } catch (IOException | ExecException | InterruptedException ignored) { // do nothing @@ -558,7 +576,7 @@ public void downloadFile_downloadRemoteFiles() throws Exception { Artifact a1 = createRemoteArtifact("file1", "hello world", metadata, cas); AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); - prefetcher.downloadFile(a1.getPath(), /* actionInput= */ null, metadata.get(a1)); + prefetcher.downloadFile(action, a1.getPath(), /* actionInput= */ null, metadata.get(a1)); assertThat(FileSystemUtils.readContent(a1.getPath(), UTF_8)).isEqualTo("hello world"); assertThat(a1.getPath().isExecutable()).isTrue(); @@ -587,7 +605,8 @@ public void downloadFile_onInterrupt_deletePartialDownloadedFile() throws Except new Thread( () -> { try { - prefetcher.downloadFile(a1.getPath(), /* actionInput= */ null, metadata.get(a1)); + prefetcher.downloadFile( + action, a1.getPath(), /* actionInput= */ null, metadata.get(a1)); } catch (IOException ignored) { // Intentionally left empty } catch (InterruptedException e) { @@ -614,7 +633,8 @@ public void missingInputs_addedToList() { AbstractActionInputPrefetcher prefetcher = createPrefetcher(cas); assertThrows( - Exception.class, () -> wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider))); + Exception.class, + () -> wait(prefetcher.prefetchFiles(action, metadata.keySet(), metadataProvider))); assertThat(prefetcher.getMissingActionInputs()).contains(a); } @@ -645,8 +665,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")); @@ -655,7 +675,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 9b33e0778a2a3d..1129fef2f11932 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 @@ -27,6 +27,7 @@ import com.google.common.collect.ImmutableMap; import com.google.common.hash.HashCode; import com.google.common.util.concurrent.Futures; +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.Artifact.SpecialArtifact; @@ -84,7 +85,7 @@ protected RemoteActionFileSystem createActionFileSystem( inputs, outputs, inputFetcher); - remoteActionFileSystem.updateContext(metadataInjector); + remoteActionFileSystem.updateContext(mock(ActionExecutionMetadata.class), metadataInjector); remoteActionFileSystem.createDirectoryAndParents(outputRoot.getRoot().asPath().asFragment()); return remoteActionFileSystem; } @@ -118,7 +119,8 @@ public void testGetInputStream() throws Exception { return Futures.immediateFuture(null); }) .when(inputFetcher) - .downloadFile(eq(remoteArtifact.getPath()), any(), eq(inputs.getMetadata(remoteArtifact))); + .downloadFile( + any(), eq(remoteArtifact.getPath()), any(), eq(inputs.getMetadata(remoteArtifact))); // act Path remoteActionFsPath = actionFs.getPath(remoteArtifact.getPath().asFragment()); @@ -133,7 +135,8 @@ public void testGetInputStream() throws Exception { assertThat(actualRemoteContents).isEqualTo("remote contents"); assertThat(actualLocalContents).isEqualTo("local contents"); verify(inputFetcher) - .downloadFile(eq(remoteArtifact.getPath()), any(), eq(inputs.getMetadata(remoteArtifact))); + .downloadFile( + any(), eq(remoteArtifact.getPath()), any(), eq(inputs.getMetadata(remoteArtifact))); 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 fca68db8b6f160..fcb1485ede0b69 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 @@ -98,7 +98,7 @@ public void testStagingVirtualActionInput() throws Exception { VirtualActionInput a = ActionsTestUtil.createVirtualActionInput("file1", "hello world"); // act - wait(actionInputFetcher.prefetchFiles(ImmutableList.of(a), metadataProvider)); + wait(actionInputFetcher.prefetchFiles(action, ImmutableList.of(a), metadataProvider)); // assert Path p = execRoot.getRelative(a.getExecPath()); @@ -128,7 +128,7 @@ public void testStagingEmptyVirtualActionInput() throws Exception { // act wait( actionInputFetcher.prefetchFiles( - ImmutableList.of(VirtualActionInput.EMPTY_MARKER), metadataProvider)); + action, ImmutableList.of(VirtualActionInput.EMPTY_MARKER), metadataProvider)); // assert that nothing happened assertThat(actionInputFetcher.downloadedFiles()).isEmpty(); @@ -145,7 +145,7 @@ public void prefetchFiles_missingFiles_failsWithSpecificMessage() throws Excepti var error = assertThrows( ExecException.class, - () -> wait(prefetcher.prefetchFiles(ImmutableList.of(a), metadataProvider))); + () -> wait(prefetcher.prefetchFiles(action, ImmutableList.of(a), metadataProvider))); assertThat(prefetcher.downloadedFiles()).isEmpty(); assertThat(prefetcher.downloadsInProgress()).isEmpty();