Pull upload(ActionResult) into super class.
Remove the custom upload(ActionResult) implementations from SimpleBlobStoreActionCache and GrpcRemoteCache.
This is a big step towards merging SimpleBlobStoreActionCache and GrpcRemoteCache.
Closes #9167.
PiperOrigin-RevId: 264563384
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 d53bb72..b5dbc4c 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
@@ -29,6 +29,7 @@
import build.bazel.remote.execution.v2.Tree;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
+import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
@@ -79,6 +80,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.concurrent.ExecutionException;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.Nullable;
@@ -120,20 +122,8 @@
abstract ActionResult getCachedActionResult(ActionKey actionKey)
throws IOException, InterruptedException;
- /**
- * Upload the result of a locally executed action to the remote cache.
- *
- * @throws IOException if there was an error uploading to the remote cache
- * @throws ExecException if uploading any of the action outputs is not supported
- */
- abstract void upload(
- SimpleBlobStore.ActionKey actionKey,
- Action action,
- Command command,
- Path execRoot,
- Collection<Path> files,
- FileOutErr outErr)
- throws ExecException, IOException, InterruptedException;
+ protected abstract void setCachedActionResult(ActionKey actionKey, ActionResult action)
+ throws IOException, InterruptedException;
/**
* Uploads a file
@@ -157,6 +147,116 @@
*/
protected abstract ListenableFuture<Void> uploadBlob(Digest digest, ByteString data);
+ protected abstract ImmutableSet<Digest> getMissingDigests(Iterable<Digest> digests)
+ throws IOException, InterruptedException;
+
+ /**
+ * Upload the result of a locally executed action to the remote cache.
+ *
+ * @throws IOException if there was an error uploading to the remote cache
+ * @throws ExecException if uploading any of the action outputs is not supported
+ */
+ public ActionResult upload(
+ ActionKey actionKey,
+ Action action,
+ Command command,
+ Path execRoot,
+ Collection<Path> outputs,
+ FileOutErr outErr,
+ int exitCode)
+ throws ExecException, IOException, InterruptedException {
+ ActionResult.Builder resultBuilder = ActionResult.newBuilder();
+ uploadOutputs(execRoot, actionKey, action, command, outputs, outErr, resultBuilder);
+ resultBuilder.setExitCode(exitCode);
+ ActionResult result = resultBuilder.build();
+ if (exitCode == 0 && !action.getDoNotCache()) {
+ setCachedActionResult(actionKey, result);
+ }
+ return result;
+ }
+
+ public ActionResult upload(
+ ActionKey actionKey,
+ Action action,
+ Command command,
+ Path execRoot,
+ Collection<Path> outputs,
+ FileOutErr outErr)
+ throws ExecException, IOException, InterruptedException {
+ return upload(actionKey, action, command, execRoot, outputs, outErr, /* exitCode= */ 0);
+ }
+
+ private void uploadOutputs(
+ Path execRoot,
+ ActionKey actionKey,
+ Action action,
+ Command command,
+ Collection<Path> files,
+ FileOutErr outErr,
+ ActionResult.Builder result)
+ throws ExecException, IOException, InterruptedException {
+ UploadManifest manifest =
+ new UploadManifest(
+ digestUtil,
+ result,
+ execRoot,
+ options.incompatibleRemoteSymlinks,
+ options.allowSymlinkUpload);
+ manifest.addFiles(files);
+ manifest.setStdoutStderr(outErr);
+ manifest.addAction(actionKey, action, command);
+
+ Map<Digest, Path> digestToFile = manifest.getDigestToFile();
+ Map<Digest, ByteString> digestToBlobs = manifest.getDigestToBlobs();
+ Collection<Digest> digests = new ArrayList<>();
+ digests.addAll(digestToFile.keySet());
+ digests.addAll(digestToBlobs.keySet());
+
+ ImmutableSet<Digest> digestsToUpload = getMissingDigests(digests);
+ ImmutableList.Builder<ListenableFuture<Void>> uploads = ImmutableList.builder();
+ for (Digest digest : digestsToUpload) {
+ Path file = digestToFile.get(digest);
+ if (file != null) {
+ uploads.add(uploadFile(digest, file));
+ } else {
+ ByteString blob = digestToBlobs.get(digest);
+ if (blob == null) {
+ String message = "FindMissingBlobs call returned an unknown digest: " + digest;
+ throw new IOException(message);
+ }
+ uploads.add(uploadBlob(digest, blob));
+ }
+ }
+
+ waitForUploads(uploads.build());
+
+ if (manifest.getStderrDigest() != null) {
+ result.setStderrDigest(manifest.getStderrDigest());
+ }
+ if (manifest.getStdoutDigest() != null) {
+ result.setStdoutDigest(manifest.getStdoutDigest());
+ }
+ }
+
+ private static void waitForUploads(List<ListenableFuture<Void>> uploads)
+ throws IOException, InterruptedException {
+ try {
+ for (ListenableFuture<Void> upload : uploads) {
+ upload.get();
+ }
+ } catch (ExecutionException e) {
+ // TODO(buchgr): Add support for cancellation and factor this method out to be shared
+ // between ByteStreamUploader as well.
+ Throwable cause = e.getCause();
+ Throwables.throwIfInstanceOf(cause, IOException.class);
+ Throwables.throwIfInstanceOf(cause, InterruptedException.class);
+ if (cause != null) {
+ throw new IOException(cause);
+ }
+ throw new IOException(e);
+ }
+ }
+
/**
* Downloads a blob with a content hash {@code digest} to {@code out}.
*
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 e6c5c63..e9fdb16 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
@@ -17,11 +17,9 @@
import static com.google.common.base.Strings.isNullOrEmpty;
import static java.lang.String.format;
-import build.bazel.remote.execution.v2.Action;
import build.bazel.remote.execution.v2.ActionCacheGrpc;
import build.bazel.remote.execution.v2.ActionCacheGrpc.ActionCacheBlockingStub;
import build.bazel.remote.execution.v2.ActionResult;
-import build.bazel.remote.execution.v2.Command;
import build.bazel.remote.execution.v2.ContentAddressableStorageGrpc;
import build.bazel.remote.execution.v2.ContentAddressableStorageGrpc.ContentAddressableStorageFutureStub;
import build.bazel.remote.execution.v2.Digest;
@@ -38,7 +36,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;
@@ -50,7 +47,6 @@
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.SettableFuture;
import com.google.devtools.build.lib.actions.ActionInput;
-import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.remote.RemoteRetrier.ProgressiveBackoff;
import com.google.devtools.build.lib.remote.common.SimpleBlobStore.ActionKey;
@@ -58,7 +54,6 @@
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
-import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.Path;
import com.google.protobuf.ByteString;
import com.google.protobuf.Message;
@@ -70,7 +65,6 @@
import java.io.IOException;
import java.io.OutputStream;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ExecutionException;
@@ -166,17 +160,8 @@
|| Ascii.toLowerCase(options.remoteCache).startsWith("https://"));
}
- private ListenableFuture<FindMissingBlobsResponse> getMissingDigests(
- FindMissingBlobsRequest request) throws IOException, InterruptedException {
- Context ctx = Context.current();
- try {
- return retrier.executeAsync(() -> ctx.call(() -> casFutureStub().findMissingBlobs(request)));
- } catch (StatusRuntimeException e) {
- throw new IOException(e);
- }
- }
-
- private ImmutableSet<Digest> getMissingDigests(Iterable<Digest> digests)
+ @Override
+ protected ImmutableSet<Digest> getMissingDigests(Iterable<Digest> digests)
throws IOException, InterruptedException {
if (Iterables.isEmpty(digests)) {
return ImmutableSet.of();
@@ -208,6 +193,16 @@
return result.build();
}
+ private ListenableFuture<FindMissingBlobsResponse> getMissingDigests(
+ FindMissingBlobsRequest request) throws IOException, InterruptedException {
+ Context ctx = Context.current();
+ try {
+ return retrier.executeAsync(() -> ctx.call(() -> casFutureStub().findMissingBlobs(request)));
+ } catch (StatusRuntimeException e) {
+ throw new IOException(e);
+ }
+ }
+
/**
* Ensures that the tree structure of the inputs, the input files themselves, and the command are
* available in the remote cache, such that the tree can be reassembled and executed on another
@@ -377,32 +372,6 @@
}
@Override
- public void upload(
- ActionKey actionKey,
- Action action,
- Command command,
- Path execRoot,
- Collection<Path> files,
- FileOutErr outErr)
- throws ExecException, IOException, InterruptedException {
- ActionResult.Builder result = ActionResult.newBuilder();
- upload(execRoot, actionKey, action, command, files, outErr, result);
- try {
- retrier.execute(
- () ->
- acBlockingStub()
- .updateActionResult(
- UpdateActionResultRequest.newBuilder()
- .setInstanceName(options.remoteInstanceName)
- .setActionDigest(actionKey.getDigest())
- .setActionResult(result)
- .build()));
- } catch (StatusRuntimeException e) {
- throw new IOException(e);
- }
- }
-
- @Override
protected ListenableFuture<Void> uploadFile(Digest digest, Path path) {
return uploader.uploadBlobAsync(
HashCode.fromString(digest.getHash()),
@@ -418,77 +387,6 @@
/* forceUpload= */ true);
}
- void upload(
- Path execRoot,
- ActionKey actionKey,
- Action action,
- Command command,
- Collection<Path> files,
- FileOutErr outErr,
- ActionResult.Builder result)
- throws ExecException, IOException, InterruptedException {
- UploadManifest manifest =
- new UploadManifest(
- digestUtil,
- result,
- execRoot,
- options.incompatibleRemoteSymlinks,
- options.allowSymlinkUpload);
- manifest.addFiles(files);
- manifest.setStdoutStderr(outErr);
- manifest.addAction(actionKey, action, command);
-
- Map<Digest, Path> digestToFile = manifest.getDigestToFile();
- Map<Digest, ByteString> digestToBlobs = manifest.getDigestToBlobs();
- Collection<Digest> digests = new ArrayList<>();
- digests.addAll(digestToFile.keySet());
- digests.addAll(digestToBlobs.keySet());
-
- ImmutableSet<Digest> digestsToUpload = getMissingDigests(digests);
- ImmutableList.Builder<ListenableFuture<Void>> uploads = ImmutableList.builder();
- for (Digest digest : digestsToUpload) {
- Path file = digestToFile.get(digest);
- if (file != null) {
- uploads.add(uploadFile(digest, file));
- } else {
- ByteString blob = digestToBlobs.get(digest);
- if (blob == null) {
- String message = "FindMissingBlobs call returned an unknown digest: " + digest;
- throw new IOException(message);
- }
- uploads.add(uploadBlob(digest, blob));
- }
- }
-
- waitForUploads(uploads.build());
-
- if (manifest.getStderrDigest() != null) {
- result.setStderrDigest(manifest.getStderrDigest());
- }
- if (manifest.getStdoutDigest() != null) {
- result.setStdoutDigest(manifest.getStdoutDigest());
- }
- }
-
- private static void waitForUploads(List<ListenableFuture<Void>> uploads)
- throws IOException, InterruptedException {
- try {
- for (ListenableFuture<Void> upload : uploads) {
- upload.get();
- }
- } catch (ExecutionException e) {
- // TODO(buchgr): Add support for cancellation and factor this method out to be shared
- // between ByteStreamUploader as well.
- Throwable cause = e.getCause();
- Throwables.throwIfInstanceOf(cause, IOException.class);
- Throwables.throwIfInstanceOf(cause, InterruptedException.class);
- if (cause != null) {
- throw new IOException(cause);
- }
- throw new IOException(e);
- }
- }
-
// Execution Cache API
@Override
@@ -511,4 +409,22 @@
throw new IOException(e);
}
}
+
+ @Override
+ protected void setCachedActionResult(ActionKey actionKey, ActionResult result)
+ throws IOException, InterruptedException {
+ try {
+ retrier.execute(
+ () ->
+ acBlockingStub()
+ .updateActionResult(
+ UpdateActionResultRequest.newBuilder()
+ .setInstanceName(options.remoteInstanceName)
+ .setActionDigest(actionKey.getDigest())
+ .setActionResult(result)
+ .build()));
+ } catch (StatusRuntimeException e) {
+ throw new IOException(e);
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java
index fba3ba0..8822c0d 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java
@@ -14,35 +14,30 @@
package com.google.devtools.build.lib.remote;
-import build.bazel.remote.execution.v2.Action;
import build.bazel.remote.execution.v2.ActionResult;
-import build.bazel.remote.execution.v2.Command;
import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.Directory;
import build.bazel.remote.execution.v2.DirectoryNode;
import build.bazel.remote.execution.v2.FileNode;
+import com.google.common.collect.ImmutableSet;
import com.google.common.hash.HashingOutputStream;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.SettableFuture;
-import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.remote.common.SimpleBlobStore;
import com.google.devtools.build.lib.remote.common.SimpleBlobStore.ActionKey;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.Utils;
-import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.Path;
import com.google.protobuf.ByteString;
import com.google.protobuf.InvalidProtocolBufferException;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
-import java.util.Collection;
-import java.util.Map;
import javax.annotation.Nullable;
/**
@@ -77,60 +72,6 @@
}
@Override
- public void upload(
- SimpleBlobStore.ActionKey actionKey,
- Action action,
- Command command,
- Path execRoot,
- Collection<Path> files,
- FileOutErr outErr)
- throws ExecException, IOException, InterruptedException {
- ActionResult.Builder result = ActionResult.newBuilder();
- upload(result, actionKey, action, command, execRoot, files, /* uploadAction= */ true);
- if (outErr.getErrorPath().exists()) {
- Digest stdErrDigest = digestUtil.compute(outErr.getErrorPath());
- getFromFuture(uploadFile(stdErrDigest, outErr.getErrorPath()));
- result.setStderrDigest(stdErrDigest);
- }
- if (outErr.getOutputPath().exists()) {
- Digest stdoutDigest = digestUtil.compute(outErr.getOutputPath());
- getFromFuture(uploadFile(stdoutDigest, outErr.getOutputPath()));
- result.setStdoutDigest(stdoutDigest);
- }
- blobStore.putActionResult(actionKey, result.build());
- }
-
- public void upload(
- ActionResult.Builder result,
- SimpleBlobStore.ActionKey actionKey,
- Action action,
- Command command,
- Path execRoot,
- Collection<Path> files,
- boolean uploadAction)
- throws ExecException, IOException, InterruptedException {
- UploadManifest manifest =
- new UploadManifest(
- digestUtil,
- result,
- execRoot,
- options.incompatibleRemoteSymlinks,
- options.allowSymlinkUpload);
- manifest.addFiles(files);
- if (uploadAction) {
- manifest.addAction(actionKey, action, command);
- }
-
- for (Map.Entry<Digest, Path> entry : manifest.getDigestToFile().entrySet()) {
- getFromFuture(uploadFile(entry.getKey(), entry.getValue()));
- }
-
- for (Map.Entry<Digest, ByteString> entry : manifest.getDigestToBlobs().entrySet()) {
- getFromFuture(uploadBlob(entry.getKey(), entry.getValue()));
- }
- }
-
- @Override
public ListenableFuture<Void> uploadFile(Digest digest, Path file) {
return blobStore.uploadFile(digest, file);
}
@@ -140,6 +81,11 @@
return blobStore.uploadBlob(digest, data);
}
+ @Override
+ protected ImmutableSet<Digest> getMissingDigests(Iterable<Digest> digests) {
+ return ImmutableSet.copyOf(digests);
+ }
+
public boolean containsKey(Digest digest) throws IOException, InterruptedException {
return blobStore.contains(digest.getHash());
}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java b/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
index d8e6959..a9b274f 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/AbstractRemoteActionCacheTests.java
@@ -24,9 +24,7 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
-import build.bazel.remote.execution.v2.Action;
import build.bazel.remote.execution.v2.ActionResult;
-import build.bazel.remote.execution.v2.Command;
import build.bazel.remote.execution.v2.Digest;
import build.bazel.remote.execution.v2.Directory;
import build.bazel.remote.execution.v2.DirectoryNode;
@@ -38,6 +36,7 @@
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
@@ -76,7 +75,6 @@
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
-import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -1150,20 +1148,12 @@
@Nullable
@Override
- ActionResult getCachedActionResult(ActionKey actionKey)
- throws IOException, InterruptedException {
+ ActionResult getCachedActionResult(ActionKey actionKey) {
throw new UnsupportedOperationException();
}
@Override
- void upload(
- ActionKey actionKey,
- Action action,
- Command command,
- Path execRoot,
- Collection<Path> files,
- FileOutErr outErr)
- throws ExecException, IOException, InterruptedException {
+ protected void setCachedActionResult(ActionKey actionKey, ActionResult action) {
throw new UnsupportedOperationException();
}
@@ -1178,6 +1168,11 @@
}
@Override
+ protected ImmutableSet<Digest> getMissingDigests(Iterable<Digest> digests) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
protected ListenableFuture<Void> downloadBlob(Digest digest, OutputStream out) {
SettableFuture<Void> result = SettableFuture.create();
ListenableFuture<byte[]> downloadResult = downloadResults.get(digest);
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
index bfa5320..4d8119ef 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java
@@ -530,6 +530,15 @@
responseObserver.onCompleted();
}
});
+ serviceRegistry.addService(
+ new ActionCacheImplBase() {
+ @Override
+ public void updateActionResult(
+ UpdateActionResultRequest request, StreamObserver<ActionResult> responseObserver) {
+ responseObserver.onNext(request.getActionResult());
+ responseObserver.onCompleted();
+ }
+ });
ActionResult result = uploadDirectory(client, ImmutableList.<Path>of(fooFile, barDir));
ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -558,6 +567,15 @@
responseObserver.onCompleted();
}
});
+ serviceRegistry.addService(
+ new ActionCacheImplBase() {
+ @Override
+ public void updateActionResult(
+ UpdateActionResultRequest request, StreamObserver<ActionResult> responseObserver) {
+ responseObserver.onNext(request.getActionResult());
+ responseObserver.onCompleted();
+ }
+ });
ActionResult result = uploadDirectory(client, ImmutableList.<Path>of(barDir));
ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -608,6 +626,15 @@
responseObserver.onCompleted();
}
});
+ serviceRegistry.addService(
+ new ActionCacheImplBase() {
+ @Override
+ public void updateActionResult(
+ UpdateActionResultRequest request, StreamObserver<ActionResult> responseObserver) {
+ responseObserver.onNext(request.getActionResult());
+ responseObserver.onCompleted();
+ }
+ });
ActionResult result = uploadDirectory(client, ImmutableList.of(barDir));
ActionResult.Builder expectedResult = ActionResult.newBuilder();
@@ -617,12 +644,10 @@
private ActionResult uploadDirectory(GrpcRemoteCache client, List<Path> outputs)
throws Exception {
- ActionResult.Builder result = ActionResult.newBuilder();
Action action = Action.getDefaultInstance();
ActionKey actionKey = DIGEST_UTIL.computeActionKey(action);
Command cmd = Command.getDefaultInstance();
- client.upload(execRoot, actionKey, action, cmd, outputs, outErr, result);
- return result.build();
+ return client.upload(actionKey, action, cmd, execRoot, outputs, outErr);
}
@Test
@@ -662,16 +687,24 @@
responseObserver.onCompleted();
}
});
+ serviceRegistry.addService(
+ new ActionCacheImplBase() {
+ @Override
+ public void updateActionResult(
+ UpdateActionResultRequest request, StreamObserver<ActionResult> responseObserver) {
+ responseObserver.onNext(request.getActionResult());
+ responseObserver.onCompleted();
+ }
+ });
- ActionResult.Builder result = ActionResult.newBuilder();
- client.upload(
- execRoot,
- DIGEST_UTIL.asActionKey(actionDigest),
- action,
- command,
- ImmutableList.<Path>of(fooFile, barFile),
- outErr,
- result);
+ ActionResult result =
+ client.upload(
+ DIGEST_UTIL.asActionKey(actionDigest),
+ action,
+ command,
+ execRoot,
+ ImmutableList.of(fooFile, barFile),
+ outErr);
ActionResult.Builder expectedResult = ActionResult.newBuilder();
expectedResult.setStdoutDigest(stdoutDigest);
expectedResult.setStderrDigest(stderrDigest);
@@ -681,7 +714,7 @@
.setPath("bar")
.setDigest(barDigest)
.setIsExecutable(true);
- assertThat(result.build()).isEqualTo(expectedResult.build());
+ assertThat(result).isEqualTo(expectedResult.build());
}
@Test
@@ -711,19 +744,27 @@
responseObserver.onCompleted();
}
});
+ serviceRegistry.addService(
+ new ActionCacheImplBase() {
+ @Override
+ public void updateActionResult(
+ UpdateActionResultRequest request, StreamObserver<ActionResult> responseObserver) {
+ responseObserver.onNext(request.getActionResult());
+ responseObserver.onCompleted();
+ }
+ });
RemoteOptions options = Options.getDefaults(RemoteOptions.class);
options.maxOutboundMessageSize = 80; // Enough for one digest, but not two.
final GrpcRemoteCache client = newClient(options);
- ActionResult.Builder result = ActionResult.newBuilder();
- client.upload(
- execRoot,
- DIGEST_UTIL.asActionKey(actionDigest),
- action,
- command,
- ImmutableList.<Path>of(fooFile, barFile),
- outErr,
- result);
+ ActionResult result =
+ client.upload(
+ DIGEST_UTIL.asActionKey(actionDigest),
+ action,
+ command,
+ execRoot,
+ ImmutableList.of(fooFile, barFile),
+ outErr);
ActionResult.Builder expectedResult = ActionResult.newBuilder();
expectedResult.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest);
expectedResult
@@ -731,7 +772,7 @@
.setPath("bar")
.setDigest(barDigest)
.setIsExecutable(true);
- assertThat(result.build()).isEqualTo(expectedResult.build());
+ assertThat(result).isEqualTo(expectedResult.build());
assertThat(numGetMissingCalls.get()).isEqualTo(4);
}
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java
index 24ad516..6c5fe6d 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcherTest.java
@@ -16,12 +16,11 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
-import build.bazel.remote.execution.v2.Action;
import build.bazel.remote.execution.v2.ActionResult;
-import build.bazel.remote.execution.v2.Command;
import build.bazel.remote.execution.v2.Digest;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
import com.google.common.hash.HashCode;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
@@ -39,7 +38,6 @@
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.StaticMetadataProvider;
import com.google.devtools.build.lib.remote.util.StringActionInput;
-import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -52,7 +50,6 @@
import java.io.IOException;
import java.io.OutputStream;
import java.nio.charset.StandardCharsets;
-import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import org.junit.Before;
@@ -232,13 +229,7 @@
}
@Override
- void upload(
- ActionKey actionKey,
- Action action,
- Command command,
- Path execRoot,
- Collection<Path> files,
- FileOutErr outErr) {
+ protected void setCachedActionResult(ActionKey actionKey, ActionResult action) {
throw new UnsupportedOperationException();
}
@@ -253,6 +244,11 @@
}
@Override
+ protected ImmutableSet<Digest> getMissingDigests(Iterable<Digest> digests) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
protected ListenableFuture<Void> downloadBlob(Digest digest, OutputStream out) {
ByteString data = cacheEntries.get(digest);
if (data == null) {
diff --git a/src/test/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCacheTest.java
index 271dc69..4071de4 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCacheTest.java
@@ -37,6 +37,7 @@
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
+import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
@@ -315,22 +316,21 @@
final ConcurrentMap<String, byte[]> map = new ConcurrentHashMap<>();
final SimpleBlobStoreActionCache client = newClient(map);
- ActionResult.Builder result = ActionResult.newBuilder();
- client.upload(
- result,
- DIGEST_UTIL.asActionKey(actionDigest),
- action,
- cmd,
- execRoot,
- ImmutableList.<Path>of(fooFile, barDir),
- /* uploadAction= */ true);
+ ActionResult result =
+ client.upload(
+ DIGEST_UTIL.asActionKey(actionDigest),
+ action,
+ cmd,
+ execRoot,
+ ImmutableList.of(fooFile, barDir),
+ new FileOutErr(execRoot.getRelative("stdout"), execRoot.getRelative("stderr")));
ActionResult.Builder expectedResult = ActionResult.newBuilder();
expectedResult.addOutputFilesBuilder().setPath("a/foo").setDigest(fooDigest);
expectedResult.addOutputDirectoriesBuilder().setPath("bar").setTreeDigest(barDigest);
- assertThat(result.build()).isEqualTo(expectedResult.build());
+ assertThat(result).isEqualTo(expectedResult.build());
assertThat(map.keySet())
- .containsExactly(
+ .containsAtLeast(
fooDigest.getHash(),
quxDigest.getHash(),
barDigest.getHash(),
@@ -402,12 +402,16 @@
private ActionResult uploadDirectory(SimpleBlobStoreActionCache client, List<Path> outputs)
throws Exception {
- ActionResult.Builder result = ActionResult.newBuilder();
Action action = Action.getDefaultInstance();
ActionKey actionKey = DIGEST_UTIL.computeActionKey(action);
Command cmd = Command.getDefaultInstance();
- client.upload(result, actionKey, action, cmd, execRoot, outputs, /* uploadAction= */ true);
- return result.build();
+ return client.upload(
+ actionKey,
+ action,
+ cmd,
+ execRoot,
+ outputs,
+ new FileOutErr(execRoot.getRelative("stdout"), execRoot.getRelative("stderr")));
}
@Test
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD
index 4c6659f..e9dc09d 100644
--- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD
+++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/BUILD
@@ -16,6 +16,7 @@
visibility = ["//src/tools/remote:__subpackages__"],
deps = [
"//src/main/java/com/google/devtools/build/lib:build-base",
+ "//src/main/java/com/google/devtools/build/lib:io",
"//src/main/java/com/google/devtools/build/lib:os_util",
"//src/main/java/com/google/devtools/build/lib:packages-internal",
"//src/main/java/com/google/devtools/build/lib:process_util",
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
index abc3e6f..925c944 100644
--- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
+++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java
@@ -47,11 +47,11 @@
import com.google.devtools.build.lib.shell.CommandException;
import com.google.devtools.build.lib.shell.CommandResult;
import com.google.devtools.build.lib.shell.FutureCommandResult;
+import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.longrunning.Operation;
import com.google.protobuf.Any;
-import com.google.protobuf.ByteString;
import com.google.protobuf.util.Durations;
import com.google.rpc.Code;
import com.google.rpc.Status;
@@ -284,84 +284,80 @@
long startTime = System.currentTimeMillis();
CommandResult cmdResult = null;
- FutureCommandResult futureCmdResult = null;
- try {
- futureCmdResult = cmd.executeAsync();
- } catch (CommandException e) {
- Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
- }
+ String uuid = UUID.randomUUID().toString();
+ Path stdout = execRoot.getChild("stdout-" + uuid);
+ Path stderr = execRoot.getChild("stderr-" + uuid);
+ try (FileOutErr outErr = new FileOutErr(stdout, stderr)) {
- if (futureCmdResult != null) {
+ FutureCommandResult futureCmdResult = null;
try {
- cmdResult = futureCmdResult.get();
- } catch (AbnormalTerminationException e) {
- cmdResult = e.getResult();
+ futureCmdResult = cmd.executeAsync(outErr.getOutputStream(), outErr.getErrorStream());
+ } catch (CommandException e) {
+ Throwables.throwIfInstanceOf(e.getCause(), IOException.class);
}
- }
- long timeoutMillis =
- action.hasTimeout()
- ? Durations.toMillis(action.getTimeout())
- : TimeUnit.MINUTES.toMillis(15);
- boolean wasTimeout =
- (cmdResult != null && cmdResult.getTerminationStatus().timedOut())
- || wasTimeout(timeoutMillis, System.currentTimeMillis() - startTime);
- final int exitCode;
- Status errStatus = null;
- ExecuteResponse.Builder resp = ExecuteResponse.newBuilder();
- if (wasTimeout) {
- final String errMessage =
- String.format(
- "Command:\n%s\nexceeded deadline of %f seconds.",
- Arrays.toString(command.getArgumentsList().toArray()), timeoutMillis / 1000.0);
- logger.warning(errMessage);
- errStatus =
- Status.newBuilder()
- .setCode(Code.DEADLINE_EXCEEDED.getNumber())
- .setMessage(errMessage)
- .build();
- exitCode = LOCAL_EXEC_ERROR;
- } else if (cmdResult == null) {
- exitCode = LOCAL_EXEC_ERROR;
- } else {
- exitCode = cmdResult.getTerminationStatus().getRawExitCode();
- }
+ if (futureCmdResult != null) {
+ try {
+ cmdResult = futureCmdResult.get();
+ } catch (AbnormalTerminationException e) {
+ cmdResult = e.getResult();
+ }
+ }
- ActionResult.Builder result = ActionResult.newBuilder();
- boolean setResult = exitCode == 0 && !action.getDoNotCache();
- try {
- cache.upload(result, actionKey, action, command, execRoot, outputs, setResult);
- } catch (ExecException e) {
- if (errStatus == null) {
+ long timeoutMillis =
+ action.hasTimeout()
+ ? Durations.toMillis(action.getTimeout())
+ : TimeUnit.MINUTES.toMillis(15);
+ boolean wasTimeout =
+ (cmdResult != null && cmdResult.getTerminationStatus().timedOut())
+ || wasTimeout(timeoutMillis, System.currentTimeMillis() - startTime);
+ final int exitCode;
+ Status errStatus = null;
+ ExecuteResponse.Builder resp = ExecuteResponse.newBuilder();
+ if (wasTimeout) {
+ final String errMessage =
+ String.format(
+ "Command:\n%s\nexceeded deadline of %f seconds.",
+ Arrays.toString(command.getArgumentsList().toArray()), timeoutMillis / 1000.0);
+ logger.warning(errMessage);
errStatus =
Status.newBuilder()
- .setCode(Code.FAILED_PRECONDITION.getNumber())
- .setMessage(e.getMessage())
+ .setCode(Code.DEADLINE_EXCEEDED.getNumber())
+ .setMessage(errMessage)
.build();
+ exitCode = LOCAL_EXEC_ERROR;
+ } else if (cmdResult == null) {
+ exitCode = LOCAL_EXEC_ERROR;
+ } else {
+ exitCode = cmdResult.getTerminationStatus().getRawExitCode();
}
- }
- byte[] stdout = cmdResult.getStdout();
- if (stdout.length > 0) {
- Digest stdoutDigest = digestUtil.compute(stdout);
- getFromFuture(cache.uploadBlob(stdoutDigest, ByteString.copyFrom(stdout)));
- result.setStdoutDigest(stdoutDigest);
- }
- byte[] stderr = cmdResult.getStderr();
- if (stderr.length > 0) {
- Digest stderrDigest = digestUtil.compute(stderr);
- getFromFuture(cache.uploadBlob(stderrDigest, ByteString.copyFrom(stderr)));
- result.setStderrDigest(stderrDigest);
- }
- ActionResult finalResult = result.setExitCode(exitCode).build();
- resp.setResult(finalResult);
- if (errStatus != null) {
- resp.setStatus(errStatus);
- throw new ExecutionStatusException(errStatus, resp.build());
- } else if (setResult) {
- cache.setCachedActionResult(actionKey, finalResult);
+ ActionResult result = null;
+ try {
+ result = cache.upload(actionKey, action, command, execRoot, outputs, outErr, exitCode);
+ } catch (ExecException e) {
+ if (errStatus == null) {
+ errStatus =
+ Status.newBuilder()
+ .setCode(Code.FAILED_PRECONDITION.getNumber())
+ .setMessage(e.getMessage())
+ .build();
+ }
+ }
+
+ if (result == null) {
+ result = ActionResult.newBuilder().setExitCode(exitCode).build();
+ }
+
+ resp.setResult(result);
+
+ if (errStatus != null) {
+ resp.setStatus(errStatus);
+ throw new ExecutionStatusException(errStatus, resp.build());
+ }
+
+ return result;
}
- return finalResult;
}
// Returns true if the OS being run on is Windows (or some close approximation thereof).
