Remote: Allow disk cache with remote exec
This allows usage of remote execution and a local disk cache.
Also update the output when using disk and remote cache to indicate which is used.
The initial PR creating a combined disk and grpc cache mentioned issues with
findMissingDigests for DiskAndRemoteCacheClient, but I noticed a couple oddities and I'm not sure if I'm missing something. [https://cs.opensource.google/bazel/bazel/+/5f4d6995db1eb6a9d35dc163c0150283e830aa3d]
The original concern was that we should ignore the disk cache and only pay attention to the remote cache when using DiskAndRemoteCacheClient with remote exec. This makes sense as bazel has to ensure the blobs are available remotely.
However, the code in findMissingDigests returns the union of all missing digests (both disk and remote). If it short-circuited the check by only checking the disk cache and only returned the disk-cache result if non-empty, I would understand the concern. But the current code returns the union of the disk-cache result and the remote-cache result.
Additionally the current code for DiskCacheClient#findMissingBlobs unconditionally returns _all_ digests as missing. So in essence, the current code is always returning _all_ digests as missing. This seems like a bug due to optimizations.
[I'm fixing this in the DiskAndRemoteCacheClient by only calling the remote for find Missing when doing remoteExec]
A test to resolve the concern would be to run remote action (which would populate both disk and remote cache).
* verify it is in the disk cache.
* clear the remote cache of both action cache and blobs
* clear disk cache of blobs, but not action cache
* run the action again.
Another concern is the current code won't upload to the remote caches (specifically the disk cache) when the remote_exec happens.
The general assumption is that most remote_exec engines will populate the remote cache themselves so currently there isn't a call to `remoteExecutionService.uploadOutputs` when doing remote exec. It is only there for local spawns.
Since the remote disk cache with remote exec is currently disabled, this hasn't come up. The next time the action is attempted, it will be found in the remote_cache and pulled down. With the current PR, the disk_cache will get populated when the ActionResult is pulled from the remote_cache on a future run. I think that is Okay.
Testing:
sh_tests were added to go through scenarios of having the ActionCache prepopulated for disk and remote and not at all.
Closes #13852.
PiperOrigin-RevId: 393106615
diff --git a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java
index a1d74b6..b499c06 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/GrpcCacheClient.java
@@ -234,9 +234,12 @@
callCredentialsProvider);
}
- private ListenableFuture<ActionResult> handleStatus(ListenableFuture<ActionResult> download) {
+ private ListenableFuture<CachedActionResult> handleStatus(
+ ListenableFuture<ActionResult> download) {
+ ListenableFuture<CachedActionResult> cachedActionResult =
+ Futures.transform(download, CachedActionResult::remote, MoreExecutors.directExecutor());
return Futures.catchingAsync(
- download,
+ cachedActionResult,
StatusRuntimeException.class,
(sre) ->
sre.getStatus().getCode() == Code.NOT_FOUND
@@ -247,7 +250,7 @@
}
@Override
- public ListenableFuture<ActionResult> downloadActionResult(
+ public ListenableFuture<CachedActionResult> downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) {
GetActionResultRequest request =
GetActionResultRequest.newBuilder()
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
index a6c5330..a61b1ef 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteCache.java
@@ -45,6 +45,7 @@
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
+import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult;
import com.google.devtools.build.lib.remote.common.RemotePathResolver;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.util.DigestUtil;
@@ -99,7 +100,7 @@
this.digestUtil = digestUtil;
}
- public ActionResult downloadActionResult(
+ public CachedActionResult downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr)
throws IOException, InterruptedException {
return getFromFuture(cacheProtocol.downloadActionResult(context, actionKey, inlineOutErr));
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
index 8d3adf6..3ce4d78 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -85,6 +85,7 @@
import com.google.devtools.build.lib.remote.common.OutputDigestMismatchException;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
+import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult;
import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
import com.google.devtools.build.lib.remote.common.RemotePathResolver;
import com.google.devtools.build.lib.remote.merkletree.MerkleTree;
@@ -375,23 +376,28 @@
public static class RemoteActionResult {
private final ActionResult actionResult;
@Nullable private final ExecuteResponse executeResponse;
+ @Nullable private final String cacheName;
/** Creates a new {@link RemoteActionResult} instance from a cached result. */
- public static RemoteActionResult createFromCache(ActionResult cachedActionResult) {
+ public static RemoteActionResult createFromCache(CachedActionResult cachedActionResult) {
checkArgument(cachedActionResult != null, "cachedActionResult is null");
- return new RemoteActionResult(cachedActionResult, null);
+ return new RemoteActionResult(
+ cachedActionResult.actionResult(), null, cachedActionResult.cacheName());
}
/** Creates a new {@link RemoteActionResult} instance from a execute response. */
public static RemoteActionResult createFromResponse(ExecuteResponse response) {
checkArgument(response.hasResult(), "response doesn't have result");
- return new RemoteActionResult(response.getResult(), response);
+ return new RemoteActionResult(response.getResult(), response, /* cacheName */ null);
}
public RemoteActionResult(
- ActionResult actionResult, @Nullable ExecuteResponse executeResponse) {
+ ActionResult actionResult,
+ @Nullable ExecuteResponse executeResponse,
+ @Nullable String cacheName) {
this.actionResult = actionResult;
this.executeResponse = executeResponse;
+ this.cacheName = cacheName;
}
/** Returns the exit code of remote executed action. */
@@ -452,6 +458,12 @@
return executeResponse.getCachedResult();
}
+ /** Returns cache name (disk/remote) when {@code cacheHit()} or {@code null} when not */
+ @Nullable
+ public String cacheName() {
+ return cacheName;
+ }
+
/**
* Returns the underlying {@link ExecuteResponse} or {@code null} if this result is from a
* cache.
@@ -485,15 +497,15 @@
throws IOException, InterruptedException {
checkState(shouldAcceptCachedResult(action.spawn), "spawn doesn't accept cached result");
- ActionResult actionResult =
+ CachedActionResult cachedActionResult =
remoteCache.downloadActionResult(
action.remoteActionExecutionContext, action.actionKey, /* inlineOutErr= */ false);
- if (actionResult == null) {
+ if (cachedActionResult == null) {
return null;
}
- return RemoteActionResult.createFromCache(actionResult);
+ return RemoteActionResult.createFromCache(cachedActionResult);
}
private static Path toTmpDownloadPath(Path actualPath) {
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
index bc0394b..1015128 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -274,9 +274,9 @@
return;
}
- if ((enableHttpCache || enableDiskCache) && enableRemoteExecution) {
+ if (enableHttpCache && enableRemoteExecution) {
throw createOptionsExitException(
- "Cannot combine gRPC based remote execution with disk caching or HTTP-based caching",
+ "Cannot combine gRPC based remote execution with HTTP-based caching",
FailureDetails.RemoteOptions.Code.EXECUTION_WITH_INVALID_CACHE);
}
@@ -504,6 +504,22 @@
uploader, cacheClient, remoteBytestreamUriPrefix, buildRequestId, invocationId));
if (enableRemoteExecution) {
+ if (enableDiskCache) {
+ try {
+ cacheClient =
+ RemoteCacheClientFactory.createDiskAndRemoteClient(
+ env.getWorkingDirectory(),
+ remoteOptions.diskCache,
+ remoteOptions.remoteVerifyDownloads,
+ digestUtil,
+ cacheClient,
+ remoteOptions);
+ } catch (IOException e) {
+ handleInitFailure(env, e, Code.CACHE_INIT_FAILURE);
+ return;
+ }
+ }
+
RemoteExecutionClient remoteExecutor;
if (remoteOptions.remoteExecutionKeepalive) {
RemoteRetrier execRetrier =
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java
index ba889b9..546ec79 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java
@@ -34,6 +34,7 @@
import com.google.devtools.build.lib.remote.common.OperationObserver;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
+import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult;
import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
import com.google.devtools.build.lib.remote.merkletree.MerkleTree;
import com.google.devtools.build.lib.remote.util.DigestUtil;
@@ -134,10 +135,15 @@
buildAction(commandHash, merkleTree.getRootDigest(), platform, timeout, acceptCached);
Digest actionDigest = digestUtil.compute(action);
ActionKey actionKey = new ActionKey(actionDigest);
- ActionResult actionResult;
+ CachedActionResult cachedActionResult;
try (SilentCloseable c =
Profiler.instance().profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) {
- actionResult = remoteCache.downloadActionResult(context, actionKey, /* inlineOutErr= */ true);
+ cachedActionResult =
+ remoteCache.downloadActionResult(context, actionKey, /* inlineOutErr= */ true);
+ }
+ ActionResult actionResult = null;
+ if (cachedActionResult != null) {
+ actionResult = cachedActionResult.actionResult();
}
if (actionResult == null || actionResult.getExitCode() != 0) {
try (SilentCloseable c =
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
index da86225..d72bb34 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -132,7 +132,7 @@
createSpawnResult(
result.getExitCode(),
/*cacheHit=*/ true,
- "remote",
+ result.cacheName(),
inMemoryOutput,
spawnMetrics.build(),
spawn.getMnemonic());
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index 653559fb..31ed039 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -195,6 +195,7 @@
try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) {
cachedResult = acceptCachedResult ? remoteExecutionService.lookupCache(action) : null;
}
+
if (cachedResult != null) {
if (cachedResult.getExitCode() != 0) {
// Failed actions are treated as a cache miss mostly in order to avoid caching flaky
@@ -207,6 +208,7 @@
action,
cachedResult,
/* cacheHit= */ true,
+ cachedResult.cacheName(),
spawn,
totalTime,
() -> action.getNetworkTime().getDuration(),
@@ -273,6 +275,7 @@
action,
result,
result.cacheHit(),
+ getName(),
spawn,
totalTime,
() -> action.getNetworkTime().getDuration(),
@@ -340,6 +343,7 @@
RemoteAction action,
RemoteActionResult result,
boolean cacheHit,
+ String cacheName,
Spawn spawn,
Stopwatch totalTime,
Supplier<Duration> networkTime,
@@ -361,7 +365,7 @@
return createSpawnResult(
result.getExitCode(),
cacheHit,
- getName(),
+ cacheName,
inMemoryOutput,
spawnMetrics
.setFetchTime(fetchTime.elapsed().minus(networkTimeEnd.minus(networkTimeStart)))
diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD
index 3049a42..1926df1 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD
@@ -22,6 +22,7 @@
"//src/main/java/com/google/devtools/build/lib/exec:spawn_runner",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
+ "//third_party:auto_value",
"//third_party:guava",
"//third_party:jsr305",
"//third_party/protobuf:protobuf_java",
diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java
index 628d214..e0bcd8f 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemoteCacheClient.java
@@ -17,6 +17,7 @@
import build.bazel.remote.execution.v2.Action;
import build.bazel.remote.execution.v2.ActionResult;
import build.bazel.remote.execution.v2.Digest;
+import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.vfs.Path;
@@ -64,6 +65,32 @@
}
/**
+ * Class to keep track of which cache (disk or remote) a given [cached] ActionResult comes from.
+ */
+ @AutoValue
+ abstract class CachedActionResult {
+ public static CachedActionResult remote(ActionResult actionResult) {
+ if (actionResult == null) {
+ return null;
+ }
+ return new AutoValue_RemoteCacheClient_CachedActionResult(actionResult, "remote");
+ }
+
+ public static CachedActionResult disk(ActionResult actionResult) {
+ if (actionResult == null) {
+ return null;
+ }
+ return new AutoValue_RemoteCacheClient_CachedActionResult(actionResult, "disk");
+ }
+
+ /** A actionResult can have a cache name ascribed to it. */
+ public abstract ActionResult actionResult();
+
+ /** Indicates which cache the {@link #actionResult} came from (disk/remote) */
+ public abstract String cacheName();
+ }
+
+ /**
* Downloads an action result for the {@code actionKey}.
*
* @param context the context for the action.
@@ -73,7 +100,7 @@
* @return A Future representing pending download of an action result. If an action result for
* {@code actionKey} cannot be found the result of the Future is {@code null}.
*/
- ListenableFuture<ActionResult> downloadActionResult(
+ ListenableFuture<CachedActionResult> downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr);
/**
diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java
index c6289de..cf4b8b3 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskAndRemoteCacheClient.java
@@ -103,6 +103,14 @@
@Override
public ListenableFuture<ImmutableSet<Digest>> findMissingDigests(
RemoteActionExecutionContext context, Iterable<Digest> digests) {
+ // If remote execution, find missing digests should only look at
+ // the remote cache, not the disk cache because the remote executor only
+ // has access to the remote cache, not the disk cache.
+ // Also, the DiskCache always returns all digests as missing
+ // and we don't want to transfer all the files all the time.
+ if (options.isRemoteExecutionEnabled()) {
+ return remoteCache.findMissingDigests(context, digests);
+ }
ListenableFuture<ImmutableSet<Digest>> diskQuery =
diskCache.findMissingDigests(context, digests);
if (shouldUploadLocalResultsToRemoteCache(options, context.getSpawn())) {
@@ -173,7 +181,7 @@
}
@Override
- public ListenableFuture<ActionResult> downloadActionResult(
+ public ListenableFuture<CachedActionResult> downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) {
if (diskCache.containsActionResult(actionKey)) {
return diskCache.downloadActionResult(context, actionKey, inlineOutErr);
@@ -182,12 +190,12 @@
if (shouldAcceptCachedResultFromRemoteCache(options, context.getSpawn())) {
return Futures.transformAsync(
remoteCache.downloadActionResult(context, actionKey, inlineOutErr),
- (actionResult) -> {
- if (actionResult == null) {
+ (cachedActionResult) -> {
+ if (cachedActionResult == null) {
return Futures.immediateFuture(null);
} else {
- diskCache.uploadActionResult(context, actionKey, actionResult);
- return Futures.immediateFuture(actionResult);
+ diskCache.uploadActionResult(context, actionKey, cachedActionResult.actionResult());
+ return Futures.immediateFuture(cachedActionResult);
}
},
MoreExecutors.directExecutor());
diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java
index 26ce20a..53649c8 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheClient.java
@@ -102,10 +102,13 @@
}
@Override
- public ListenableFuture<ActionResult> downloadActionResult(
+ public ListenableFuture<CachedActionResult> downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) {
- return Utils.downloadAsActionResult(
- actionKey, (digest, out) -> download(digest, out, /* isActionCache= */ true));
+ return Futures.transform(
+ Utils.downloadAsActionResult(
+ actionKey, (digest, out) -> download(digest, out, /* isActionCache= */ true)),
+ CachedActionResult::disk,
+ MoreExecutors.directExecutor());
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java
index 1efecd3..1c9e2ec 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/http/HttpCacheClient.java
@@ -573,10 +573,13 @@
}
@Override
- public ListenableFuture<ActionResult> downloadActionResult(
+ public ListenableFuture<CachedActionResult> downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) {
- return Utils.downloadAsActionResult(
- actionKey, (digest, out) -> get(digest, out, /* casDownload= */ false));
+ return Futures.transform(
+ Utils.downloadAsActionResult(
+ actionKey, (digest, out) -> get(digest, out, /* casDownload= */ false)),
+ CachedActionResult::remote,
+ MoreExecutors.directExecutor());
}
@SuppressWarnings("FutureReturnValueIgnored")
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/SpawnStats.java b/src/main/java/com/google/devtools/build/lib/runtime/SpawnStats.java
index 1d241bf..e24a12b 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/SpawnStats.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/SpawnStats.java
@@ -31,7 +31,8 @@
/** Collects results from SpawnResult. */
@ThreadSafe
public class SpawnStats {
- private static final ImmutableList<String> REPORT_FIRST = ImmutableList.of("remote cache hit");
+ private static final ImmutableList<String> REPORT_FIRST =
+ ImmutableList.of("disk cache hit", "remote cache hit");
private final ConcurrentHashMultiset<String> runners = ConcurrentHashMultiset.create();
private final AtomicLong totalWallTimeMillis = new AtomicLong();
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
index 33f7fff..aa3ea86 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
@@ -64,6 +64,7 @@
import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteAction;
import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
+import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult;
import com.google.devtools.build.lib.remote.common.RemotePathResolver;
import com.google.devtools.build.lib.remote.common.RemotePathResolver.DefaultRemotePathResolver;
import com.google.devtools.build.lib.remote.common.RemotePathResolver.SiblingRepositoryLayoutResolver;
@@ -146,7 +147,8 @@
.setPath("outputs/bar")
.setDigest(barDigest)
.setIsExecutable(true);
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -173,7 +175,8 @@
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputFilesBuilder().setPath("execroot/outputs/foo").setDigest(fooDigest);
builder.addOutputFilesBuilder().setPath("execroot/outputs/bar").setDigest(barDigest);
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -210,7 +213,8 @@
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputFilesBuilder().setPath("outputs/a/foo").setDigest(fooDigest);
builder.addOutputDirectoriesBuilder().setPath("outputs/a/bar").setTreeDigest(barTreeDigest);
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -236,7 +240,8 @@
cache.addContents(remoteActionExecutionContext, barTreeMessage.toByteArray());
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputDirectoriesBuilder().setPath("outputs/a/bar").setTreeDigest(barTreeDigest);
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -281,7 +286,8 @@
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputFilesBuilder().setPath("outputs/a/foo").setDigest(fooDigest);
builder.addOutputDirectoriesBuilder().setPath("outputs/a/bar").setTreeDigest(barTreeDigest);
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -331,7 +337,8 @@
Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray());
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputDirectoriesBuilder().setPath("outputs/a/").setTreeDigest(treeDigest);
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -352,7 +359,8 @@
public void downloadOutputs_relativeFileSymlink_success() throws Exception {
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputFileSymlinksBuilder().setPath("outputs/a/b/link").setTarget("../../foo");
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -371,7 +379,8 @@
public void downloadOutputs_relativeDirectorySymlink_success() throws Exception {
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputDirectorySymlinksBuilder().setPath("outputs/a/b/link").setTarget("foo");
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -397,7 +406,8 @@
Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray());
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputDirectoriesBuilder().setPath("outputs/dir").setTreeDigest(treeDigest);
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -416,7 +426,8 @@
public void downloadOutputs_absoluteFileSymlink_error() throws Exception {
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputFileSymlinksBuilder().setPath("outputs/foo").setTarget("/abs/link");
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -434,7 +445,8 @@
public void downloadOutputs_absoluteDirectorySymlink_error() throws Exception {
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputDirectorySymlinksBuilder().setPath("outputs/foo").setTarget("/abs/link");
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -459,7 +471,8 @@
Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray());
ActionResult.Builder builder = ActionResult.newBuilder();
builder.addOutputDirectoriesBuilder().setPath("outputs/dir").setTreeDigest(treeDigest);
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -492,7 +505,8 @@
.setDigest(outputFileDigest));
builder.addOutputFiles(
OutputFile.newBuilder().setPath("outputs/otherfile").setDigest(otherFileDigest));
- RemoteActionResult result = RemoteActionResult.createFromCache(builder.build());
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(builder.build()));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -522,7 +536,8 @@
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file2").setDigest(digest2))
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file3").setDigest(digest3))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(actionResult);
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -552,7 +567,8 @@
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file2").setDigest(digest2))
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file3").setDigest(digest3))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(actionResult);
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -581,7 +597,8 @@
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file2").setDigest(digest2))
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file3").setDigest(digest3))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(actionResult);
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -610,7 +627,8 @@
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file2").setDigest(digest2))
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file3").setDigest(digest3))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(actionResult);
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -639,7 +657,8 @@
.setStdoutDigest(digestStdout)
.setStderrDigest(digestStderr)
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(actionResult);
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn, spyOutErr);
RemoteExecutionService service = newRemoteExecutionService();
@@ -678,7 +697,8 @@
.setStdoutDigest(digestStdout)
.setStderrDigest(digestStderr)
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(actionResult);
+ RemoteActionResult result =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn, spyOutErr);
RemoteExecutionService service = newRemoteExecutionService();
@@ -710,7 +730,7 @@
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/foo.tmp").setDigest(d1))
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/foo").setDigest(d2))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(r);
+ RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
Spawn spawn = newSpawnFromResult(result);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn);
RemoteExecutionService service = newRemoteExecutionService();
@@ -735,7 +755,7 @@
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file1").setDigest(d1))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(r);
+ RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
Spawn spawn = newSpawnFromResult(result);
MetadataInjector injector = mock(MetadataInjector.class);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn, injector);
@@ -764,7 +784,7 @@
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file1").setDigest(d1))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(r);
+ RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
Spawn spawn = newSpawnFromResult(result);
MetadataInjector injector = mock(MetadataInjector.class);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn, injector);
@@ -799,7 +819,7 @@
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file2").setDigest(d2))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(r);
+ RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
Spawn spawn = newSpawnFromResult(result);
MetadataInjector injector = mock(MetadataInjector.class);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn, injector);
@@ -851,7 +871,7 @@
OutputDirectory.newBuilder().setPath("outputs/dir").setTreeDigest(dt))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(r);
+ RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
Spawn spawn = newSpawnFromResult(result);
MetadataInjector injector = mock(MetadataInjector.class);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn, injector);
@@ -918,7 +938,7 @@
OutputDirectory.newBuilder().setPath("outputs/dir").setTreeDigest(dt))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(r);
+ RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
Spawn spawn = newSpawnFromResult(result);
MetadataInjector injector = mock(MetadataInjector.class);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn, injector);
@@ -951,7 +971,7 @@
.setStderrDigest(dErr)
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(r);
+ RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
Spawn spawn = newSpawnFromResult(result);
MetadataInjector injector = mock(MetadataInjector.class);
FakeSpawnExecutionContext context = newSpawnExecutionContext(spawn, injector);
@@ -986,7 +1006,7 @@
.addOutputFiles(OutputFile.newBuilder().setPath("outputs/file2").setDigest(d2))
.build();
- RemoteActionResult result = RemoteActionResult.createFromCache(r);
+ RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
// a1 should be provided as an InMemoryOutput
PathFragment inMemoryOutputPathFragment = PathFragment.create("outputs/file1");
Spawn spawn = newSpawnFromResultWithInMemoryOutput(result, inMemoryOutputPathFragment);
@@ -1023,7 +1043,7 @@
// arrange
Digest d1 = cache.addContents(remoteActionExecutionContext, "in-memory output");
ActionResult r = ActionResult.newBuilder().setExitCode(0).build();
- RemoteActionResult result = RemoteActionResult.createFromCache(r);
+ RemoteActionResult result = RemoteActionResult.createFromCache(CachedActionResult.remote(r));
Artifact a1 = ActionsTestUtil.createArtifact(artifactRoot, "file1");
Spawn spawn =
newSpawn(
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java
index 8133d5c..4d59b78 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutorTest.java
@@ -26,6 +26,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSortedMap;
+import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult;
import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor.ExecutionResult;
@@ -74,7 +75,7 @@
// Arrange
ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build();
when(remoteCache.downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true)))
- .thenReturn(cachedResult);
+ .thenReturn(CachedActionResult.remote(cachedResult));
// Act
ExecutionResult executionResult =
@@ -101,7 +102,7 @@
// Arrange
ActionResult cachedResult = ActionResult.newBuilder().setExitCode(1).build();
when(remoteCache.downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true)))
- .thenReturn(cachedResult);
+ .thenReturn(CachedActionResult.remote(cachedResult));
ExecuteResponse response = ExecuteResponse.newBuilder().setResult(cachedResult).build();
when(remoteExecutor.executeRemotely(any(), any(), any())).thenReturn(response);
@@ -138,7 +139,7 @@
.setStderrRaw(ByteString.copyFrom(stderr))
.build();
when(remoteCache.downloadActionResult(any(), any(), /* inlineOutErr= */ eq(true)))
- .thenReturn(cachedResult);
+ .thenReturn(CachedActionResult.remote(cachedResult));
ExecuteResponse response = ExecuteResponse.newBuilder().setResult(cachedResult).build();
when(remoteExecutor.executeRemotely(any(), any(), any())).thenReturn(response);
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
index a85d398d..7a073e0 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java
@@ -66,6 +66,7 @@
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
+import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult;
import com.google.devtools.build.lib.remote.common.RemotePathResolver;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.remote.options.RemoteOutputsMode;
@@ -258,14 +259,14 @@
any(ActionKey.class),
/* inlineOutErr= */ eq(false)))
.thenAnswer(
- new Answer<ActionResult>() {
+ new Answer<CachedActionResult>() {
@Override
- public ActionResult answer(InvocationOnMock invocation) {
+ public CachedActionResult answer(InvocationOnMock invocation) {
RemoteActionExecutionContext context = invocation.getArgument(0);
RequestMetadata meta = context.getRequestMetadata();
assertThat(meta.getCorrelatedInvocationsId()).isEqualTo(BUILD_REQUEST_ID);
assertThat(meta.getToolInvocationId()).isEqualTo(COMMAND_ID);
- return actionResult;
+ return CachedActionResult.remote(actionResult);
}
});
doAnswer(
@@ -279,7 +280,8 @@
return null;
})
.when(service)
- .downloadOutputs(any(), eq(RemoteActionResult.createFromCache(actionResult)));
+ .downloadOutputs(
+ any(), eq(RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult))));
// act
CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy);
@@ -288,7 +290,9 @@
// assert
// All other methods on RemoteActionCache have side effects, so we verify all of them.
- verify(service).downloadOutputs(any(), eq(RemoteActionResult.createFromCache(actionResult)));
+ verify(service)
+ .downloadOutputs(
+ any(), eq(RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult))));
verify(remoteCache, never())
.upload(
any(RemoteActionExecutionContext.class),
@@ -642,19 +646,20 @@
any(ActionKey.class),
/* inlineOutErr= */ eq(false)))
.thenAnswer(
- new Answer<ActionResult>() {
+ new Answer<CachedActionResult>() {
@Override
- public ActionResult answer(InvocationOnMock invocation) {
+ public CachedActionResult answer(InvocationOnMock invocation) {
RemoteActionExecutionContext context = invocation.getArgument(0);
RequestMetadata meta = context.getRequestMetadata();
assertThat(meta.getCorrelatedInvocationsId()).isEqualTo(BUILD_REQUEST_ID);
assertThat(meta.getToolInvocationId()).isEqualTo(COMMAND_ID);
- return actionResult;
+ return CachedActionResult.remote(actionResult);
}
});
doThrow(new CacheNotFoundException(digest))
.when(cache.getRemoteExecutionService())
- .downloadOutputs(any(), eq(RemoteActionResult.createFromCache(actionResult)));
+ .downloadOutputs(
+ any(), eq(RemoteActionResult.createFromCache(CachedActionResult.remote(actionResult))));
CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy);
assertThat(entry.hasResult()).isFalse();
@@ -711,7 +716,7 @@
any(RemoteActionExecutionContext.class),
any(ActionKey.class),
/* inlineOutErr= */ eq(false)))
- .thenReturn(actionResult);
+ .thenReturn(CachedActionResult.remote(actionResult));
CacheHandle entry = cache.lookup(simpleSpawn, simplePolicy);
@@ -728,7 +733,7 @@
ActionResult success = ActionResult.newBuilder().setExitCode(0).build();
when(remoteCache.downloadActionResult(
any(RemoteActionExecutionContext.class), any(), /* inlineOutErr= */ eq(false)))
- .thenReturn(success);
+ .thenReturn(CachedActionResult.remote(success));
// act
CacheHandle cacheHandle = cache.lookup(simpleSpawn, simplePolicy);
@@ -737,7 +742,8 @@
assertThat(cacheHandle.hasResult()).isTrue();
assertThat(cacheHandle.getResult().exitCode()).isEqualTo(0);
verify(cache.getRemoteExecutionService())
- .downloadOutputs(any(), eq(RemoteActionResult.createFromCache(success)));
+ .downloadOutputs(
+ any(), eq(RemoteActionResult.createFromCache(CachedActionResult.remote(success))));
}
@Test
@@ -752,10 +758,11 @@
ActionResult success = ActionResult.newBuilder().setExitCode(0).build();
when(remoteCache.downloadActionResult(
any(RemoteActionExecutionContext.class), any(), /* inlineOutErr= */ eq(false)))
- .thenReturn(success);
+ .thenReturn(CachedActionResult.remote(success));
doThrow(downloadFailure)
.when(cache.getRemoteExecutionService())
- .downloadOutputs(any(), eq(RemoteActionResult.createFromCache(success)));
+ .downloadOutputs(
+ any(), eq(RemoteActionResult.createFromCache(CachedActionResult.remote(success))));
// act
CacheHandle cacheHandle = cache.lookup(simpleSpawn, simplePolicy);
@@ -763,7 +770,8 @@
// assert
assertThat(cacheHandle.hasResult()).isFalse();
verify(cache.getRemoteExecutionService())
- .downloadOutputs(any(), eq(RemoteActionResult.createFromCache(success)));
+ .downloadOutputs(
+ any(), eq(RemoteActionResult.createFromCache(CachedActionResult.remote(success))));
assertThat(eventHandler.getEvents().size()).isEqualTo(1);
Event evt = eventHandler.getEvents().get(0);
assertThat(evt.getKind()).isEqualTo(EventKind.WARNING);
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
index 6ecd3f8..a234a711 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
@@ -83,6 +83,7 @@
import com.google.devtools.build.lib.remote.common.OperationObserver;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
+import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult;
import com.google.devtools.build.lib.remote.common.RemoteExecutionClient;
import com.google.devtools.build.lib.remote.common.RemotePathResolver;
import com.google.devtools.build.lib.remote.common.RemotePathResolver.SiblingRepositoryLayoutResolver;
@@ -340,7 +341,8 @@
remoteOptions.remoteLocalFallback = true;
remoteOptions.remoteUploadLocalResults = true;
- ActionResult failedAction = ActionResult.newBuilder().setExitCode(1).build();
+ CachedActionResult failedAction =
+ CachedActionResult.remote(ActionResult.newBuilder().setExitCode(1).build());
when(cache.downloadActionResult(
any(RemoteActionExecutionContext.class),
any(ActionKey.class),
@@ -380,7 +382,8 @@
// Test that bazel treats failed cache action as a cache miss and attempts to execute action
// remotely
- ActionResult failedAction = ActionResult.newBuilder().setExitCode(1).build();
+ CachedActionResult failedAction =
+ CachedActionResult.remote(ActionResult.newBuilder().setExitCode(1).build());
when(cache.downloadActionResult(
any(RemoteActionExecutionContext.class),
any(ActionKey.class),
@@ -746,7 +749,8 @@
// arrange
RemoteSpawnRunner runner = newSpawnRunner();
- ActionResult cachedResult = ActionResult.newBuilder().setExitCode(0).build();
+ CachedActionResult cachedResult =
+ CachedActionResult.remote(ActionResult.newBuilder().setExitCode(0).build());
when(cache.downloadActionResult(
any(RemoteActionExecutionContext.class),
any(ActionKey.class),
@@ -1129,10 +1133,13 @@
remoteOptions.remoteOutputsMode = RemoteOutputsMode.MINIMAL;
ActionResult succeededAction = ActionResult.newBuilder().setExitCode(0).build();
- RemoteActionResult actionResult = RemoteActionResult.createFromCache(succeededAction);
+ RemoteActionResult actionResult =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(succeededAction));
RemoteSpawnRunner runner = newSpawnRunner();
- doReturn(actionResult).when(service).lookupCache(any());
+ doReturn(RemoteActionResult.createFromCache(CachedActionResult.remote(succeededAction)))
+ .when(service)
+ .lookupCache(any());
Spawn spawn = newSimpleSpawn();
SpawnExecutionContext policy = getSpawnContext(spawn);
@@ -1182,12 +1189,15 @@
remoteOptions.remoteOutputsMode = RemoteOutputsMode.MINIMAL;
ActionResult succeededAction = ActionResult.newBuilder().setExitCode(0).build();
- RemoteActionResult cachedActionResult = RemoteActionResult.createFromCache(succeededAction);
+ RemoteActionResult cachedActionResult =
+ RemoteActionResult.createFromCache(CachedActionResult.remote(succeededAction));
IOException downloadFailure = new IOException("downloadMinimal failed");
RemoteSpawnRunner runner = newSpawnRunner();
- doReturn(cachedActionResult).when(service).lookupCache(any());
+ doReturn(RemoteActionResult.createFromCache(CachedActionResult.remote(succeededAction)))
+ .when(service)
+ .lookupCache(any());
doThrow(downloadFailure).when(service).downloadOutputs(any(), eq(cachedActionResult));
Spawn spawn = newSimpleSpawn();
@@ -1211,8 +1221,9 @@
Artifact topLevelOutput =
ActionsTestUtil.createArtifact(outputRoot, outputRoot.getRoot().getRelative("foo.bin"));
- ActionResult succeededAction = ActionResult.newBuilder().setExitCode(0).build();
- RemoteActionResult cachedActionResult = RemoteActionResult.createFromCache(succeededAction);
+ RemoteActionResult cachedActionResult =
+ RemoteActionResult.createFromCache(
+ CachedActionResult.remote(ActionResult.newBuilder().setExitCode(0).build()));
RemoteSpawnRunner runner = newSpawnRunner(ImmutableSet.of(topLevelOutput));
doReturn(cachedActionResult).when(service).lookupCache(any());
diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java
index b5ce3ee..29eb4b2 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/util/InMemoryCacheClient.java
@@ -91,13 +91,13 @@
}
@Override
- public ListenableFuture<ActionResult> downloadActionResult(
+ public ListenableFuture<CachedActionResult> downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) {
ActionResult actionResult = ac.get(actionKey);
if (actionResult == null) {
return Futures.immediateFailedFuture(new CacheNotFoundException(actionKey.getDigest()));
}
- return Futures.immediateFuture(actionResult);
+ return Futures.immediateFuture(CachedActionResult.remote(actionResult));
}
@Override
diff --git a/src/test/shell/bazel/remote/remote_execution_http_test.sh b/src/test/shell/bazel/remote/remote_execution_http_test.sh
index 688444a..262170f 100755
--- a/src/test/shell/bazel/remote/remote_execution_http_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_http_test.sh
@@ -389,7 +389,7 @@
bazel clean
bazel build $disk_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_upload_local_results &> $TEST_log \
|| fail "Failed to fetch //a:test from disk cache"
- expect_log "1 remote cache hit" "Fetch from disk cache failed"
+ expect_log "1 disk cache hit" "Fetch from disk cache failed"
diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
|| fail "Disk cache generated different result"
@@ -427,7 +427,7 @@
bazel clean
bazel build $disk_flags $http_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_accept_cached &> $TEST_log \
|| fail "Failed to build //a:test"
- expect_log "1 remote cache hit" "Fetch from disk cache failed"
+ expect_log "1 disk cache hit" "Fetch from disk cache failed"
diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
|| fail "Disk cache generated different result"
@@ -445,7 +445,7 @@
bazel clean
bazel build $disk_flags //a:test &> $TEST_log \
|| fail "Failed to fetch //a:test from disk cache"
- expect_log "1 remote cache hit" "Fetch from disk cache failed"
+ expect_log "1 disk cache hit" "Fetch from disk cache failed"
diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
|| fail "Disk cache generated different result"
@@ -472,7 +472,7 @@
bazel clean
bazel build $disk_flags //a:test &> $TEST_log \
|| fail "Failed to fetch //a:test from disk cache"
- expect_log "1 remote cache hit" "Fetch from disk cache after copy from http cache failed"
+ expect_log "1 disk cache hit" "Fetch from disk cache after copy from http cache failed"
diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
|| fail "Disk cache generated different result"
diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
index aa2198c..93c2864 100755
--- a/src/test/shell/bazel/remote/remote_execution_test.sh
+++ b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -1926,7 +1926,7 @@
--disk_cache=$CACHEDIR \
//a:foo >& $TEST_log || "Failed to build //a:foo"
- expect_log "1 remote cache hit"
+ expect_log "1 disk cache hit"
}
function test_tag_no_remote_exec() {
@@ -2058,6 +2058,140 @@
expect_log "Setting both --remote_default_platform_properties and --remote_default_exec_properties is not allowed"
}
+
+function test_genrule_combined_disk_remote_exec() {
+ # Test for the combined disk and grpc cache with remote_exec
+ local cache="${TEST_TMPDIR}/disk_cache"
+ local disk_flags="--disk_cache=$cache"
+ local grpc_flags="--remote_cache=grpc://localhost:${worker_port}"
+ local remote_exec_flags="--remote_executor=grpc://localhost:${worker_port}"
+
+ # if exist in disk cache or remote cache, don't run remote exec, don't update caches.
+ # [CASE]disk_cache, remote_cache: remote_exec, disk_cache, remote_cache
+ # 1) notexist notexist run OK - , update
+ # 2) notexist exist no run update, no update
+ # 3) exist notexist no run no update, no update
+ # 4) exist exist no run no update, no update
+ # 5) another rule that depends on 4, but run before 5
+ # Our setup ensures the first 2 columns, our validation checks the last 3.
+ # NOTE that remote_exec will NOT update the disk cache, we expect the remote
+ # execution to update the remote_cache and when we pull from the remote cache
+ # we will then mirror to the disk cache.
+ #
+ # We measure if it was run remotely via the "1 remote." in the output and caches
+ # from the cache hit on the same line.
+
+ # https://cs.opensource.google/bazel/bazel/+/master:third_party/remoteapis/build/bazel/remote/execution/v2/remote_execution.proto;l=447;drc=29ac010f3754c308de2ff13d3480b870dc7cb7f6
+ #
+ # Also test with these flags.
+ # flags:
+ # --noremote_upload_local_results
+ # --noremote_accept_cached
+ # --incompatible_remote_results_ignore_disk=true
+ #
+ # tags: [nocache, noremoteexec]
+ mkdir -p a
+ cat > a/BUILD <<'EOF'
+package(default_visibility = ["//visibility:public"])
+genrule(
+ name = 'test',
+ cmd = 'echo "Hello world" > $@',
+ outs = ['test.txt'],
+)
+
+genrule(
+ name = 'test2',
+ srcs = [':test'],
+ cmd = 'cat $(SRCS) > $@',
+ outs = ['test2.txt'],
+)
+EOF
+ rm -rf $cache
+ mkdir $cache
+
+ # Case 1)
+ # disk_cache, remote_cache: remote_exec, disk_cache, remote_cache
+ # notexist notexist run OK - , update
+ #
+ # Do a build to populate the disk and remote cache.
+ # Then clean and do another build to validate nothing updates.
+ bazel build --spawn_strategy=remote --genrule_strategy=remote $remote_exec_flags $grpc_flags $disk_flags //a:test &> $TEST_log \
+ || fail "CASE 1 Failed to build"
+
+ echo "Hello world" > ${TEST_TMPDIR}/test_expected
+ expect_log "2 processes: 1 internal, 1 remote." "CASE 1: unexpected action line [[$(grep processes $TEST_log)]]"
+
+ diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
+ || fail "Disk cache generated different result [$(cat bazel-genfiles/a/test.txt)] [$(cat $TEST_TEMPDIR/test_expected)]"
+
+ disk_action_cache_files="$(count_disk_ac_files "$cache")"
+ remote_action_cache_files="$(count_remote_ac_files)"
+
+ [[ "$disk_action_cache_files" == 0 ]] || fail "Expected 0 disk action cache entries, not $disk_action_cache_files"
+ # Even though bazel isn't writing the remote action cache, we expect the worker to write one or the
+ # the rest of our tests will fail.
+ [[ "$remote_action_cache_files" == 1 ]] || fail "Expected 1 remote action cache entries, not $remote_action_cache_files"
+
+ # Case 2)
+ # disk_cache, remote_cache: remote_exec, disk_cache, remote_cache
+ # notexist exist no run update, no update
+ bazel clean
+ bazel build --spawn_strategy=remote --genrule_strategy=remote $remote_exec_flags $grpc_flags $disk_flags //a:test &> $TEST_log \
+ || fail "CASE 2 Failed to build"
+ expect_log "2 processes: 1 remote cache hit, 1 internal." "CASE 2: unexpected action line [[$(grep processes $TEST_log)]]"
+
+ # ensure disk and remote cache populated
+ disk_action_cache_files="$(count_disk_ac_files "$cache")"
+ remote_action_cache_files="$(count_remote_ac_files)"
+ [[ "$disk_action_cache_files" == 1 ]] || fail "Expected 1 disk action cache entries, not $disk_action_cache_files"
+ [[ "$remote_action_cache_files" == 1 ]] || fail "Expected 1 remote action cache entries, not $remote_action_cache_files"
+
+ # Case 3)
+ # disk_cache, remote_cache: remote_exec, disk_cache, remote_cache
+ # exist notexist no run no update, no update
+ # stop the worker to clear the remote cache and then restart it.
+ # This ensures that if we hit the disk cache and it returns valid values
+ # for FindMissingBLobs, the remote exec can still find it from the remote cache.
+
+ stop_worker
+ start_worker
+ # need to reset flags after restarting worker [on new port]
+ local grpc_flags="--remote_cache=grpc://localhost:${worker_port}"
+ local remote_exec_flags="--remote_executor=grpc://localhost:${worker_port}"
+
+ bazel clean
+ bazel build --spawn_strategy=remote --genrule_strategy=remote $remote_exec_flags $grpc_flags $disk_flags //a:test &> $TEST_log \
+ || fail "CASE 3 failed to build"
+ expect_log "2 processes: 1 disk cache hit, 1 internal." "CASE 3: unexpected action line [[$(grep processes $TEST_log)]]"
+
+ # Case 4)
+ # disk_cache, remote_cache: remote_exec, disk_cache, remote_cache
+ # exist exist no run no update, no update
+
+ # This one is not interesting after case 3.
+ bazel clean
+ bazel build --spawn_strategy=remote --genrule_strategy=remote $remote_exec_flags $grpc_flags $disk_flags //a:test &> $TEST_log \
+ || fail "CASE 4 failed to build"
+ expect_log "2 processes: 1 disk cache hit, 1 internal." "CASE 4: unexpected action line [[$(grep processes $TEST_log)]]"
+
+
+ # One last slightly more complicated case.
+ # Build a target that depended on the last target but we clean and clear the remote cache.
+ # We should get one cache hit from disk and and one remote exec.
+
+ stop_worker
+ start_worker
+ # reset port
+ local grpc_flags="--remote_cache=grpc://localhost:${worker_port}"
+ local remote_exec_flags="--remote_executor=grpc://localhost:${worker_port}"
+
+ bazel clean
+ bazel build --spawn_strategy=remote --genrule_strategy=remote $remote_exec_flags $grpc_flags $disk_flags //a:test2 &> $TEST_log \
+ || fail "CASE 5 failed to build //a:test2"
+ expect_log "3 processes: 1 disk cache hit, 1 internal, 1 remote." "CASE 5: unexpected action line [[$(grep processes $TEST_log)]]"
+}
+
+
function test_genrule_combined_disk_grpc_cache() {
# Test for the combined disk and grpc cache.
# Built items should be pushed to both the disk and grpc cache.
@@ -2093,7 +2227,7 @@
bazel clean
bazel build $disk_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_upload_local_results &> $TEST_log \
|| fail "Failed to fetch //a:test from disk cache"
- expect_log "1 remote cache hit" "Fetch from disk cache failed"
+ expect_log "1 disk cache hit" "Fetch from disk cache failed"
diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
|| fail "Disk cache generated different result"
@@ -2131,7 +2265,7 @@
bazel clean
bazel build $disk_flags $grpc_flags //a:test --incompatible_remote_results_ignore_disk=true --noremote_accept_cached &> $TEST_log \
|| fail "Failed to build //a:test"
- expect_log "1 remote cache hit" "Fetch from disk cache failed"
+ expect_log "1 disk cache hit" "Fetch from disk cache failed"
diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
|| fail "Disk cache generated different result"
@@ -2149,7 +2283,7 @@
bazel clean
bazel build $disk_flags //a:test &> $TEST_log \
|| fail "Failed to fetch //a:test from disk cache"
- expect_log "1 remote cache hit" "Fetch from disk cache failed"
+ expect_log "1 disk cache hit" "Fetch from disk cache failed"
diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
|| fail "Disk cache generated different result"
@@ -2176,7 +2310,7 @@
bazel clean
bazel build $disk_flags //a:test &> $TEST_log \
|| fail "Failed to fetch //a:test from disk cache"
- expect_log "1 remote cache hit" "Fetch from disk cache after copy from grpc cache failed"
+ expect_log "1 disk cache hit" "Fetch from disk cache after copy from grpc cache failed"
diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
|| fail "Disk cache generated different result"
@@ -2214,7 +2348,7 @@
bazel clean
bazel build $disk_flags //a:test --incompatible_remote_results_ignore_disk=true &> $TEST_log \
|| fail "Failed to fetch //a:test from disk cache"
- expect_log "1 remote cache hit" "Fetch from disk cache failed"
+ expect_log "1 disk cache hit" "Fetch from disk cache failed"
diff bazel-genfiles/a/test.txt ${TEST_TMPDIR}/test_expected \
|| fail "Disk cache generated different result"
@@ -2897,4 +3031,4 @@
run_suite "Remote execution and remote cache tests"
-}
\ No newline at end of file
+}
diff --git a/src/test/shell/bazel/remote/remote_utils.sh b/src/test/shell/bazel/remote/remote_utils.sh
index ce01160..9a0191a 100644
--- a/src/test/shell/bazel/remote/remote_utils.sh
+++ b/src/test/shell/bazel/remote/remote_utils.sh
@@ -60,3 +60,21 @@
rm -rf "${cas_path}"
fi
}
+
+# Pass in the root of the disk cache and count number of files under /ac directory
+# output int to stdout
+function count_disk_ac_files() {
+ if [ -d "$1/ac" ]; then
+ expr $(find "$1/ac" -type f | wc -l)
+ else
+ echo 0
+ fi
+}
+
+function count_remote_ac_files() {
+ if [ -d "$cas_path/ac" ]; then
+ expr $(find "$cas_path/ac" -type f | wc -l)
+ else
+ echo 0
+ fi
+}
diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java
index 51972dd..f841791 100644
--- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java
+++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ActionCacheServer.java
@@ -23,6 +23,7 @@
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey;
+import com.google.devtools.build.lib.remote.common.RemoteCacheClient.CachedActionResult;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
import io.grpc.stub.StreamObserver;
@@ -47,7 +48,7 @@
RemoteActionExecutionContext context = RemoteActionExecutionContext.create(requestMetadata);
ActionKey actionKey = digestUtil.asActionKey(request.getActionDigest());
- ActionResult result =
+ CachedActionResult result =
cache.downloadActionResult(context, actionKey, /* inlineOutErr= */ false);
if (result == null) {
@@ -55,7 +56,7 @@
return;
}
- responseObserver.onNext(result);
+ responseObserver.onNext(result.actionResult());
responseObserver.onCompleted();
} catch (Exception e) {
logger.atWarning().withCause(e).log("getActionResult request failed");
