diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnProgressEvent.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnProgressEvent.java index 9ddde03a1e1497..4f4b139a576be5 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnProgressEvent.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnProgressEvent.java @@ -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) { 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 478d30d5b4c44e..bcdf604b2f81bc 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 @@ -157,7 +157,11 @@ private boolean shouldDownloadFile(Path path, FileArtifactValue metadata) { * @param tempPath the temporary path which the input should be written to. */ protected abstract ListenableFuture 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 {} @@ -309,7 +313,8 @@ private Completable prefetchInputTree( return toTransferResult( toCompletable( () -> - doDownloadFile(tempPath, treeFile.getExecPath(), metadata, priority), + doDownloadFile( + reporter, tempPath, treeFile.getExecPath(), metadata, priority), directExecutor())); }); @@ -455,7 +460,11 @@ private Completable downloadFileNoCheckRx( toCompletable( () -> doDownloadFile( - tempPath, finalPath.relativeTo(execRoot), metadata, priority), + reporter, + tempPath, + finalPath.relativeTo(execRoot), + metadata, + priority), directExecutor()) .doOnComplete( () -> { 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 bd482a2d9da823..bcda6374686676 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,6 +14,7 @@ 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; @@ -24,7 +25,10 @@ import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput; +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; @@ -90,7 +94,11 @@ protected boolean canDownloadFile(Path path, FileArtifactValue metadata) { @Override protected ListenableFuture 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 = @@ -98,7 +106,44 @@ protected ListenableFuture doDownloadFile( RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata); Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize()); - return remoteCache.downloadFile(context, tempPath, digest); + + 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(); + } } @Override diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java index f081add8ad9caa..1fe79e16906f0b 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java @@ -248,6 +248,7 @@ private ListenableFuture 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; @@ -255,6 +256,12 @@ public static class DownloadProgressReporter { 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); @@ -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)); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/ProgressStatusListener.java b/src/main/java/com/google/devtools/build/lib/remote/common/ProgressStatusListener.java index df5a8beae54758..55b4b57f7c669f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/ProgressStatusListener.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/ProgressStatusListener.java @@ -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 = diff --git a/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java b/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java index 116e25bb8e6a41..dac8c42ed3b7bc 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java @@ -993,7 +993,8 @@ synchronized boolean hasActivities() { return !(buildCompleted() && bepOpenTransports.isEmpty() && activeActionUploads.get() == 0 - && activeActionDownloads.get() == 0); + && activeActionDownloads.get() == 0 + && runningDownloads.isEmpty()); } /** @@ -1106,7 +1107,8 @@ 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(); @@ -1116,7 +1118,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, 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 fae2d964f9ef04..68232c3918dbe6 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 @@ -173,7 +173,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(); } @@ -190,7 +190,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"); @@ -531,8 +531,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")); @@ -541,7 +541,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 {