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