Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Report background download for BwoB #17604

Closed
wants to merge 2 commits into from
Closed
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 @@ -28,13 +28,13 @@ public static SpawnProgressEvent create(String resourceId, String progress, bool
}

/** The id that uniquely determines the progress among all progress events for this spawn. */
abstract String progressId();
public abstract String progressId();

/** Human readable description of the progress. */
abstract String progress();
public abstract String progress();

/** Whether the progress reported about is finished already. */
abstract boolean finished();
public abstract boolean finished();

@Override
public void postTo(ExtendedEventHandler eventHandler, ActionExecutionMetadata action) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,11 @@ private boolean shouldDownloadFile(Path path, FileArtifactValue metadata) {
* @param tempPath the temporary path which the input should be written to.
*/
protected abstract ListenableFuture<Void> doDownloadFile(
Path tempPath, PathFragment execPath, FileArtifactValue metadata, Priority priority)
Reporter reporter,
Path tempPath,
PathFragment execPath,
FileArtifactValue metadata,
Priority priority)
throws IOException;

protected void prefetchVirtualActionInput(VirtualActionInput input) throws IOException {}
Expand Down Expand Up @@ -321,7 +325,8 @@ private Completable prefetchInputTree(
return toTransferResult(
toCompletable(
() ->
doDownloadFile(tempPath, treeFile.getExecPath(), metadata, priority),
doDownloadFile(
reporter, tempPath, treeFile.getExecPath(), metadata, priority),
directExecutor()));
});

Expand Down Expand Up @@ -467,7 +472,11 @@ private Completable downloadFileNoCheckRx(
toCompletable(
() ->
doDownloadFile(
tempPath, finalPath.relativeTo(execRoot), metadata, priority),
reporter,
tempPath,
finalPath.relativeTo(execRoot),
metadata,
priority),
directExecutor())
.doOnComplete(
() -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,10 @@
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;
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.BulkTransferException;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.util.DigestUtil;
Expand Down Expand Up @@ -87,15 +90,50 @@ protected boolean canDownloadFile(Path path, FileArtifactValue metadata) {

@Override
protected ListenableFuture<Void> doDownloadFile(
Path tempPath, PathFragment execPath, FileArtifactValue metadata, Priority priority)
Reporter reporter,
Path tempPath,
PathFragment execPath,
FileArtifactValue metadata,
Priority priority)
throws IOException {
checkArgument(metadata.isRemote(), "Cannot download file that is not a remote file.");
RequestMetadata requestMetadata =
TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "prefetcher", null);
RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata);

Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize());
return remoteCache.downloadFile(context, tempPath, digest);
return remoteCache.downloadFile(
context,
tempPath,
digest,
new DownloadProgressReporter(
/* includeFile= */ false,
progress -> reporter.post(new DownloadProgress(progress)),
execPath.toString(),
metadata.getSize()));
}

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();
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -248,13 +248,20 @@ private ListenableFuture<Void> downloadBlob(
/** A reporter that reports download progresses. */
public static class DownloadProgressReporter {
private static final Pattern PATTERN = Pattern.compile("^bazel-out/[^/]+/[^/]+/");
private final boolean includeFile;
private final ProgressStatusListener listener;
private final String id;
private final String file;
private final String totalSize;
private final AtomicLong downloadedBytes = new AtomicLong(0);

public DownloadProgressReporter(ProgressStatusListener listener, String file, long totalSize) {
this(/* includeFile= */ true, listener, file, totalSize);
}

public DownloadProgressReporter(
boolean includeFile, ProgressStatusListener listener, String file, long totalSize) {
this.includeFile = includeFile;
this.listener = listener;
this.id = file;
this.totalSize = bytesCountToDisplayString(totalSize);
Expand All @@ -279,12 +286,21 @@ void finished() {
private void reportProgress(boolean includeBytes, boolean finished) {
String progress;
if (includeBytes) {
progress =
String.format(
"Downloading %s, %s / %s",
file, bytesCountToDisplayString(downloadedBytes.get()), totalSize);
if (includeFile) {
progress =
String.format(
"Downloading %s, %s / %s",
file, bytesCountToDisplayString(downloadedBytes.get()), totalSize);
} else {
progress =
String.format("%s / %s", bytesCountToDisplayString(downloadedBytes.get()), totalSize);
}
} else {
progress = String.format("Downloading %s", file);
if (includeFile) {
progress = String.format("Downloading %s", file);
} else {
progress = "";
}
}
listener.onProgressStatus(SpawnProgressEvent.create(id, progress, finished));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@
// limitations under the License.
package com.google.devtools.build.lib.remote.common;

import com.google.devtools.build.lib.exec.SpawnProgressEvent;
import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;

/** An interface that is used to receive {@link ProgressStatus} updates during spawn execution. */
@FunctionalInterface
public interface ProgressStatusListener {

void onProgressStatus(ProgressStatus progress);
void onProgressStatus(SpawnProgressEvent progress);

/** A {@link ProgressStatusListener} that does nothing. */
ProgressStatusListener NO_ACTION =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,8 @@ synchronized boolean hasActivities() {
return !(buildCompleted()
&& bepOpenTransports.isEmpty()
&& activeActionUploads.get() == 0
&& activeActionDownloads.get() == 0);
&& activeActionDownloads.get() == 0
&& runningDownloads.isEmpty());
}

/**
Expand Down Expand Up @@ -1106,7 +1107,7 @@ private void reportOnOneDownload(
terminalWriter.append(url + postfix);
}

protected void reportOnDownloads(AnsiTerminalWriter terminalWriter) throws IOException {
protected void reportOnDownloads(PositionAwareAnsiTerminalWriter terminalWriter) throws IOException {
int count = 0;
long nanoTime = clock.nanoTime();
int downloadCount = runningDownloads.size();
Expand All @@ -1116,7 +1117,10 @@ protected void reportOnDownloads(AnsiTerminalWriter terminalWriter) throws IOExc
break;
}
count++;
terminalWriter.newline().append(FETCH_PREFIX);
if (terminalWriter.getPosition() != 0) {
terminalWriter.newline();
}
terminalWriter.append(FETCH_PREFIX);
reportOnOneDownload(
url,
nanoTime,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public void prefetchFiles_fileExists_doNotDownload()

wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider));

verify(prefetcher, never()).doDownloadFile(any(), any(), any(), any());
verify(prefetcher, never()).doDownloadFile(any(), any(), any(), any(), any());
assertThat(prefetcher.downloadedFiles()).containsExactly(a.getPath());
assertThat(prefetcher.downloadsInProgress()).isEmpty();
}
Expand All @@ -189,7 +189,7 @@ public void prefetchFiles_fileExistsButContentMismatches_download()

wait(prefetcher.prefetchFiles(metadata.keySet(), metadataProvider));

verify(prefetcher).doDownloadFile(any(), eq(a.getExecPath()), any(), any());
verify(prefetcher).doDownloadFile(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");
Expand Down Expand Up @@ -530,8 +530,8 @@ protected static void mockDownload(
throws IOException {
doAnswer(
invocation -> {
Path path = invocation.getArgument(0);
FileArtifactValue metadata = invocation.getArgument(2);
Path path = invocation.getArgument(1);
FileArtifactValue metadata = invocation.getArgument(3);
byte[] content = cas.get(HashCode.fromBytes(metadata.getDigest()));
if (content == null) {
return Futures.immediateFailedFuture(new IOException("Not found"));
Expand All @@ -540,7 +540,7 @@ protected static void mockDownload(
return resultSupplier.get();
})
.when(prefetcher)
.doDownloadFile(any(), any(), any(), any());
.doDownloadFile(any(), any(), any(), any(), any());
}

private void assertReadableNonWritableAndExecutable(Path path) throws IOException {
Expand Down