Skip to content

Commit 0f812eb

Browse files
coeuvrecopybara-github
coeuvre
authored and
copybara-github
committed
Remote: Display download progress when actions are downloading outputs from remote cache.
Normally, when executing action with remote cache/execution, the UI only display the "remote"/"remote-cache" strategy: ``` [500 / 1000] 500 actions, 3 running [Sched] Executing genrule //:test-1; Executing genrule //:test-2; 2s remote Executing genrule //:test-3; 3s remote ... ``` However, it doesn't tell users what is happening under the hood. #13555 fixed the confusion which the UI display the action is scheduling while it is actually downloading the outputs. With this change, Bazel will display the downloads if action is downloading outputs. e.g. ``` [500 / 1000] 500 actions, 3 running [Sched] Executing genrule //:test-1; 1s remote Executing genrule //:test-2; Downloading 2.out, 20.1 KiB / 100 KiB; 2s remote Executing genrule //:test-3; 3s remote ... ``` Add a generic `ActionProgressEvent` which can be reported within action execution to display detailed execution progress for that action. Closes #13557. PiperOrigin-RevId: 383224334
1 parent ec3d4bd commit 0f812eb

16 files changed

Lines changed: 619 additions & 36 deletions

‎src/main/java/com/google/devtools/build/lib/actions/ActionProgressEvent.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright 2021 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.actions;
15+
16+
import com.google.auto.value.AutoValue;
17+
import com.google.devtools.build.lib.events.ExtendedEventHandler.ProgressLike;
18+
19+
/** Notifications for the progress of an in-flight action. */
20+
@AutoValue
21+
public abstract class ActionProgressEvent implements ProgressLike {
22+
23+
public static ActionProgressEvent create(
24+
ActionExecutionMetadata action, String progressId, String progress, boolean finished) {
25+
return new AutoValue_ActionProgressEvent(action, progressId, progress, finished);
26+
}
27+
28+
/** Gets the metadata associated with the action being scheduled. */
29+
public abstract ActionExecutionMetadata action();
30+
31+
/** The id that uniquely determines the progress among all progress events within an action. */
32+
public abstract String progressId();
33+
34+
/** Human readable description of the progress. */
35+
public abstract String progress();
36+
37+
/** Whether the download progress reported about is finished already. */
38+
public abstract boolean finished();
39+
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,6 @@ public void report(ProgressStatus progress) {
328328
return;
329329
}
330330

331-
// TODO(ulfjack): We should report more details to the UI.
332331
ExtendedEventHandler eventHandler = actionExecutionContext.getEventHandler();
333332
progress.postTo(eventHandler, action);
334333
}

‎src/main/java/com/google/devtools/build/lib/exec/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,7 @@ java_library(
269269
srcs = [
270270
"SpawnCheckingCacheEvent.java",
271271
"SpawnExecutingEvent.java",
272+
"SpawnProgressEvent.java",
272273
"SpawnRunner.java",
273274
"SpawnSchedulingEvent.java",
274275
],

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright 2021 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.exec;
15+
16+
import com.google.auto.value.AutoValue;
17+
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
18+
import com.google.devtools.build.lib.actions.ActionProgressEvent;
19+
import com.google.devtools.build.lib.events.ExtendedEventHandler;
20+
import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
21+
22+
/** The {@link SpawnRunner} is making some progress. */
23+
@AutoValue
24+
public abstract class SpawnProgressEvent implements ProgressStatus {
25+
26+
public static SpawnProgressEvent create(String resourceId, String progress, boolean finished) {
27+
return new AutoValue_SpawnProgressEvent(resourceId, progress, finished);
28+
}
29+
30+
/** The id that uniquely determines the progress among all progress events for this spawn. */
31+
abstract String progressId();
32+
33+
/** Human readable description of the progress. */
34+
abstract String progress();
35+
36+
/** Whether the progress reported about is finished already. */
37+
abstract boolean finished();
38+
39+
@Override
40+
public void postTo(ExtendedEventHandler eventHandler, ActionExecutionMetadata action) {
41+
eventHandler.post(ActionProgressEvent.create(action, progressId(), progress(), finished()));
42+
}
43+
}

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

Lines changed: 122 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515

1616
import static com.google.common.util.concurrent.Futures.immediateFuture;
1717
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
18+
import static com.google.devtools.build.lib.remote.common.ProgressStatusListener.NO_ACTION;
19+
import static com.google.devtools.build.lib.remote.util.Utils.bytesCountToDisplayString;
1820
import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture;
1921

2022
import build.bazel.remote.execution.v2.Action;
@@ -50,6 +52,7 @@
5052
import com.google.devtools.build.lib.actions.UserExecException;
5153
import com.google.devtools.build.lib.actions.cache.MetadataInjector;
5254
import com.google.devtools.build.lib.concurrent.ThreadSafety;
55+
import com.google.devtools.build.lib.exec.SpawnProgressEvent;
5356
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
5457
import com.google.devtools.build.lib.profiler.Profiler;
5558
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -58,6 +61,7 @@
5861
import com.google.devtools.build.lib.remote.RemoteCache.ActionResultMetadata.SymlinkMetadata;
5962
import com.google.devtools.build.lib.remote.common.LazyFileOutputStream;
6063
import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException;
64+
import com.google.devtools.build.lib.remote.common.ProgressStatusListener;
6165
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
6266
import com.google.devtools.build.lib.remote.common.RemoteActionFileArtifactValue;
6367
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
@@ -91,6 +95,9 @@
9195
import java.util.List;
9296
import java.util.Map;
9397
import java.util.Map.Entry;
98+
import java.util.concurrent.atomic.AtomicLong;
99+
import java.util.regex.Matcher;
100+
import java.util.regex.Pattern;
94101
import java.util.stream.Collectors;
95102
import java.util.stream.Stream;
96103
import javax.annotation.Nullable;
@@ -314,6 +321,50 @@ private static Path toTmpDownloadPath(Path actualPath) {
314321
return actualPath.getParentDirectory().getRelative(actualPath.getBaseName() + ".tmp");
315322
}
316323

324+
static class DownloadProgressReporter {
325+
private static final Pattern PATTERN = Pattern.compile("^bazel-out/[^/]+/[^/]+/");
326+
private final ProgressStatusListener listener;
327+
private final String id;
328+
private final String file;
329+
private final String totalSize;
330+
private final AtomicLong downloadedBytes = new AtomicLong(0);
331+
332+
DownloadProgressReporter(ProgressStatusListener listener, String file, long totalSize) {
333+
this.listener = listener;
334+
this.id = file;
335+
this.totalSize = bytesCountToDisplayString(totalSize);
336+
337+
Matcher matcher = PATTERN.matcher(file);
338+
this.file = matcher.replaceFirst("");
339+
}
340+
341+
void started() {
342+
reportProgress(false, false);
343+
}
344+
345+
void downloadedBytes(int count) {
346+
downloadedBytes.addAndGet(count);
347+
reportProgress(true, false);
348+
}
349+
350+
void finished() {
351+
reportProgress(true, true);
352+
}
353+
354+
private void reportProgress(boolean includeBytes, boolean finished) {
355+
String progress;
356+
if (includeBytes) {
357+
progress =
358+
String.format(
359+
"Downloading %s, %s / %s",
360+
file, bytesCountToDisplayString(downloadedBytes.get()), totalSize);
361+
} else {
362+
progress = String.format("Downloading %s", file);
363+
}
364+
listener.onProgressStatus(SpawnProgressEvent.create(id, progress, finished));
365+
}
366+
}
367+
317368
/**
318369
* Download the output files and directory trees of a remotely executed action to the local
319370
* machine, as well stdin / stdout to the given files.
@@ -330,7 +381,8 @@ public void download(
330381
RemotePathResolver remotePathResolver,
331382
ActionResult result,
332383
FileOutErr origOutErr,
333-
OutputFilesLocker outputFilesLocker)
384+
OutputFilesLocker outputFilesLocker,
385+
ProgressStatusListener progressStatusListener)
334386
throws ExecException, IOException, InterruptedException {
335387
ActionResultMetadata metadata = parseActionResultMetadata(context, remotePathResolver, result);
336388

@@ -347,7 +399,11 @@ public void download(
347399
context,
348400
remotePathResolver.localPathToOutputPath(file.path()),
349401
toTmpDownloadPath(file.path()),
350-
file.digest());
402+
file.digest(),
403+
new DownloadProgressReporter(
404+
progressStatusListener,
405+
remotePathResolver.localPathToOutputPath(file.path()),
406+
file.digest().getSizeBytes()));
351407
return Futures.transform(download, (d) -> file, directExecutor());
352408
} catch (IOException e) {
353409
return Futures.<FileMetadata>immediateFailedFuture(e);
@@ -499,10 +555,14 @@ private void createSymlinks(Iterable<SymlinkMetadata> symlinks) throws IOExcepti
499555
}
500556

501557
public ListenableFuture<Void> downloadFile(
502-
RemoteActionExecutionContext context, String outputPath, Path localPath, Digest digest)
558+
RemoteActionExecutionContext context,
559+
String outputPath,
560+
Path localPath,
561+
Digest digest,
562+
DownloadProgressReporter reporter)
503563
throws IOException {
504564
SettableFuture<Void> outerF = SettableFuture.create();
505-
ListenableFuture<Void> f = downloadFile(context, localPath, digest);
565+
ListenableFuture<Void> f = downloadFile(context, localPath, digest, reporter);
506566
Futures.addCallback(
507567
f,
508568
new FutureCallback<Void>() {
@@ -529,6 +589,16 @@ public void onFailure(Throwable throwable) {
529589
/** Downloads a file (that is not a directory). The content is fetched from the digest. */
530590
public ListenableFuture<Void> downloadFile(
531591
RemoteActionExecutionContext context, Path path, Digest digest) throws IOException {
592+
return downloadFile(context, path, digest, new DownloadProgressReporter(NO_ACTION, "", 0));
593+
}
594+
595+
/** Downloads a file (that is not a directory). The content is fetched from the digest. */
596+
public ListenableFuture<Void> downloadFile(
597+
RemoteActionExecutionContext context,
598+
Path path,
599+
Digest digest,
600+
DownloadProgressReporter reporter)
601+
throws IOException {
532602
Preconditions.checkNotNull(path.getParentDirectory()).createDirectoryAndParents();
533603
if (digest.getSizeBytes() == 0) {
534604
// Handle empty file locally.
@@ -549,7 +619,9 @@ public ListenableFuture<Void> downloadFile(
549619
return COMPLETED_SUCCESS;
550620
}
551621

552-
OutputStream out = new LazyFileOutputStream(path);
622+
reporter.started();
623+
OutputStream out = new ReportingOutputStream(new LazyFileOutputStream(path), reporter);
624+
553625
SettableFuture<Void> outerF = SettableFuture.create();
554626
ListenableFuture<Void> f = cacheProtocol.downloadBlob(context, digest, out);
555627
Futures.addCallback(
@@ -560,6 +632,7 @@ public void onSuccess(Void result) {
560632
try {
561633
out.close();
562634
outerF.set(null);
635+
reporter.finished();
563636
} catch (IOException e) {
564637
outerF.setException(e);
565638
} catch (RuntimeException e) {
@@ -572,6 +645,7 @@ public void onSuccess(Void result) {
572645
public void onFailure(Throwable t) {
573646
try {
574647
out.close();
648+
reporter.finished();
575649
} catch (IOException e) {
576650
if (t != e) {
577651
t.addSuppressed(e);
@@ -1100,6 +1174,49 @@ private static FailureDetail createFailureDetail(String message, Code detailedCo
11001174
.build();
11011175
}
11021176

1177+
/**
1178+
* An {@link OutputStream} that reports all the write operations with {@link
1179+
* DownloadProgressReporter}.
1180+
*/
1181+
private static class ReportingOutputStream extends OutputStream {
1182+
1183+
private final OutputStream out;
1184+
private final DownloadProgressReporter reporter;
1185+
1186+
ReportingOutputStream(OutputStream out, DownloadProgressReporter reporter) {
1187+
this.out = out;
1188+
this.reporter = reporter;
1189+
}
1190+
1191+
@Override
1192+
public void write(byte[] b) throws IOException {
1193+
out.write(b);
1194+
reporter.downloadedBytes(b.length);
1195+
}
1196+
1197+
@Override
1198+
public void write(byte[] b, int off, int len) throws IOException {
1199+
out.write(b, off, len);
1200+
reporter.downloadedBytes(len);
1201+
}
1202+
1203+
@Override
1204+
public void write(int b) throws IOException {
1205+
out.write(b);
1206+
reporter.downloadedBytes(1);
1207+
}
1208+
1209+
@Override
1210+
public void flush() throws IOException {
1211+
out.flush();
1212+
}
1213+
1214+
@Override
1215+
public void close() throws IOException {
1216+
out.close();
1217+
}
1218+
}
1219+
11031220
/** In-memory representation of action result metadata. */
11041221
static class ActionResultMetadata {
11051222

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,8 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
386386
remotePathResolver,
387387
result.actionResult,
388388
action.spawnExecutionContext.getFileOutErr(),
389-
action.spawnExecutionContext::lockOutputFiles);
389+
action.spawnExecutionContext::lockOutputFiles,
390+
action.spawnExecutionContext::report);
390391
} else {
391392
PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.spawn);
392393
inMemoryOutput =

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright 2021 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.remote.common;
15+
16+
import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus;
17+
18+
/** An interface that is used to receive {@link ProgressStatus} updates during spawn execution. */
19+
@FunctionalInterface
20+
public interface ProgressStatusListener {
21+
22+
void onProgressStatus(ProgressStatus progress);
23+
24+
/** A {@link ProgressStatusListener} that does nothing. */
25+
ProgressStatusListener NO_ACTION =
26+
progress -> {
27+
// Intentionally left empty
28+
};
29+
}

0 commit comments

Comments
 (0)