Skip to content

Commit

Permalink
[7.1.0] Distinguish the disk and remote caches in the action progress…
Browse files Browse the repository at this point in the history
… status. (#21084)

Previously, they were both displayed as `remote-cache`; there's now a
separate `disk-cache` form. If a combined cache is used, one or both
forms will appear, depending on which caches were looked up.

As a result, the progress status reporting is moved to the individual
cache implementations. While this is kind of unfortunate from an
architectural standpoint, it's likely the best we can do until we recast
cache lookups as spawn strategies (see #19904).

Closes #20935.

PiperOrigin-RevId: 601748051
Change-Id: I03710219973c95d4fca999d931b3513f6d240d94
  • Loading branch information
tjgq authored Jan 26, 2024
1 parent 855aa90 commit 40019a8
Show file tree
Hide file tree
Showing 16 changed files with 297 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import com.google.common.util.concurrent.SettableFuture;
import com.google.devtools.build.lib.authandtls.CallCredentialsProvider;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent;
import com.google.devtools.build.lib.remote.RemoteRetrier.ProgressiveBackoff;
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
import com.google.devtools.build.lib.remote.common.MissingDigestsFinder;
Expand Down Expand Up @@ -81,6 +82,9 @@
public class GrpcCacheClient implements RemoteCacheClient, MissingDigestsFinder {
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();

private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT =
SpawnCheckingCacheEvent.create("remote-cache");

private final CallCredentialsProvider callCredentialsProvider;
private final ReferenceCountedChannel channel;
private final RemoteOptions options;
Expand Down Expand Up @@ -274,6 +278,10 @@ public ListenableFuture<String> getAuthority() {
@Override
public ListenableFuture<CachedActionResult> downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) {
if (context.getSpawnExecutionContext() != null) {
context.getSpawnExecutionContext().report(SPAWN_CHECKING_CACHE_EVENT);
}

GetActionResultRequest request =
GetActionResultRequest.newBuilder()
.setInstanceName(options.remoteInstanceName)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context
buildRequestId, commandId, actionKey.getDigest().getHash(), spawn.getResourceOwner());
RemoteActionExecutionContext remoteActionExecutionContext =
RemoteActionExecutionContext.create(
spawn, metadata, getWriteCachePolicy(spawn), getReadCachePolicy(spawn));
spawn, context, metadata, getWriteCachePolicy(spawn), getReadCachePolicy(spawn));

return new RemoteAction(
spawn,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.exec.SpawnCache;
import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
Expand All @@ -51,9 +50,6 @@
@ThreadSafe // If the RemoteActionCache implementation is thread-safe.
final class RemoteSpawnCache implements SpawnCache {

private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT =
SpawnCheckingCacheEvent.create("remote-cache");

private final Path execRoot;
private final RemoteOptions options;
private final RemoteExecutionService remoteExecutionService;
Expand Down Expand Up @@ -101,7 +97,6 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)

Profiler prof = Profiler.instance();
if (shouldAcceptCachedResult) {
context.report(SPAWN_CHECKING_CACHE_EVENT);
// Metadata will be available in context.current() until we detach.
// This is done via a thread-local variable.
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import build.bazel.remote.execution.v2.RequestMetadata;
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.exec.SpawnRunner;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
import javax.annotation.Nullable;

/** A context that provide remote execution related information for executing an action remotely. */
Expand Down Expand Up @@ -61,27 +63,35 @@ public CachePolicy addRemoteCache() {
}

@Nullable private final Spawn spawn;
@Nullable private final SpawnExecutionContext spawnExecutionContext;
private final RequestMetadata requestMetadata;
private final NetworkTime networkTime;
private final CachePolicy writeCachePolicy;
private final CachePolicy readCachePolicy;

private RemoteActionExecutionContext(
@Nullable Spawn spawn, RequestMetadata requestMetadata, NetworkTime networkTime) {
this.spawn = spawn;
this.requestMetadata = requestMetadata;
this.networkTime = networkTime;
this.writeCachePolicy = CachePolicy.ANY_CACHE;
this.readCachePolicy = CachePolicy.ANY_CACHE;
@Nullable Spawn spawn,
@Nullable SpawnRunner.SpawnExecutionContext spawnExecutionContext,
RequestMetadata requestMetadata,
NetworkTime networkTime) {
this(
spawn,
spawnExecutionContext,
requestMetadata,
networkTime,
CachePolicy.ANY_CACHE,
CachePolicy.ANY_CACHE);
}

private RemoteActionExecutionContext(
@Nullable Spawn spawn,
@Nullable SpawnExecutionContext spawnExecutionContext,
RequestMetadata requestMetadata,
NetworkTime networkTime,
CachePolicy writeCachePolicy,
CachePolicy readCachePolicy) {
this.spawn = spawn;
this.spawnExecutionContext = spawnExecutionContext;
this.requestMetadata = requestMetadata;
this.networkTime = networkTime;
this.writeCachePolicy = writeCachePolicy;
Expand All @@ -90,20 +100,42 @@ private RemoteActionExecutionContext(

public RemoteActionExecutionContext withWriteCachePolicy(CachePolicy writeCachePolicy) {
return new RemoteActionExecutionContext(
spawn, requestMetadata, networkTime, writeCachePolicy, readCachePolicy);
spawn,
spawnExecutionContext,
requestMetadata,
networkTime,
writeCachePolicy,
readCachePolicy);
}

public RemoteActionExecutionContext withReadCachePolicy(CachePolicy readCachePolicy) {
return new RemoteActionExecutionContext(
spawn, requestMetadata, networkTime, writeCachePolicy, readCachePolicy);
spawn,
spawnExecutionContext,
requestMetadata,
networkTime,
writeCachePolicy,
readCachePolicy);
}

/** Returns the {@link Spawn} of the action being executed or {@code null}. */
/**
* Returns the {@link Spawn} of the {@link RemoteAction} being executed, or {@code null} if it has
* no associated {@link Spawn}.
*/
@Nullable
public Spawn getSpawn() {
return spawn;
}

/**
* Returns the {@link SpawnExecutionContext} of the {@link RemoteAction} being executed, or {@code
* null} if it has no associated {@link Spawn}.
*/
@Nullable
public SpawnExecutionContext getSpawnExecutionContext() {
return spawnExecutionContext;
}

/** Returns the {@link RequestMetadata} for the action being executed. */
public RequestMetadata getRequestMetadata() {
return requestMetadata;
Expand Down Expand Up @@ -137,24 +169,32 @@ public CachePolicy getReadCachePolicy() {

/** Creates a {@link RemoteActionExecutionContext} with given {@link RequestMetadata}. */
public static RemoteActionExecutionContext create(RequestMetadata metadata) {
return new RemoteActionExecutionContext(/*spawn=*/ null, metadata, new NetworkTime());
return new RemoteActionExecutionContext(
/* spawn= */ null, /* spawnExecutionContext= */ null, metadata, new NetworkTime());
}

/**
* Creates a {@link RemoteActionExecutionContext} with given {@link Spawn} and {@link
* RequestMetadata}.
*/
public static RemoteActionExecutionContext create(
@Nullable Spawn spawn, RequestMetadata metadata) {
return new RemoteActionExecutionContext(spawn, metadata, new NetworkTime());
Spawn spawn, SpawnExecutionContext spawnExecutionContext, RequestMetadata metadata) {
return new RemoteActionExecutionContext(
spawn, spawnExecutionContext, metadata, new NetworkTime());
}

public static RemoteActionExecutionContext create(
@Nullable Spawn spawn,
Spawn spawn,
SpawnExecutionContext spawnExecutionContext,
RequestMetadata requestMetadata,
CachePolicy writeCachePolicy,
CachePolicy readCachePolicy) {
return new RemoteActionExecutionContext(
spawn, requestMetadata, new NetworkTime(), writeCachePolicy, readCachePolicy);
spawn,
spawnExecutionContext,
requestMetadata,
new NetworkTime(),
writeCachePolicy,
readCachePolicy);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ java_library(
name = "disk",
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/devtools/build/lib/exec:spawn_runner",
"//src/main/java/com/google/devtools/build/lib/remote:store",
"//src/main/java/com/google/devtools/build/lib/remote/common",
"//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.common.util.concurrent.MoreExecutors;
import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent;
import com.google.devtools.build.lib.remote.Store;
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
Expand Down Expand Up @@ -60,6 +61,10 @@
* safe to trim the cache at the same time a Bazel process is accessing it.
*/
public class DiskCacheClient implements RemoteCacheClient {

private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT =
SpawnCheckingCacheEvent.create("disk-cache");

private final Path root;
private final boolean verifyDownloads;
private final DigestUtil digestUtil;
Expand Down Expand Up @@ -222,6 +227,10 @@ public ListenableFuture<String> getAuthority() {
@Override
public ListenableFuture<CachedActionResult> downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) {
if (context.getSpawnExecutionContext() != null) {
context.getSpawnExecutionContext().report(SPAWN_CHECKING_CACHE_EVENT);
}

return Futures.transformAsync(
Utils.downloadAsActionResult(actionKey, (digest, out) -> download(digest, out, Store.AC)),
actionResult -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ java_library(
deps = [
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
"//src/main/java/com/google/devtools/build/lib/authandtls",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_runner",
"//src/main/java/com/google/devtools/build/lib/remote:Retrier",
"//src/main/java/com/google/devtools/build/lib/remote/common",
"//src/main/java/com/google/devtools/build/lib/remote/common:cache_not_found_exception",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.common.util.concurrent.MoreExecutors;
import com.google.common.util.concurrent.SettableFuture;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent;
import com.google.devtools.build.lib.remote.RemoteRetrier;
import com.google.devtools.build.lib.remote.common.CacheNotFoundException;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
Expand Down Expand Up @@ -122,6 +123,9 @@
public final class HttpCacheClient implements RemoteCacheClient {
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();

private static final SpawnCheckingCacheEvent SPAWN_CHECKING_CACHE_EVENT =
SpawnCheckingCacheEvent.create("remote-cache");

public static final String AC_PREFIX = "ac/";
public static final String CAS_PREFIX = "cas/";
private static final Pattern INVALID_TOKEN_ERROR =
Expand Down Expand Up @@ -617,6 +621,10 @@ public ListenableFuture<String> getAuthority() {
@Override
public ListenableFuture<CachedActionResult> downloadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, boolean inlineOutErr) {
if (context.getSpawnExecutionContext() != null) {
context.getSpawnExecutionContext().report(SPAWN_CHECKING_CACHE_EVENT);
}

return Futures.transform(
retrier.executeAsync(
() ->
Expand Down
1 change: 1 addition & 0 deletions src/test/java/com/google/devtools/build/lib/remote/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ filegroup(
testonly = 0,
srcs = glob(["**"]) + [
"//src/test/java/com/google/devtools/build/lib/remote/circuitbreaker:srcs",
"//src/test/java/com/google/devtools/build/lib/remote/disk:srcs",
"//src/test/java/com/google/devtools/build/lib/remote/downloader:srcs",
"//src/test/java/com/google/devtools/build/lib/remote/grpc:srcs",
"//src/test/java/com/google/devtools/build/lib/remote/http:srcs",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
import static org.mockito.AdditionalAnswers.answerVoid;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;

import build.bazel.remote.execution.v2.Action;
import build.bazel.remote.execution.v2.ActionCacheGrpc.ActionCacheImplBase;
Expand Down Expand Up @@ -58,13 +60,16 @@
import com.google.common.util.concurrent.MoreExecutors;
import com.google.devtools.build.lib.actions.ActionInputHelper;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
import com.google.devtools.build.lib.authandtls.CallCredentialsProvider;
import com.google.devtools.build.lib.authandtls.GoogleAuthUtils;
import com.google.devtools.build.lib.clock.JavaClock;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.exec.SpawnCheckingCacheEvent;
import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext;
import com.google.devtools.build.lib.remote.RemoteRetrier.ExponentialBackoff;
import com.google.devtools.build.lib.remote.Retrier.Backoff;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
Expand Down Expand Up @@ -257,7 +262,9 @@ public final void setUp() throws Exception {
RequestMetadata metadata =
TracingMetadataUtils.buildMetadata(
"none", "none", Digest.getDefaultInstance().getHash(), null);
context = RemoteActionExecutionContext.create(metadata);
context =
RemoteActionExecutionContext.create(
mock(Spawn.class), mock(SpawnExecutionContext.class), metadata);
retryService = MoreExecutors.listeningDecorator(Executors.newScheduledThreadPool(1));
}

Expand All @@ -272,6 +279,30 @@ public void tearDown() throws Exception {
fakeServer.awaitTermination();
}

@Test
public void testSpawnCheckingCacheEvent() throws Exception {
GrpcCacheClient client = newClient();

serviceRegistry.addService(
new ActionCacheImplBase() {
@Override
public void getActionResult(
GetActionResultRequest request, StreamObserver<ActionResult> responseObserver) {
responseObserver.onError(Status.NOT_FOUND.asRuntimeException());
}
});

var unused =
getFromFuture(
client.downloadActionResult(
context,
DIGEST_UTIL.asActionKey(DIGEST_UTIL.computeAsUtf8("key")),
/* inlineOutErr= */ false));

verify(context.getSpawnExecutionContext())
.report(SpawnCheckingCacheEvent.create("remote-cache"));
}

@Test
public void testVirtualActionInputSupport() throws Exception {
RemoteOptions options = Options.getDefaults(RemoteOptions.class);
Expand Down
Loading

0 comments on commit 40019a8

Please sign in to comment.