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.

Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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.

Loading

0 comments on commit 92ec798

Please sign in to comment.