Automated rollback of commit be26ba030319cf1cda44649d94d7beee4ccc86c4.
*** Reason for rollback ***
It was not the culprit
*** Original change description ***
Automated rollback of commit a6b5b0540a0167f84437127d3dd6cef804286d6f.
*** Reason for rollback ***
Probably the culprit that broke Windows builds with remote caching: https://github.com/bazelbuild/bazel/issues/9072
*** Original change description ***
Streamline uploading of stdout and stderr
This change removes extra upload logic for stdout/stderr. Besides
the streamlined code this also saves two round trips uploading an
action that produced both stdout and s...
***
ROLLBACK_OF=261700869
RELNOTES: None
PiperOrigin-RevId: 261864282
diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
index 716ee4d..03b417d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCache.java
@@ -640,8 +640,10 @@
private final Path execRoot;
private final boolean allowSymlinks;
private final boolean uploadSymlinks;
- private final Map<Digest, Path> digestToFile;
- private final Map<Digest, Chunker> digestToChunkers;
+ private final Map<Digest, Path> digestToFile = new HashMap<>();
+ private final Map<Digest, Chunker> digestToChunkers = new HashMap<>();
+ private Digest stderrDigest;
+ private Digest stdoutDigest;
/**
* Create an UploadManifest from an ActionResult builder and an exec root. The ActionResult
@@ -658,9 +660,17 @@
this.execRoot = execRoot;
this.uploadSymlinks = uploadSymlinks;
this.allowSymlinks = allowSymlinks;
+ }
- this.digestToFile = new HashMap<>();
- this.digestToChunkers = new HashMap<>();
+ public void setStdoutStderr(FileOutErr outErr) throws IOException {
+ if (outErr.getErrorPath().exists()) {
+ stderrDigest = digestUtil.compute(outErr.getErrorPath());
+ addFile(stderrDigest, outErr.getErrorPath());
+ }
+ if (outErr.getOutputPath().exists()) {
+ stdoutDigest = digestUtil.compute(outErr.getOutputPath());
+ addFile(stdoutDigest, outErr.getOutputPath());
+ }
}
/**
@@ -747,6 +757,16 @@
return digestToChunkers;
}
+ @Nullable
+ public Digest getStdoutDigest() {
+ return stdoutDigest;
+ }
+
+ @Nullable
+ public Digest getStderrDigest() {
+ return stderrDigest;
+ }
+
private void addFileSymbolicLink(Path file, PathFragment target) throws IOException {
result
.addOutputFileSymlinksBuilder()
diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
index 110e8f2..355e510 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcRemoteCache.java
@@ -38,7 +38,6 @@
import com.google.common.base.Ascii;
import com.google.common.base.Preconditions;
import com.google.common.base.Throwables;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Maps;
@@ -419,6 +418,7 @@
options.incompatibleRemoteSymlinks,
options.allowSymlinkUpload);
manifest.addFiles(files);
+ manifest.setStdoutStderr(outErr);
manifest.addAction(actionKey, action, command);
Map<HashCode, Chunker> filesToUpload = Maps.newHashMap();
@@ -449,34 +449,13 @@
uploader.uploadBlobs(filesToUpload, /*forceUpload=*/true);
}
- // TODO(olaola): inline small stdout/stderr here.
- if (outErr.getErrorPath().exists()) {
- Digest stderr = uploadFileContents(outErr.getErrorPath());
- result.setStderrDigest(stderr);
+ if (manifest.getStderrDigest() != null) {
+ result.setStderrDigest(manifest.getStderrDigest());
}
- if (outErr.getOutputPath().exists()) {
- Digest stdout = uploadFileContents(outErr.getOutputPath());
- result.setStdoutDigest(stdout);
+ if (manifest.getStdoutDigest() != null) {
+ result.setStdoutDigest(manifest.getStdoutDigest());
}
}
-
- /**
- * Put the file contents cache if it is not already in it. No-op if the file is already stored in
- * cache. The given path must be a full absolute path.
- *
- * @return The key for fetching the file contents blob from cache.
- */
- private Digest uploadFileContents(Path file) throws IOException, InterruptedException {
- Digest digest = digestUtil.compute(file);
- ImmutableSet<Digest> missing = getMissingDigests(ImmutableList.of(digest));
- if (!missing.isEmpty()) {
- uploader.uploadBlob(
- HashCode.fromString(digest.getHash()),
- Chunker.builder().setInput(digest.getSizeBytes(), file).build(),
- /* forceUpload=*/ true);
- }
- return digest;
- }
// Execution Cache API
