Skip to content

Commit

Permalink
Remote: Merge download and downloadMinimal.
Browse files Browse the repository at this point in the history
Cleans up the code structure without introducing new features and regressions. Future changes depend on this.

`remoteCache.download` is used to download outputs for remote executed actions and `remoteCache.downloadMinimal` is used to only download metadata of these outputs to save network time. The decision of which method to call is based on remote options.

This change merge these two methods into `remoteExecutionService.downloadOutputs` so that:
1. One step forward to blur the boundary between two download modes: having a single entry point. The ultimate goal is to use one code path for both modes.
2. Open opportunities to implement "remote output streaming": only download metadata within spawn execution and kick off the downloads into background so we don't need to wait for the downloads before starting another action execution.

Tests for these two methods are also updated:
1. Rename tests following the `testedMethodName_input_expectedOutcome` pattern.
2. Remove some duplicated tests in `GrpcCacheClientTest`.

Closes bazelbuild#13630.

PiperOrigin-RevId: 388147877
  • Loading branch information
coeuvre authored and copybara-github committed Aug 2, 2021
1 parent 23f171e commit c785f02
Show file tree
Hide file tree
Showing 13 changed files with 2,027 additions and 2,116 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,37 +73,22 @@ public static RemoteActionContextProvider createForPlaceholder(
env, /*cache=*/ null, /*executor=*/ null, retryScheduler, digestUtil, /*logDir=*/ null);
}

private static void maybeSetCaptureCorruptedOutputsDir(
RemoteOptions remoteOptions, RemoteCache remoteCache, Path workingDirectory) {
if (remoteOptions.remoteCaptureCorruptedOutputs != null
&& !remoteOptions.remoteCaptureCorruptedOutputs.isEmpty()) {
remoteCache.setCaptureCorruptedOutputsDir(
workingDirectory.getRelative(remoteOptions.remoteCaptureCorruptedOutputs));
}
}

public static RemoteActionContextProvider createForRemoteCaching(
CommandEnvironment env,
RemoteOptions options,
RemoteCache cache,
ListeningScheduledExecutorService retryScheduler,
DigestUtil digestUtil) {
maybeSetCaptureCorruptedOutputsDir(options, cache, env.getWorkingDirectory());

return new RemoteActionContextProvider(
env, cache, /*executor=*/ null, retryScheduler, digestUtil, /*logDir=*/ null);
}

public static RemoteActionContextProvider createForRemoteExecution(
CommandEnvironment env,
RemoteOptions options,
RemoteExecutionCache cache,
RemoteExecutionClient executor,
ListeningScheduledExecutorService retryScheduler,
DigestUtil digestUtil,
Path logDir) {
maybeSetCaptureCorruptedOutputsDir(options, cache, env.getWorkingDirectory());

return new RemoteActionContextProvider(
env, cache, executor, retryScheduler, digestUtil, logDir);
}
Expand All @@ -126,6 +111,15 @@ private RemotePathResolver createRemotePathResolver() {

private RemoteExecutionService getRemoteExecutionService() {
if (remoteExecutionService == null) {
Path workingDirectory = env.getWorkingDirectory();
RemoteOptions remoteOptions = checkNotNull(env.getOptions().getOptions(RemoteOptions.class));
Path captureCorruptedOutputsDir = null;
if (remoteOptions.remoteCaptureCorruptedOutputs != null
&& !remoteOptions.remoteCaptureCorruptedOutputs.isEmpty()) {
captureCorruptedOutputsDir =
workingDirectory.getRelative(remoteOptions.remoteCaptureCorruptedOutputs);
}

remoteExecutionService =
new RemoteExecutionService(
env.getExecRoot(),
Expand All @@ -136,7 +130,8 @@ private RemoteExecutionService getRemoteExecutionService() {
checkNotNull(env.getOptions().getOptions(RemoteOptions.class)),
cache,
executor,
filesToDownload);
filesToDownload,
captureCorruptedOutputsDir);
}

return remoteExecutionService;
Expand Down
Loading

0 comments on commit c785f02

Please sign in to comment.