Skip to content

Commit 92ec798

Browse files
larsrc-googlelarsrc-google
committed
Revert "Remote: Display download progress when actions are downloading outputs from remote cache."
This reverts commit 306527a.
1 parent 861c3ca commit 92ec798

File tree

16 files changed

+36
-619
lines changed

16 files changed

+36
-619
lines changed

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

Lines changed: 0 additions & 39 deletions
This file was deleted.

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ public void report(ProgressStatus progress) {
318318
return;
319319
}
320320

321+
// TODO(ulfjack): We should report more details to the UI.
321322
ExtendedEventHandler eventHandler = actionExecutionContext.getEventHandler();
322323
progress.postTo(eventHandler, action);
323324
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,6 @@ java_library(
267267
srcs = [
268268
"SpawnCheckingCacheEvent.java",
269269
"SpawnExecutingEvent.java",
270-
"SpawnProgressEvent.java",
271270
"SpawnRunner.java",
272271
"SpawnSchedulingEvent.java",
273272
],

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

Lines changed: 0 additions & 43 deletions
This file was deleted.

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

Lines changed: 5 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
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;
2018
import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture;
2119

2220
import build.bazel.remote.execution.v2.Action;
@@ -52,7 +50,6 @@
5250
import com.google.devtools.build.lib.actions.UserExecException;
5351
import com.google.devtools.build.lib.actions.cache.MetadataInjector;
5452
import com.google.devtools.build.lib.concurrent.ThreadSafety;
55-
import com.google.devtools.build.lib.exec.SpawnProgressEvent;
5653
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
5754
import com.google.devtools.build.lib.profiler.Profiler;
5855
import com.google.devtools.build.lib.profiler.SilentCloseable;
@@ -61,7 +58,6 @@
6158
import com.google.devtools.build.lib.remote.RemoteCache.ActionResultMetadata.SymlinkMetadata;
6259
import com.google.devtools.build.lib.remote.common.LazyFileOutputStream;
6360
import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException;
64-
import com.google.devtools.build.lib.remote.common.ProgressStatusListener;
6561
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
6662
import com.google.devtools.build.lib.remote.common.RemoteActionFileArtifactValue;
6763
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
@@ -95,9 +91,6 @@
9591
import java.util.List;
9692
import java.util.Map;
9793
import java.util.Map.Entry;
98-
import java.util.concurrent.atomic.AtomicLong;
99-
import java.util.regex.Matcher;
100-
import java.util.regex.Pattern;
10194
import java.util.stream.Collectors;
10295
import java.util.stream.Stream;
10396
import javax.annotation.Nullable;
@@ -362,50 +355,6 @@ private static Path toTmpDownloadPath(Path actualPath) {
362355
return actualPath.getParentDirectory().getRelative(actualPath.getBaseName() + ".tmp");
363356
}
364357

365-
static class DownloadProgressReporter {
366-
private static final Pattern PATTERN = Pattern.compile("^bazel-out/[^/]+/[^/]+/");
367-
private final ProgressStatusListener listener;
368-
private final String id;
369-
private final String file;
370-
private final String totalSize;
371-
private final AtomicLong downloadedBytes = new AtomicLong(0);
372-
373-
DownloadProgressReporter(ProgressStatusListener listener, String file, long totalSize) {
374-
this.listener = listener;
375-
this.id = file;
376-
this.totalSize = bytesCountToDisplayString(totalSize);
377-
378-
Matcher matcher = PATTERN.matcher(file);
379-
this.file = matcher.replaceFirst("");
380-
}
381-
382-
void started() {
383-
reportProgress(false, false);
384-
}
385-
386-
void downloadedBytes(int count) {
387-
downloadedBytes.addAndGet(count);
388-
reportProgress(true, false);
389-
}
390-
391-
void finished() {
392-
reportProgress(true, true);
393-
}
394-
395-
private void reportProgress(boolean includeBytes, boolean finished) {
396-
String progress;
397-
if (includeBytes) {
398-
progress =
399-
String.format(
400-
"Downloading %s, %s / %s",
401-
file, bytesCountToDisplayString(downloadedBytes.get()), totalSize);
402-
} else {
403-
progress = String.format("Downloading %s", file);
404-
}
405-
listener.onProgressStatus(SpawnProgressEvent.create(id, progress, finished));
406-
}
407-
}
408-
409358
/**
410359
* Download the output files and directory trees of a remotely executed action to the local
411360
* machine, as well stdin / stdout to the given files.
@@ -422,8 +371,7 @@ public void download(
422371
RemotePathResolver remotePathResolver,
423372
ActionResult result,
424373
FileOutErr origOutErr,
425-
OutputFilesLocker outputFilesLocker,
426-
ProgressStatusListener progressStatusListener)
374+
OutputFilesLocker outputFilesLocker)
427375
throws ExecException, IOException, InterruptedException {
428376
ActionResultMetadata metadata = parseActionResultMetadata(context, remotePathResolver, result);
429377

@@ -440,11 +388,7 @@ public void download(
440388
context,
441389
remotePathResolver.localPathToOutputPath(file.path()),
442390
toTmpDownloadPath(file.path()),
443-
file.digest(),
444-
new DownloadProgressReporter(
445-
progressStatusListener,
446-
remotePathResolver.localPathToOutputPath(file.path()),
447-
file.digest().getSizeBytes()));
391+
file.digest());
448392
return Futures.transform(download, (d) -> file, directExecutor());
449393
} catch (IOException e) {
450394
return Futures.<FileMetadata>immediateFailedFuture(e);
@@ -596,14 +540,10 @@ private void createSymlinks(Iterable<SymlinkMetadata> symlinks) throws IOExcepti
596540
}
597541

598542
public ListenableFuture<Void> downloadFile(
599-
RemoteActionExecutionContext context,
600-
String outputPath,
601-
Path localPath,
602-
Digest digest,
603-
DownloadProgressReporter reporter)
543+
RemoteActionExecutionContext context, String outputPath, Path localPath, Digest digest)
604544
throws IOException {
605545
SettableFuture<Void> outerF = SettableFuture.create();
606-
ListenableFuture<Void> f = downloadFile(context, localPath, digest, reporter);
546+
ListenableFuture<Void> f = downloadFile(context, localPath, digest);
607547
Futures.addCallback(
608548
f,
609549
new FutureCallback<Void>() {
@@ -630,16 +570,6 @@ public void onFailure(Throwable throwable) {
630570
/** Downloads a file (that is not a directory). The content is fetched from the digest. */
631571
public ListenableFuture<Void> downloadFile(
632572
RemoteActionExecutionContext context, Path path, Digest digest) throws IOException {
633-
return downloadFile(context, path, digest, new DownloadProgressReporter(NO_ACTION, "", 0));
634-
}
635-
636-
/** Downloads a file (that is not a directory). The content is fetched from the digest. */
637-
public ListenableFuture<Void> downloadFile(
638-
RemoteActionExecutionContext context,
639-
Path path,
640-
Digest digest,
641-
DownloadProgressReporter reporter)
642-
throws IOException {
643573
Preconditions.checkNotNull(path.getParentDirectory()).createDirectoryAndParents();
644574
if (digest.getSizeBytes() == 0) {
645575
// Handle empty file locally.
@@ -660,9 +590,7 @@ public ListenableFuture<Void> downloadFile(
660590
return COMPLETED_SUCCESS;
661591
}
662592

663-
reporter.started();
664-
OutputStream out = new ReportingOutputStream(new LazyFileOutputStream(path), reporter);
665-
593+
OutputStream out = new LazyFileOutputStream(path);
666594
SettableFuture<Void> outerF = SettableFuture.create();
667595
ListenableFuture<Void> f = cacheProtocol.downloadBlob(context, digest, out);
668596
Futures.addCallback(
@@ -673,7 +601,6 @@ public void onSuccess(Void result) {
673601
try {
674602
out.close();
675603
outerF.set(null);
676-
reporter.finished();
677604
} catch (IOException e) {
678605
outerF.setException(e);
679606
} catch (RuntimeException e) {
@@ -686,7 +613,6 @@ public void onSuccess(Void result) {
686613
public void onFailure(Throwable t) {
687614
try {
688615
out.close();
689-
reporter.finished();
690616
} catch (IOException e) {
691617
if (t != e) {
692618
t.addSuppressed(e);
@@ -1220,49 +1146,6 @@ private static FailureDetail createFailureDetail(String message, Code detailedCo
12201146
.build();
12211147
}
12221148

1223-
/**
1224-
* An {@link OutputStream} that reports all the write operations with {@link
1225-
* DownloadProgressReporter}.
1226-
*/
1227-
private static class ReportingOutputStream extends OutputStream {
1228-
1229-
private final OutputStream out;
1230-
private final DownloadProgressReporter reporter;
1231-
1232-
ReportingOutputStream(OutputStream out, DownloadProgressReporter reporter) {
1233-
this.out = out;
1234-
this.reporter = reporter;
1235-
}
1236-
1237-
@Override
1238-
public void write(byte[] b) throws IOException {
1239-
out.write(b);
1240-
reporter.downloadedBytes(b.length);
1241-
}
1242-
1243-
@Override
1244-
public void write(byte[] b, int off, int len) throws IOException {
1245-
out.write(b, off, len);
1246-
reporter.downloadedBytes(len);
1247-
}
1248-
1249-
@Override
1250-
public void write(int b) throws IOException {
1251-
out.write(b);
1252-
reporter.downloadedBytes(1);
1253-
}
1254-
1255-
@Override
1256-
public void flush() throws IOException {
1257-
out.flush();
1258-
}
1259-
1260-
@Override
1261-
public void close() throws IOException {
1262-
out.close();
1263-
}
1264-
}
1265-
12661149
/** In-memory representation of action result metadata. */
12671150
static class ActionResultMetadata {
12681151

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
385385
remotePathResolver,
386386
result.actionResult,
387387
action.spawnExecutionContext.getFileOutErr(),
388-
action.spawnExecutionContext::lockOutputFiles,
389-
action.spawnExecutionContext::report);
388+
action.spawnExecutionContext::lockOutputFiles);
390389
} else {
391390
PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.spawn);
392391
inMemoryOutput =

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

Lines changed: 0 additions & 29 deletions
This file was deleted.

0 commit comments

Comments
 (0)