Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streamline uploading of stdout and stderr #9025

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,10 @@ static class UploadManifest {
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
Expand All @@ -658,9 +660,17 @@ public UploadManifest(
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());
}
}

/**
Expand Down Expand Up @@ -747,6 +757,16 @@ public Map<Digest, Chunker> getDigestToChunkers() {
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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,7 @@ void upload(
options.incompatibleRemoteSymlinks,
options.allowSymlinkUpload);
manifest.addFiles(files);
manifest.setStdoutStderr(outErr);
manifest.addAction(actionKey, action, command);

Map<HashCode, Chunker> filesToUpload = Maps.newHashMap();
Expand Down Expand Up @@ -449,35 +450,14 @@ void upload(
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;
}

Digest uploadBlob(byte[] blob) throws IOException, InterruptedException {
Digest digest = digestUtil.compute(blob);
ImmutableSet<Digest> missing = getMissingDigests(ImmutableList.of(digest));
Expand Down