Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[8.1.0] Distinguish between input and output prefetching in request m… #25058

Merged
merged 1 commit into from
Jan 27, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ public ListenableFuture<Void> prefetchFiles(
ActionExecutionMetadata action,
Iterable<? extends ActionInput> inputs,
MetadataSupplier metadataSupplier,
Priority priority) {
Priority priority,
Reason reason) {
// Do nothing.
return immediateVoidFuture();
}
Expand Down Expand Up @@ -72,6 +73,15 @@ public enum Priority {
LOW,
}

/** The reason for prefetching. */
enum Reason {
/** The requested files are needed as inputs to the given action. */
INPUTS,

/** The requested files are requested as outputs of the given action. */
OUTPUTS,
}

/**
* Initiates best-effort prefetching of all given inputs.
*
Expand All @@ -83,7 +93,8 @@ ListenableFuture<Void> prefetchFiles(
ActionExecutionMetadata action,
Iterable<? extends ActionInput> inputs,
MetadataSupplier metadataSupplier,
Priority priority);
Priority priority,
Reason reason);

/**
* Whether the prefetcher requires the metadata for a tree artifact to be available whenever one
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher.Priority;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher.Reason;
import com.google.devtools.build.lib.actions.ArtifactExpander;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
Expand Down Expand Up @@ -290,7 +291,8 @@ public ListenableFuture<Void> prefetchInputs() throws ForbiddenActionInputExcept
getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true)
.values(),
getInputMetadataProvider()::getInputMetadata,
Priority.MEDIUM),
Priority.MEDIUM,
Reason.INPUTS),
BulkTransferException.class,
(BulkTransferException e) -> {
if (executionOptions.useNewExitCodeForLostInputs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,14 +242,14 @@ protected abstract ListenableFuture<Void> doDownloadFile(
Path tempPath,
PathFragment execPath,
FileArtifactValue metadata,
Priority priority)
Priority priority,
Reason reason)
throws IOException;

protected void prefetchVirtualActionInput(VirtualActionInput input) throws IOException {}

/**
* Fetches remotely stored action outputs, that are inputs to this spawn, and stores them under
* their path in the output base.
* Fetches remotely stored action outputs and stores them under their path in the output base.
*
* <p>The {@code inputs} may not contain any unexpanded directories.
*
Expand All @@ -263,7 +263,8 @@ public ListenableFuture<Void> prefetchFiles(
ActionExecutionMetadata action,
Iterable<? extends ActionInput> inputs,
MetadataSupplier metadataSupplier,
Priority priority) {
Priority priority,
Reason reason) {
List<ActionInput> files = new ArrayList<>();

for (ActionInput input : inputs) {
Expand Down Expand Up @@ -297,7 +298,8 @@ public ListenableFuture<Void> prefetchFiles(
try (var s = Profiler.instance().profile("compose prefetches")) {
for (var file : files) {
transfers.add(
prefetchFile(action, dirsWithOutputPermissions, metadataSupplier, file, priority));
prefetchFile(
action, dirsWithOutputPermissions, metadataSupplier, file, priority, reason));
}
}

Expand Down Expand Up @@ -328,7 +330,8 @@ private ListenableFuture<Void> prefetchFile(
Set<Path> dirsWithOutputPermissions,
MetadataSupplier metadataSupplier,
ActionInput input,
Priority priority) {
Priority priority,
Reason reason) {
try {
if (input instanceof VirtualActionInput virtualActionInput) {
prefetchVirtualActionInput(virtualActionInput);
Expand Down Expand Up @@ -360,7 +363,8 @@ private ListenableFuture<Void> prefetchFile(
dirsWithOutputPermissions,
input,
metadata,
priority);
priority,
reason);

if (symlink != null) {
result = result.andThen(plantSymlink(symlink));
Expand Down Expand Up @@ -479,7 +483,8 @@ private Completable downloadFileNoCheckRx(
Set<Path> dirsWithOutputPermissions,
ActionInput actionInput,
FileArtifactValue metadata,
Priority priority) {
Priority priority,
Reason reason) {
// If the path to be prefetched is a non-dangling symlink, prefetch its target path instead.
// Note that this only applies to symlinks created by spawns (or, currently, with the internal
// version of BwoB); symlinks created in-process through an ActionFileSystem should have already
Expand Down Expand Up @@ -524,7 +529,8 @@ private Completable downloadFileNoCheckRx(
tempPath,
finalPath.relativeTo(execRoot),
metadata,
priority),
priority,
reason),
directExecutor())
.doOnComplete(
() -> {
Expand Down Expand Up @@ -686,7 +692,11 @@ public void finalizeAction(Action action, OutputMetadataStore outputMetadataStor
try (var s = Profiler.instance().profile(ProfilerTask.REMOTE_DOWNLOAD, "Download outputs")) {
getFromFuture(
prefetchFiles(
action, outputsToDownload, outputMetadataStore::getOutputMetadata, Priority.HIGH));
action,
outputsToDownload,
outputMetadataStore::getOutputMetadata,
Priority.HIGH,
Reason.OUTPUTS));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ActionInputMap;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher.Priority;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher.Reason;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact;
import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
Expand Down Expand Up @@ -753,7 +754,11 @@ private void downloadFileIfRemote(PathFragment path) throws IOException {
}
getFromFuture(
inputFetcher.prefetchFiles(
action, ImmutableList.of(input), this::getInputMetadata, Priority.CRITICAL));
action,
ImmutableList.of(input),
this::getInputMetadata,
Priority.CRITICAL,
Reason.INPUTS));
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
throw new IOException(String.format("Received interrupt while fetching file '%s'", path), e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,19 @@ protected ListenableFuture<Void> doDownloadFile(
Path tempPath,
PathFragment execPath,
FileArtifactValue metadata,
Priority priority)
Priority priority,
Reason reason)
throws IOException {
checkArgument(metadata.isRemote(), "Cannot download file that is not a remote file.");
RequestMetadata requestMetadata =
TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "prefetcher", action);
TracingMetadataUtils.buildMetadata(
buildRequestId,
commandId,
switch (reason) {
case INPUTS -> "input";
case OUTPUTS -> "output";
},
action);
RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata);

Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.devtools.build.lib.actions.ActionInputMap;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher.Priority;
import com.google.devtools.build.lib.actions.ActionInputPrefetcher.Reason;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact;
import com.google.devtools.build.lib.actions.CompletionContext;
Expand Down Expand Up @@ -469,7 +470,7 @@ private void downloadArtifact(
ActionUtils.getActionForLookupData(env, derivedArtifact.getGeneratingActionKey());
var future =
actionInputPrefetcher.prefetchFiles(
action, filesToDownload, inputMap::getInputMetadata, Priority.LOW);
action, filesToDownload, inputMap::getInputMetadata, Priority.LOW, Reason.OUTPUTS);
futures.add(future);
}
} else {
Expand All @@ -485,7 +486,7 @@ private void downloadArtifact(
ActionUtils.getActionForLookupData(env, derivedArtifact.getGeneratingActionKey());
var future =
actionInputPrefetcher.prefetchFiles(
action, ImmutableList.of(artifact), inputMap::getInputMetadata, Priority.LOW);
action, ImmutableList.of(artifact), inputMap::getInputMetadata, Priority.LOW, Reason.OUTPUTS);
futures.add(future);
}
}
Expand Down
Loading
Loading