Skip to content

Commit

Permalink
Revert "Remote: Display download progress when actions are downloadin…
Browse files Browse the repository at this point in the history
…g outputs from remote cache."

This reverts commit 306527a.
  • Loading branch information
larsrc-google committed Jul 30, 2021
1 parent 861c3ca commit 92ec798
Show file tree
Hide file tree
Showing 16 changed files with 36 additions and 619 deletions.

This file was deleted.

Expand Up @@ -318,6 +318,7 @@ public void report(ProgressStatus progress) {
return;
}

// TODO(ulfjack): We should report more details to the UI.
ExtendedEventHandler eventHandler = actionExecutionContext.getEventHandler();
progress.postTo(eventHandler, action);
}
Expand Down
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/exec/BUILD
Expand Up @@ -267,7 +267,6 @@ java_library(
srcs = [
"SpawnCheckingCacheEvent.java",
"SpawnExecutingEvent.java",
"SpawnProgressEvent.java",
"SpawnRunner.java",
"SpawnSchedulingEvent.java",
],
Expand Down

This file was deleted.

127 changes: 5 additions & 122 deletions src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
Expand Up @@ -15,8 +15,6 @@

import static com.google.common.util.concurrent.Futures.immediateFuture;
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
import static com.google.devtools.build.lib.remote.common.ProgressStatusListener.NO_ACTION;
import static com.google.devtools.build.lib.remote.util.Utils.bytesCountToDisplayString;
import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture;

import build.bazel.remote.execution.v2.Action;
Expand Down Expand Up @@ -52,7 +50,6 @@
import com.google.devtools.build.lib.actions.UserExecException;
import com.google.devtools.build.lib.actions.cache.MetadataInjector;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
import com.google.devtools.build.lib.exec.SpawnProgressEvent;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.SilentCloseable;
Expand All @@ -61,7 +58,6 @@
import com.google.devtools.build.lib.remote.RemoteCache.ActionResultMetadata.SymlinkMetadata;
import com.google.devtools.build.lib.remote.common.LazyFileOutputStream;
import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException;
import com.google.devtools.build.lib.remote.common.ProgressStatusListener;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteActionFileArtifactValue;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
Expand Down Expand Up @@ -95,9 +91,6 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicLong;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -362,50 +355,6 @@ private static Path toTmpDownloadPath(Path actualPath) {
return actualPath.getParentDirectory().getRelative(actualPath.getBaseName() + ".tmp");
}

static class DownloadProgressReporter {
private static final Pattern PATTERN = Pattern.compile("^bazel-out/[^/]+/[^/]+/");
private final ProgressStatusListener listener;
private final String id;
private final String file;
private final String totalSize;
private final AtomicLong downloadedBytes = new AtomicLong(0);

DownloadProgressReporter(ProgressStatusListener listener, String file, long totalSize) {
this.listener = listener;
this.id = file;
this.totalSize = bytesCountToDisplayString(totalSize);

Matcher matcher = PATTERN.matcher(file);
this.file = matcher.replaceFirst("");
}

void started() {
reportProgress(false, false);
}

void downloadedBytes(int count) {
downloadedBytes.addAndGet(count);
reportProgress(true, false);
}

void finished() {
reportProgress(true, true);
}

private void reportProgress(boolean includeBytes, boolean finished) {
String progress;
if (includeBytes) {
progress =
String.format(
"Downloading %s, %s / %s",
file, bytesCountToDisplayString(downloadedBytes.get()), totalSize);
} else {
progress = String.format("Downloading %s", file);
}
listener.onProgressStatus(SpawnProgressEvent.create(id, progress, finished));
}
}

/**
* Download the output files and directory trees of a remotely executed action to the local
* machine, as well stdin / stdout to the given files.
Expand All @@ -422,8 +371,7 @@ public void download(
RemotePathResolver remotePathResolver,
ActionResult result,
FileOutErr origOutErr,
OutputFilesLocker outputFilesLocker,
ProgressStatusListener progressStatusListener)
OutputFilesLocker outputFilesLocker)
throws ExecException, IOException, InterruptedException {
ActionResultMetadata metadata = parseActionResultMetadata(context, remotePathResolver, result);

Expand All @@ -440,11 +388,7 @@ public void download(
context,
remotePathResolver.localPathToOutputPath(file.path()),
toTmpDownloadPath(file.path()),
file.digest(),
new DownloadProgressReporter(
progressStatusListener,
remotePathResolver.localPathToOutputPath(file.path()),
file.digest().getSizeBytes()));
file.digest());
return Futures.transform(download, (d) -> file, directExecutor());
} catch (IOException e) {
return Futures.<FileMetadata>immediateFailedFuture(e);
Expand Down Expand Up @@ -596,14 +540,10 @@ private void createSymlinks(Iterable<SymlinkMetadata> symlinks) throws IOExcepti
}

public ListenableFuture<Void> downloadFile(
RemoteActionExecutionContext context,
String outputPath,
Path localPath,
Digest digest,
DownloadProgressReporter reporter)
RemoteActionExecutionContext context, String outputPath, Path localPath, Digest digest)
throws IOException {
SettableFuture<Void> outerF = SettableFuture.create();
ListenableFuture<Void> f = downloadFile(context, localPath, digest, reporter);
ListenableFuture<Void> f = downloadFile(context, localPath, digest);
Futures.addCallback(
f,
new FutureCallback<Void>() {
Expand All @@ -630,16 +570,6 @@ public void onFailure(Throwable throwable) {
/** Downloads a file (that is not a directory). The content is fetched from the digest. */
public ListenableFuture<Void> downloadFile(
RemoteActionExecutionContext context, Path path, Digest digest) throws IOException {
return downloadFile(context, path, digest, new DownloadProgressReporter(NO_ACTION, "", 0));
}

/** Downloads a file (that is not a directory). The content is fetched from the digest. */
public ListenableFuture<Void> downloadFile(
RemoteActionExecutionContext context,
Path path,
Digest digest,
DownloadProgressReporter reporter)
throws IOException {
Preconditions.checkNotNull(path.getParentDirectory()).createDirectoryAndParents();
if (digest.getSizeBytes() == 0) {
// Handle empty file locally.
Expand All @@ -660,9 +590,7 @@ public ListenableFuture<Void> downloadFile(
return COMPLETED_SUCCESS;
}

reporter.started();
OutputStream out = new ReportingOutputStream(new LazyFileOutputStream(path), reporter);

OutputStream out = new LazyFileOutputStream(path);
SettableFuture<Void> outerF = SettableFuture.create();
ListenableFuture<Void> f = cacheProtocol.downloadBlob(context, digest, out);
Futures.addCallback(
Expand All @@ -673,7 +601,6 @@ public void onSuccess(Void result) {
try {
out.close();
outerF.set(null);
reporter.finished();
} catch (IOException e) {
outerF.setException(e);
} catch (RuntimeException e) {
Expand All @@ -686,7 +613,6 @@ public void onSuccess(Void result) {
public void onFailure(Throwable t) {
try {
out.close();
reporter.finished();
} catch (IOException e) {
if (t != e) {
t.addSuppressed(e);
Expand Down Expand Up @@ -1220,49 +1146,6 @@ private static FailureDetail createFailureDetail(String message, Code detailedCo
.build();
}

/**
* An {@link OutputStream} that reports all the write operations with {@link
* DownloadProgressReporter}.
*/
private static class ReportingOutputStream extends OutputStream {

private final OutputStream out;
private final DownloadProgressReporter reporter;

ReportingOutputStream(OutputStream out, DownloadProgressReporter reporter) {
this.out = out;
this.reporter = reporter;
}

@Override
public void write(byte[] b) throws IOException {
out.write(b);
reporter.downloadedBytes(b.length);
}

@Override
public void write(byte[] b, int off, int len) throws IOException {
out.write(b, off, len);
reporter.downloadedBytes(len);
}

@Override
public void write(int b) throws IOException {
out.write(b);
reporter.downloadedBytes(1);
}

@Override
public void flush() throws IOException {
out.flush();
}

@Override
public void close() throws IOException {
out.close();
}
}

/** In-memory representation of action result metadata. */
static class ActionResultMetadata {

Expand Down
Expand Up @@ -385,8 +385,7 @@ public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult re
remotePathResolver,
result.actionResult,
action.spawnExecutionContext.getFileOutErr(),
action.spawnExecutionContext::lockOutputFiles,
action.spawnExecutionContext::report);
action.spawnExecutionContext::lockOutputFiles);
} else {
PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.spawn);
inMemoryOutput =
Expand Down

This file was deleted.

0 comments on commit 92ec798

Please sign in to comment.