diff options
author | 2017-03-29 15:38:28 +0000 | |
---|---|---|
committer | 2017-03-29 19:28:35 +0200 | |
commit | 81940bd238de0f01c9adb5b075fb436f51baf42d (patch) | |
tree | 86208dbbb29d995d8f679c3458ec57ed8495c153 /src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java | |
parent | e1d692e486a2f838c3c894fd9de693fabd6685ed (diff) |
Clone the remote execution implementation into a new class
The new RemoteExecutionClient class only performs remote execution, and
nothing else; all higher-level functions, local rety, etc. will live outside
of the client.
In order to add unit tests, I had to add another layer of indirection between
the Grpc{RemoteExecutor,ActionCache} and GRPC, since GRPC generates final,
non-mockable classes. While a testing approach that uses a fake server can
also get some test coverage (as in GrpcActionCacheTest), it doesn't allow us
to test the full range of bad things that can happen at the GRPC layer.
The cloned implementation uses a single GRPC channel, as was recommended to
me by Jakob, who worked on GRPC. A single channel should be sufficiently
scalable, it's thread-safe, and it performs chunking internally. On the
server-side, the requests from a single channel can be dispatched to a thread
pool, so this should not be a blocker for server-side parallelism.
I also changed it to throw an exception whenever anything bad happens - this
makes it much more obvious if there's still bug in this code; the old code
silently swallows many errors, falling back to local execution, which papers
over many issues.
Furthermore, we now return a RemoteExecutionResult to indicate whether the
action ran at all (regardless of exit code), as well as the exit code.
All in all, this implementation is closer to the production code we're using
internally, although quite a few things are still missing.
The cloned implementation is not hooked up to RemoteSpawnStrategy yet. It
also does not support combining remote caching with local execution, but note
that RemoteSpawnStrategy regressed in that respect and currently also does
not support that mode.
PiperOrigin-RevId: 151578409
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java | 3 |
1 files changed, 1 insertions, 2 deletions
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 6d7e218773..6eb90fa4a1 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 @@ -130,8 +130,7 @@ public final class SimpleBlobStoreActionCache implements RemoteActionCache { } } - @Override - public void downloadFileContents(ContentDigest digest, Path dest, boolean executable) + private void downloadFileContents(ContentDigest digest, Path dest, boolean executable) throws IOException, CacheNotFoundException { // This unconditionally downloads the whole file into memory first! byte[] contents = downloadBlob(digest); |