Skip to content

Commit 034a281

Browse files
coeuvrecoeuvre
authored
Report background download for BwoB (bazelbuild#17619)
When building without the bytes, downloads can happen when Bazel need to fetch inputs for local actions, download outputs of toplevel targets, and etc. The downloads happen silently at the background. This PR makes output downloads of toplevel targets visible to the UI. For downloads during the build, the UI looks like: ``` [2 / 5] 2 actions, 1 running [Prepa] Executing genrule //:foo Executing genrule //:bar; 2s Fetching bazel-out/darwin-fastbuild/bin/baz; 50.8 MiB / 1.0 GiB; 4s ``` For downloads after build is completed, the UI looks like: ``` INFO: 2 processes: 1 remote cache hit, 1 internal. INFO: Build completed successfully, 2 total actions Fetching bazel-out/darwin-fastbuild/bin/baz; 48.8 MiB / 1.0 GiB ``` Fixes bazelbuild#17417. Closes bazelbuild#17604. PiperOrigin-RevId: 512934756 Change-Id: I502489420ab9b84efb74ceabbe3dd4cb4a7a62e2
1 parent 938e348 commit 034a281

File tree

7 files changed

+98
-22
lines changed

7 files changed

+98
-22
lines changed

‎src/main/java/com/google/devtools/build/lib/exec/SpawnProgressEvent.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ public static SpawnProgressEvent create(String resourceId, String progress, bool
2828
}
2929

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

3333
/** Human readable description of the progress. */
34-
abstract String progress();
34+
public abstract String progress();
3535

3636
/** Whether the progress reported about is finished already. */
37-
abstract boolean finished();
37+
public abstract boolean finished();
3838

3939
@Override
4040
public void postTo(ExtendedEventHandler eventHandler, ActionExecutionMetadata action) {

‎src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,11 @@ private boolean shouldDownloadFile(Path path, FileArtifactValue metadata) {
157157
* @param tempPath the temporary path which the input should be written to.
158158
*/
159159
protected abstract ListenableFuture<Void> doDownloadFile(
160-
Path tempPath, PathFragment execPath, FileArtifactValue metadata, Priority priority)
160+
Reporter reporter,
161+
Path tempPath,
162+
PathFragment execPath,
163+
FileArtifactValue metadata,
164+
Priority priority)
161165
throws IOException;
162166

163167
protected void prefetchVirtualActionInput(VirtualActionInput input) throws IOException {}
@@ -309,7 +313,8 @@ private Completable prefetchInputTree(
309313
return toTransferResult(
310314
toCompletable(
311315
() ->
312-
doDownloadFile(tempPath, treeFile.getExecPath(), metadata, priority),
316+
doDownloadFile(
317+
reporter, tempPath, treeFile.getExecPath(), metadata, priority),
313318
directExecutor()));
314319
});
315320

@@ -455,7 +460,11 @@ private Completable downloadFileNoCheckRx(
455460
toCompletable(
456461
() ->
457462
doDownloadFile(
458-
tempPath, finalPath.relativeTo(execRoot), metadata, priority),
463+
reporter,
464+
tempPath,
465+
finalPath.relativeTo(execRoot),
466+
metadata,
467+
priority),
459468
directExecutor())
460469
.doOnComplete(
461470
() -> {

‎src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java‎

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package com.google.devtools.build.lib.remote;
1515

1616
import static com.google.common.base.Preconditions.checkArgument;
17+
import static com.google.devtools.build.lib.remote.common.ProgressStatusListener.NO_ACTION;
1718

1819
import build.bazel.remote.execution.v2.Digest;
1920
import build.bazel.remote.execution.v2.RequestMetadata;
@@ -24,7 +25,10 @@
2425
import com.google.devtools.build.lib.actions.FileArtifactValue;
2526
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
2627
import com.google.devtools.build.lib.actions.cache.VirtualActionInput.EmptyActionInput;
28+
import com.google.devtools.build.lib.events.ExtendedEventHandler.FetchProgress;
2729
import com.google.devtools.build.lib.events.Reporter;
30+
import com.google.devtools.build.lib.exec.SpawnProgressEvent;
31+
import com.google.devtools.build.lib.remote.RemoteCache.DownloadProgressReporter;
2832
import com.google.devtools.build.lib.remote.common.BulkTransferException;
2933
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
3034
import com.google.devtools.build.lib.remote.util.DigestUtil;
@@ -90,15 +94,56 @@ protected boolean canDownloadFile(Path path, FileArtifactValue metadata) {
9094

9195
@Override
9296
protected ListenableFuture<Void> doDownloadFile(
93-
Path tempPath, PathFragment execPath, FileArtifactValue metadata, Priority priority)
97+
Reporter reporter,
98+
Path tempPath,
99+
PathFragment execPath,
100+
FileArtifactValue metadata,
101+
Priority priority)
94102
throws IOException {
95103
checkArgument(metadata.isRemote(), "Cannot download file that is not a remote file.");
96104
RequestMetadata requestMetadata =
97105
TracingMetadataUtils.buildMetadata(buildRequestId, commandId, metadata.getActionId(), null);
98106
RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata);
99107

100108
Digest digest = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize());
101-
return remoteCache.downloadFile(context, tempPath, digest);
109+
110+
DownloadProgressReporter downloadProgressReporter;
111+
if (priority == Priority.LOW) {
112+
// Only report download progress for toplevel outputs
113+
downloadProgressReporter =
114+
new DownloadProgressReporter(
115+
/* includeFile= */ false,
116+
progress -> reporter.post(new DownloadProgress(progress)),
117+
execPath.toString(),
118+
metadata.getSize());
119+
} else {
120+
downloadProgressReporter = new DownloadProgressReporter(NO_ACTION, "", 0);
121+
}
122+
123+
return remoteCache.downloadFile(context, tempPath, digest, downloadProgressReporter);
124+
}
125+
126+
public static class DownloadProgress implements FetchProgress {
127+
private final SpawnProgressEvent progress;
128+
129+
public DownloadProgress(SpawnProgressEvent progress) {
130+
this.progress = progress;
131+
}
132+
133+
@Override
134+
public String getResourceIdentifier() {
135+
return progress.progressId();
136+
}
137+
138+
@Override
139+
public String getProgress() {
140+
return progress.progress();
141+
}
142+
143+
@Override
144+
public boolean isFinished() {
145+
return progress.finished();
146+
}
102147
}
103148

104149
@Override

‎src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -248,13 +248,20 @@ private ListenableFuture<Void> downloadBlob(
248248
/** A reporter that reports download progresses. */
249249
public static class DownloadProgressReporter {
250250
private static final Pattern PATTERN = Pattern.compile("^bazel-out/[^/]+/[^/]+/");
251+
private final boolean includeFile;
251252
private final ProgressStatusListener listener;
252253
private final String id;
253254
private final String file;
254255
private final String totalSize;
255256
private final AtomicLong downloadedBytes = new AtomicLong(0);
256257

257258
public DownloadProgressReporter(ProgressStatusListener listener, String file, long totalSize) {
259+
this(/* includeFile= */ true, listener, file, totalSize);
260+
}
261+
262+
public DownloadProgressReporter(
263+
boolean includeFile, ProgressStatusListener listener, String file, long totalSize) {
264+
this.includeFile = includeFile;
258265
this.listener = listener;
259266
this.id = file;
260267
this.totalSize = bytesCountToDisplayString(totalSize);
@@ -279,12 +286,21 @@ void finished() {
279286
private void reportProgress(boolean includeBytes, boolean finished) {
280287
String progress;
281288
if (includeBytes) {
282-
progress =
283-
String.format(
284-
"Downloading %s, %s / %s",
285-
file, bytesCountToDisplayString(downloadedBytes.get()), totalSize);
289+
if (includeFile) {
290+
progress =
291+
String.format(
292+
"Downloading %s, %s / %s",
293+
file, bytesCountToDisplayString(downloadedBytes.get()), totalSize);
294+
} else {
295+
progress =
296+
String.format("%s / %s", bytesCountToDisplayString(downloadedBytes.get()), totalSize);
297+
}
286298
} else {
287-
progress = String.format("Downloading %s", file);
299+
if (includeFile) {
300+
progress = String.format("Downloading %s", file);
301+
} else {
302+
progress = "";
303+
}
288304
}
289305
listener.onProgressStatus(SpawnProgressEvent.create(id, progress, finished));
290306
}

‎src/main/java/com/google/devtools/build/lib/remote/common/ProgressStatusListener.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,14 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.lib.remote.common;
1515

16+
import com.google.devtools.build.lib.exec.SpawnProgressEvent;
1617
import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
1718

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

22-
void onProgressStatus(ProgressStatus progress);
23+
void onProgressStatus(SpawnProgressEvent progress);
2324

2425
/** A {@link ProgressStatusListener} that does nothing. */
2526
ProgressStatusListener NO_ACTION =

‎src/main/java/com/google/devtools/build/lib/runtime/UiStateTracker.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,8 @@ synchronized boolean hasActivities() {
993993
return !(buildCompleted()
994994
&& bepOpenTransports.isEmpty()
995995
&& activeActionUploads.get() == 0
996-
&& activeActionDownloads.get() == 0);
996+
&& activeActionDownloads.get() == 0
997+
&& runningDownloads.isEmpty());
997998
}
998999

9991000
/**
@@ -1106,7 +1107,8 @@ private void reportOnOneDownload(
11061107
terminalWriter.append(url + postfix);
11071108
}
11081109

1109-
protected void reportOnDownloads(AnsiTerminalWriter terminalWriter) throws IOException {
1110+
protected void reportOnDownloads(PositionAwareAnsiTerminalWriter terminalWriter)
1111+
throws IOException {
11101112
int count = 0;
11111113
long nanoTime = clock.nanoTime();
11121114
int downloadCount = runningDownloads.size();
@@ -1116,7 +1118,10 @@ protected void reportOnDownloads(AnsiTerminalWriter terminalWriter) throws IOExc
11161118
break;
11171119
}
11181120
count++;
1119-
terminalWriter.newline().append(FETCH_PREFIX);
1121+
if (terminalWriter.getPosition() != 0) {
1122+
terminalWriter.newline();
1123+
}
1124+
terminalWriter.append(FETCH_PREFIX);
11201125
reportOnOneDownload(
11211126
url,
11221127
nanoTime,

‎src/test/java/com/google/devtools/build/lib/remote/ActionInputPrefetcherTestBase.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public void prefetchFiles_fileExists_doNotDownload()
173173

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

176-
verify(prefetcher, never()).doDownloadFile(any(), any(), any(), any());
176+
verify(prefetcher, never()).doDownloadFile(any(), any(), any(), any(), any());
177177
assertThat(prefetcher.downloadedFiles()).containsExactly(a.getPath());
178178
assertThat(prefetcher.downloadsInProgress()).isEmpty();
179179
}
@@ -190,7 +190,7 @@ public void prefetchFiles_fileExistsButContentMismatches_download()
190190

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

193-
verify(prefetcher).doDownloadFile(any(), eq(a.getExecPath()), any(), any());
193+
verify(prefetcher).doDownloadFile(any(), any(), eq(a.getExecPath()), any(), any());
194194
assertThat(prefetcher.downloadedFiles()).containsExactly(a.getPath());
195195
assertThat(prefetcher.downloadsInProgress()).isEmpty();
196196
assertThat(FileSystemUtils.readContent(a.getPath(), UTF_8)).isEqualTo("hello world remote");
@@ -531,8 +531,8 @@ protected static void mockDownload(
531531
throws IOException {
532532
doAnswer(
533533
invocation -> {
534-
Path path = invocation.getArgument(0);
535-
FileArtifactValue metadata = invocation.getArgument(2);
534+
Path path = invocation.getArgument(1);
535+
FileArtifactValue metadata = invocation.getArgument(3);
536536
byte[] content = cas.get(HashCode.fromBytes(metadata.getDigest()));
537537
if (content == null) {
538538
return Futures.immediateFailedFuture(new IOException("Not found"));
@@ -541,7 +541,7 @@ protected static void mockDownload(
541541
return resultSupplier.get();
542542
})
543543
.when(prefetcher)
544-
.doDownloadFile(any(), any(), any(), any());
544+
.doDownloadFile(any(), any(), any(), any(), any());
545545
}
546546

547547
private void assertReadableNonWritableAndExecutable(Path path) throws IOException {

0 commit comments

Comments
 (0)