Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[7.1.0] Distinguish the disk and remote caches in the action progress status. #21084

Merged
merged 1 commit into from
Jan 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading