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

[5.3.0] Refactor combined cache. #16110

Merged
merged 1 commit into from
Aug 16, 2022
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 @@ -30,7 +30,7 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext.Step;
import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext.CachePolicy;
import com.google.devtools.build.lib.remote.util.DigestUtil;
import com.google.devtools.build.lib.remote.util.TracingMetadataUtils;
import com.google.devtools.build.lib.vfs.Path;
Expand Down Expand Up @@ -274,8 +274,9 @@ private Single<PathConverter> upload(Set<Path> files) {

RequestMetadata metadata =
TracingMetadataUtils.buildMetadata(buildRequestId, commandId, "bes-upload", null);
RemoteActionExecutionContext context = RemoteActionExecutionContext.create(metadata);
context.setStep(Step.UPLOAD_BES_FILES);
RemoteActionExecutionContext context =
RemoteActionExecutionContext.create(metadata)
.withWriteCachePolicy(CachePolicy.REMOTE_CACHE_ONLY);

return Single.using(
remoteCache::retain,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,11 @@ public static RemoteCacheClient createDiskAndRemoteClient(
PathFragment diskCachePath,
boolean remoteVerifyDownloads,
DigestUtil digestUtil,
RemoteCacheClient remoteCacheClient,
RemoteOptions options)
RemoteCacheClient remoteCacheClient)
throws IOException {
DiskCacheClient diskCacheClient =
createDiskCache(workingDirectory, diskCachePath, remoteVerifyDownloads, digestUtil);
return new DiskAndRemoteCacheClient(diskCacheClient, remoteCacheClient, options);
return new DiskAndRemoteCacheClient(diskCacheClient, remoteCacheClient);
}

public static RemoteCacheClient create(
Expand Down Expand Up @@ -147,12 +146,7 @@ private static RemoteCacheClient createDiskAndHttpCache(

RemoteCacheClient httpCache = createHttp(options, cred, digestUtil);
return createDiskAndRemoteClient(
workingDirectory,
diskCachePath,
options.remoteVerifyDownloads,
digestUtil,
httpCache,
options);
workingDirectory, diskCachePath, options.remoteVerifyDownloads, digestUtil, httpCache);
}

public static boolean isDiskCache(RemoteOptions options) {
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -567,8 +567,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
remoteOptions.diskCache,
remoteOptions.remoteVerifyDownloads,
digestUtil,
cacheClient,
remoteOptions);
cacheClient);
} catch (IOException e) {
handleInitFailure(env, e, Code.CACHE_INIT_FAILURE);
return;
Expand Down Expand Up @@ -626,8 +625,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
remoteOptions.diskCache,
remoteOptions.remoteVerifyDownloads,
digestUtil,
cacheClient,
remoteOptions);
cacheClient);
} catch (IOException e) {
handleInitFailure(env, e, Code.CACHE_INIT_FAILURE);
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,10 @@ RemoteExecutionService getRemoteExecutionService() {
@Override
public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
throws InterruptedException, IOException, ExecException, ForbiddenActionInputException {
boolean shouldAcceptCachedResult = remoteExecutionService.shouldAcceptCachedResult(spawn);
boolean shouldUploadLocalResults = remoteExecutionService.shouldUploadLocalResults(spawn);
boolean shouldAcceptCachedResult =
remoteExecutionService.getReadCachePolicy(spawn).allowAnyCache();
boolean shouldUploadLocalResults =
remoteExecutionService.getWriteCachePolicy(spawn).allowAnyCache();
if (!shouldAcceptCachedResult && !shouldUploadLocalResults) {
return SpawnCache.NO_RESULT_NO_STORE;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,11 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
"Spawn can't be executed remotely. This is a bug.");

Stopwatch totalTime = Stopwatch.createStarted();
boolean uploadLocalResults = remoteExecutionService.shouldUploadLocalResults(spawn);
boolean acceptCachedResult = remoteExecutionService.shouldAcceptCachedResult(spawn);
boolean acceptCachedResult = remoteExecutionService.getReadCachePolicy(spawn).allowAnyCache();
boolean uploadLocalResults = remoteExecutionService.getWriteCachePolicy(spawn).allowAnyCache();

RemoteAction action = remoteExecutionService.buildRemoteAction(spawn, context);

SpawnMetrics.Builder spawnMetrics =
SpawnMetrics.Builder.forRemoteExec()
.setInputBytes(action.getInputBytes())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,48 +13,89 @@
// limitations under the License.
package com.google.devtools.build.lib.remote.common;

import build.bazel.remote.execution.v2.ExecuteResponse;
import build.bazel.remote.execution.v2.RequestMetadata;
import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
import com.google.devtools.build.lib.actions.Spawn;
import javax.annotation.Nullable;

/** A context that provide remote execution related information for executing an action remotely. */
public class RemoteActionExecutionContext {
/** The current step of the context. */
public enum Step {
INIT,
CHECK_ACTION_CACHE,
UPLOAD_INPUTS,
EXECUTE_REMOTELY,
UPLOAD_OUTPUTS,
DOWNLOAD_OUTPUTS,
UPLOAD_BES_FILES,
/** Determines whether to read/write remote cache, disk cache or both. */
public enum CachePolicy {
NO_CACHE,
REMOTE_CACHE_ONLY,
DISK_CACHE_ONLY,
ANY_CACHE;

public static CachePolicy create(boolean allowRemoteCache, boolean allowDiskCache) {
if (allowRemoteCache && allowDiskCache) {
return ANY_CACHE;
} else if (allowRemoteCache) {
return REMOTE_CACHE_ONLY;
} else if (allowDiskCache) {
return DISK_CACHE_ONLY;
} else {
return NO_CACHE;
}
}

public boolean allowAnyCache() {
return this != NO_CACHE;
}

public boolean allowRemoteCache() {
return this == REMOTE_CACHE_ONLY || this == ANY_CACHE;
}

public boolean allowDiskCache() {
return this == DISK_CACHE_ONLY || this == ANY_CACHE;
}

public CachePolicy addRemoteCache() {
if (this == DISK_CACHE_ONLY || this == ANY_CACHE) {
return ANY_CACHE;
}

return REMOTE_CACHE_ONLY;
}
}

@Nullable private final Spawn spawn;
private final RequestMetadata requestMetadata;
private final NetworkTime networkTime;

@Nullable private ExecuteResponse executeResponse;
private Step step;
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.step = Step.INIT;
this.writeCachePolicy = CachePolicy.ANY_CACHE;
this.readCachePolicy = CachePolicy.ANY_CACHE;
}

/** Returns current {@link Step} of the context. */
public Step getStep() {
return step;
private RemoteActionExecutionContext(
@Nullable Spawn spawn,
RequestMetadata requestMetadata,
NetworkTime networkTime,
CachePolicy writeCachePolicy,
CachePolicy readCachePolicy) {
this.spawn = spawn;
this.requestMetadata = requestMetadata;
this.networkTime = networkTime;
this.writeCachePolicy = writeCachePolicy;
this.readCachePolicy = readCachePolicy;
}

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

/** Sets current {@link Step} of the context. */
public void setStep(Step step) {
this.step = step;
public RemoteActionExecutionContext withReadCachePolicy(CachePolicy readCachePolicy) {
return new RemoteActionExecutionContext(
spawn, requestMetadata, networkTime, writeCachePolicy, readCachePolicy);
}

/** Returns the {@link Spawn} of the action being executed or {@code null}. */
Expand Down Expand Up @@ -86,13 +127,12 @@ public ActionExecutionMetadata getSpawnOwner() {
return spawn.getResourceOwner();
}

public void setExecuteResponse(@Nullable ExecuteResponse executeResponse) {
this.executeResponse = executeResponse;
public CachePolicy getWriteCachePolicy() {
return writeCachePolicy;
}

@Nullable
public ExecuteResponse getExecuteResponse() {
return executeResponse;
public CachePolicy getReadCachePolicy() {
return readCachePolicy;
}

/** Creates a {@link RemoteActionExecutionContext} with given {@link RequestMetadata}. */
Expand All @@ -108,4 +148,13 @@ public static RemoteActionExecutionContext create(
@Nullable Spawn spawn, RequestMetadata metadata) {
return new RemoteActionExecutionContext(spawn, metadata, new NetworkTime());
}

public static RemoteActionExecutionContext create(
@Nullable Spawn spawn,
RequestMetadata requestMetadata,
CachePolicy writeCachePolicy,
CachePolicy readCachePolicy) {
return new RemoteActionExecutionContext(
spawn, requestMetadata, new NetworkTime(), writeCachePolicy, readCachePolicy);
}
}
Loading