Skip to content

Commit

Permalink
Rework to emit event from cache client implementations
Browse files Browse the repository at this point in the history
  • Loading branch information
tjgq committed Jan 19, 2024
1 parent dc6158f commit 40274e7
Show file tree
Hide file tree
Showing 20 changed files with 299 additions and 111 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 All @@ -57,6 +58,7 @@
import com.google.devtools.build.lib.remote.util.DigestOutputStream;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
import com.google.devtools.build.lib.remote.util.Utils;
import com.google.devtools.build.lib.remote.zstd.ZstdDecompressingOutputStream;
import com.google.devtools.build.lib.vfs.Path;
Expand All @@ -76,11 +78,17 @@
import java.util.function.Supplier;
import javax.annotation.Nullable;

/** A RemoteActionCache implementation that uses gRPC calls to a remote cache server. */
/**
* A RemoteActionCache implementation that uses gRPC calls to a remote cache server.
*/
@ThreadSafe
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 @@ -117,11 +125,6 @@ public GrpcCacheClient(
maxMissingBlobsDigestsPerMessage > 0, "Error: gRPC message size too small.");
}

@Override
public String getDisplayName() {
return "remote-cache";
}

private int computeMaxMissingBlobsDigestsPerMessage() {
final int overhead =
FindMissingBlobsRequest.newBuilder()
Expand Down Expand Up @@ -279,6 +282,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 @@ -95,10 +95,6 @@ public RemoteCache(
this.options = options;
this.digestUtil = digestUtil;
}

public String getDisplayName() {
return cacheProtocol.getDisplayName();
}

public CacheCapabilities getCacheCapabilities() throws IOException {
return cacheProtocol.getCacheCapabilities();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,15 +289,6 @@ private static boolean useDiskCache(RemoteOptions options) {
return options.diskCache != null && !options.diskCache.isEmpty();
}

/**
* If using a disk, remote or combined cache, returns the display name for the cache. Otherwise,
* returns null.
*/
@Nullable
public String getCacheDisplayName() {
return remoteCache != null ? checkNotNull(remoteCache.getDisplayName()) : null;
}

public CachePolicy getReadCachePolicy(Spawn spawn) {
if (remoteCache == null) {
return CachePolicy.NO_CACHE;
Expand Down Expand Up @@ -614,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 @@ -107,7 +107,6 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)

Profiler prof = Profiler.instance();
if (shouldAcceptCachedResult) {
reportSpawnCheckingCacheEvent(context);
// Metadata will be available in context.current() until we detach.
// This is done via a thread-local variable.
try {
Expand Down Expand Up @@ -218,13 +217,6 @@ private void checkForConcurrentModifications()
}
}

private void reportSpawnCheckingCacheEvent(SpawnExecutionContext context) {
String displayName = checkNotNull(remoteExecutionService.getCacheDisplayName());
SpawnCheckingCacheEvent event = EVENT_MAP.computeIfAbsent(displayName,
SpawnCheckingCacheEvent::create);
context.report(event);
}

@Override
public boolean usefulInDynamicExecution() {
return false;
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 @@ -60,28 +62,31 @@ public CachePolicy addRemoteCache() {
}
}

@Nullable private final Spawn spawn;
@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,21 +95,37 @@ 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 RequestMetadata} for the action being executed. */
/**
* 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 +158,28 @@ 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}.
* 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 @@ -34,8 +34,6 @@
*/
public interface RemoteCacheClient extends MissingDigestsFinder {

String getDisplayName();

CacheCapabilities getCacheCapabilities() throws IOException;

ListenableFuture<String> getAuthority();
Expand Down
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 @@ -49,11 +49,6 @@ public DiskAndRemoteCacheClient(DiskCacheClient diskCache, RemoteCacheClient rem
this.remoteCache = Preconditions.checkNotNull(remoteCache);
}

@Override
public String getDisplayName() {
return "combined-cache";
}

@Override
public ListenableFuture<Void> uploadActionResult(
RemoteActionExecutionContext context, ActionKey actionKey, ActionResult actionResult) {
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 All @@ -84,11 +89,6 @@ public DiskCacheClient(Path root, boolean verifyDownloads, DigestUtil digestUtil
this.root.createDirectoryAndParents();
}

@Override
public String getDisplayName() {
return "disk-cache";
}

/**
* If the given path exists, updates its mtime and returns true. Otherwise, returns false.
*
Expand Down Expand Up @@ -227,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 @@ -120,8 +121,12 @@
* <p>The implementation currently does not support transfer encoding chunked.
*/
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 @@ -300,11 +305,6 @@ public void channelCreated(Channel ch) {
this.retrier = retrier;
}

@Override
public String getDisplayName() {
return "remote-cache";
}

@SuppressWarnings("FutureReturnValueIgnored")
private Promise<Channel> acquireUploadChannel() {
Promise<Channel> channelReady = eventLoop.next().newPromise();
Expand Down Expand Up @@ -622,6 +622,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
Loading

0 comments on commit 40274e7

Please sign in to comment.