From ae53991f2e207edacd1352ba94261e2473b79f14 Mon Sep 17 00:00:00 2001 From: chiwang Date: Fri, 23 Apr 2021 00:25:02 -0700 Subject: [PATCH] Remote: Add RemoteExecutionService as a layer between spawn execution and remote execution. The initiative for this change is to let it easier to make changes to RemoteCache and other places. RemoteCache provides a set of methods to access the remote cache in a protocol independent manner and is used in the following places: - In RemoteSpawnCache (for remote cache only mode), we lookup remote cache before local execution. If no cache found, we execute the spawn locally and then upload ouputs to remote cache. - In RemoteSpawnRunner (for remote execution mode), we lookup remote cache before requesting remote execution. If no cache found, we upload inputs to remote cache, submit execution request and then download outputs from remote cache. - In RemoteRepositoryRemoteExecutor (for executing repository command remotely), we lookup remote cache and upload inputs. - In ExecutionServer (for remote worker), we download inputs from remote cache, execute the action and upload outputs to remote cache. Among these methods, only a few need spawn specific types as members, most of them are just helper functions for easily calling RemoteCacheClient. It's hard to use RemoteCache in other places since we need spawn specific types to construct it. Besides that, some state are not available at the time when we construct RemoteCache e.g. execRoot which only make sense when executing actions. It's impossible to add state as members that are only available during spawn execution to RemoteCache. We move these methods that depend spawn specific types to RemoteExecutionService so RemoteCache is decoupled from spawn execution. RemoteExecutionService is designed to provide a set of primitive operations for remote cache and execution of spawn actions. RemoteSpwanCache/RemoteSpawnRunner are the intended call sites and do the orchestration. This change only restructure the code in the call sides. Following changes will update RemoteCache. PiperOrigin-RevId: 370028737 --- .../remote/RemoteActionContextProvider.java | 34 +- .../lib/remote/RemoteExecutionService.java | 481 ++++++++++++++++++ .../RemoteRepositoryRemoteExecutor.java | 5 +- .../build/lib/remote/RemoteSpawnCache.java | 143 +----- .../build/lib/remote/RemoteSpawnRunner.java | 409 +++------------ .../devtools/build/lib/remote/util/Utils.java | 23 + .../lib/remote/RemoteSpawnCacheTest.java | 24 +- .../lib/remote/RemoteSpawnRunnerTest.java | 71 ++- ...SpawnRunnerWithGrpcRemoteExecutorTest.java | 19 +- 9 files changed, 663 insertions(+), 546 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java index b3ff7ded638886..c1f8462f077f34 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java @@ -48,6 +48,7 @@ final class RemoteActionContextProvider implements ExecutorLifecycleListener { private final DigestUtil digestUtil; @Nullable private final Path logDir; private ImmutableSet filesToDownload = ImmutableSet.of(); + private RemoteExecutionService remoteExecutionService; private RemoteActionContextProvider( CommandEnvironment env, @@ -100,6 +101,24 @@ RemotePathResolver createRemotePathResolver() { return remotePathResolver; } + RemoteExecutionService getRemoteExecutionService() { + if (remoteExecutionService == null) { + remoteExecutionService = + new RemoteExecutionService( + env.getExecRoot(), + createRemotePathResolver(), + env.getBuildRequestId(), + env.getCommandId().toString(), + digestUtil, + checkNotNull(env.getOptions().getOptions(RemoteOptions.class)), + cache, + executor, + filesToDownload); + } + + return remoteExecutionService; + } + /** * Registers a remote spawn strategy if this instance was created with an executor, otherwise does * nothing. @@ -121,15 +140,9 @@ public void registerRemoteSpawnStrategyIfApplicable( env.getOptions().getOptions(ExecutionOptions.class), verboseFailures, env.getReporter(), - env.getBuildRequestId(), - env.getCommandId().toString(), - (RemoteExecutionCache) cache, - executor, retryScheduler, - digestUtil, logDir, - filesToDownload, - createRemotePathResolver()); + getRemoteExecutionService()); registryBuilder.registerStrategy( new RemoteSpawnStrategy(env.getExecRoot(), spawnRunner, verboseFailures), "remote"); } @@ -145,13 +158,8 @@ public void registerSpawnCache(ModuleActionContextRegistry.Builder registryBuild env.getExecRoot(), checkNotNull(env.getOptions().getOptions(RemoteOptions.class)), checkNotNull(env.getOptions().getOptions(ExecutionOptions.class)).verboseFailures, - cache, - env.getBuildRequestId(), - env.getCommandId().toString(), env.getReporter(), - digestUtil, - filesToDownload, - createRemotePathResolver()); + getRemoteExecutionService()); registryBuilder.register(SpawnCache.class, spawnCache, "remote-cache"); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java new file mode 100644 index 00000000000000..aecd293d7ca92e --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -0,0 +1,481 @@ +// Copyright 2021 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +package com.google.devtools.build.lib.remote; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture; +import static com.google.devtools.build.lib.remote.util.Utils.getInMemoryOutputPath; +import static com.google.devtools.build.lib.remote.util.Utils.hasFilesToDownload; +import static com.google.devtools.build.lib.remote.util.Utils.shouldDownloadAllSpawnOutputs; + +import build.bazel.remote.execution.v2.Action; +import build.bazel.remote.execution.v2.ActionResult; +import build.bazel.remote.execution.v2.Command; +import build.bazel.remote.execution.v2.Digest; +import build.bazel.remote.execution.v2.ExecuteRequest; +import build.bazel.remote.execution.v2.ExecuteResponse; +import build.bazel.remote.execution.v2.ExecutedActionMetadata; +import build.bazel.remote.execution.v2.LogFile; +import build.bazel.remote.execution.v2.Platform; +import build.bazel.remote.execution.v2.RequestMetadata; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.Spawn; +import com.google.devtools.build.lib.actions.Spawns; +import com.google.devtools.build.lib.actions.UserExecException; +import com.google.devtools.build.lib.analysis.platform.PlatformUtils; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; +import com.google.devtools.build.lib.remote.common.NetworkTime; +import com.google.devtools.build.lib.remote.common.OperationObserver; +import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; +import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; +import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; +import com.google.devtools.build.lib.remote.common.RemotePathResolver; +import com.google.devtools.build.lib.remote.merkletree.MerkleTree; +import com.google.devtools.build.lib.remote.options.RemoteOptions; +import com.google.devtools.build.lib.remote.options.RemoteOutputsMode; +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.Utils; +import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.protobuf.Message; +import io.grpc.Status.Code; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.SortedMap; +import java.util.TreeSet; +import javax.annotation.Nullable; + +/** + * A layer between spawn execution and remote execution exposing primitive operations for remote + * cache and execution with spawn specific types. + */ +public class RemoteExecutionService { + private final Path execRoot; + private final RemotePathResolver remotePathResolver; + private final String buildRequestId; + private final String commandId; + private final DigestUtil digestUtil; + private final RemoteOptions remoteOptions; + private final RemoteCache remoteCache; + @Nullable private final RemoteExecutionClient remoteExecutor; + private final ImmutableSet filesToDownload; + + public RemoteExecutionService( + Path execRoot, + RemotePathResolver remotePathResolver, + String buildRequestId, + String commandId, + DigestUtil digestUtil, + RemoteOptions remoteOptions, + RemoteCache remoteCache, + @Nullable RemoteExecutionClient remoteExecutor, + ImmutableSet filesToDownload) { + this.execRoot = execRoot; + this.remotePathResolver = remotePathResolver; + this.buildRequestId = buildRequestId; + this.commandId = commandId; + this.digestUtil = digestUtil; + this.remoteOptions = remoteOptions; + this.remoteCache = remoteCache; + this.remoteExecutor = remoteExecutor; + this.filesToDownload = filesToDownload; + } + + static Command buildCommand( + Collection outputs, + List arguments, + ImmutableMap env, + @Nullable Platform platform, + RemotePathResolver remotePathResolver) { + Command.Builder command = Command.newBuilder(); + ArrayList outputFiles = new ArrayList<>(); + ArrayList outputDirectories = new ArrayList<>(); + for (ActionInput output : outputs) { + String pathString = remotePathResolver.localPathToOutputPath(output); + if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) { + outputDirectories.add(pathString); + } else { + outputFiles.add(pathString); + } + } + Collections.sort(outputFiles); + Collections.sort(outputDirectories); + command.addAllOutputFiles(outputFiles); + command.addAllOutputDirectories(outputDirectories); + + if (platform != null) { + command.setPlatform(platform); + } + command.addAllArguments(arguments); + // Sorting the environment pairs by variable name. + TreeSet variables = new TreeSet<>(env.keySet()); + for (String var : variables) { + command.addEnvironmentVariablesBuilder().setName(var).setValue(env.get(var)); + } + + String workingDirectory = remotePathResolver.getWorkingDirectory(); + if (!Strings.isNullOrEmpty(workingDirectory)) { + command.setWorkingDirectory(workingDirectory); + } + return command.build(); + } + + /** A value class representing an action which can be executed remotely. */ + public static class RemoteAction { + private final Spawn spawn; + private final SpawnExecutionContext spawnExecutionContext; + private final RemoteActionExecutionContext remoteActionExecutionContext; + private final SortedMap inputMap; + private final MerkleTree merkleTree; + private final Digest commandHash; + private final Command command; + private final Action action; + private final ActionKey actionKey; + + RemoteAction( + Spawn spawn, + SpawnExecutionContext spawnExecutionContext, + RemoteActionExecutionContext remoteActionExecutionContext, + SortedMap inputMap, + MerkleTree merkleTree, + Digest commandHash, + Command command, + Action action, + ActionKey actionKey) { + this.spawn = spawn; + this.spawnExecutionContext = spawnExecutionContext; + this.remoteActionExecutionContext = remoteActionExecutionContext; + this.inputMap = inputMap; + this.merkleTree = merkleTree; + this.commandHash = commandHash; + this.command = command; + this.action = action; + this.actionKey = actionKey; + } + + /** + * Returns the sum of file sizes plus protobuf sizes used to represent the inputs of this + * action. + */ + public long getInputBytes() { + return merkleTree.getInputBytes(); + } + + /** Returns the number of input files of this action. */ + public long getInputFiles() { + return merkleTree.getInputFiles(); + } + + /** Returns the id this is action. */ + public String getActionId() { + return actionKey.getDigest().getHash(); + } + + /** + * Returns a {@link SortedMap} which maps from input paths for remote action to {@link + * ActionInput}. + */ + public SortedMap getInputMap() { + return inputMap; + } + + /** + * Returns the {@link NetworkTime} instance used to measure the network time during the action + * execution. + */ + public NetworkTime getNetworkTime() { + return remoteActionExecutionContext.getNetworkTime(); + } + } + + /** Creates a new {@link RemoteAction} instance from spawn. */ + public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context) + throws IOException, UserExecException { + SortedMap inputMap = remotePathResolver.getInputMapping(context); + final MerkleTree merkleTree = + MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil); + + // Get the remote platform properties. + Platform platform = PlatformUtils.getPlatformProto(spawn, remoteOptions); + + Command command = + buildCommand( + spawn.getOutputFiles(), + spawn.getArguments(), + spawn.getEnvironment(), + platform, + remotePathResolver); + Digest commandHash = digestUtil.compute(command); + Action action = + Utils.buildAction( + commandHash, + merkleTree.getRootDigest(), + platform, + context.getTimeout(), + Spawns.mayBeCachedRemotely(spawn)); + + ActionKey actionKey = digestUtil.computeActionKey(action); + + RequestMetadata metadata = + TracingMetadataUtils.buildMetadata( + buildRequestId, commandId, actionKey.getDigest().getHash(), spawn.getResourceOwner()); + RemoteActionExecutionContext remoteActionExecutionContext = + RemoteActionExecutionContext.create(metadata); + + return new RemoteAction( + spawn, + context, + remoteActionExecutionContext, + inputMap, + merkleTree, + commandHash, + command, + action, + actionKey); + } + + /** A value class representing the result of remotely executed {@link RemoteAction}. */ + public static class RemoteActionResult { + private final ActionResult actionResult; + @Nullable private final ExecuteResponse executeResponse; + + /** Creates a new {@link RemoteActionResult} instance from a cached result. */ + public static RemoteActionResult createFromCache(ActionResult cachedActionResult) { + checkArgument(cachedActionResult != null, "cachedActionResult is null"); + return new RemoteActionResult(cachedActionResult, null); + } + + /** Creates a new {@link RemoteActionResult} instance from a execute response. */ + public static RemoteActionResult createFromResponse(ExecuteResponse response) { + checkArgument(response.hasResult(), "response doesn't have result"); + return new RemoteActionResult(response.getResult(), response); + } + + public RemoteActionResult( + ActionResult actionResult, @Nullable ExecuteResponse executeResponse) { + this.actionResult = actionResult; + this.executeResponse = executeResponse; + } + + /** Returns the exit code of remote executed action. */ + public int getExitCode() { + return actionResult.getExitCode(); + } + + /** + * Returns the freeform informational message with details on the execution of the action that + * may be displayed to the user upon failure or when requested explicitly. + */ + public String getMessage() { + return executeResponse != null ? executeResponse.getMessage() : ""; + } + + /** Returns the details of the execution that originally produced this result. */ + public ExecutedActionMetadata getExecutionMetadata() { + return actionResult.getExecutionMetadata(); + } + + /** Returns whether the action is executed successfully. */ + public boolean success() { + if (executeResponse != null) { + if (executeResponse.getStatus().getCode() != Code.OK.value()) { + return false; + } + } + + return actionResult.getExitCode() == 0; + } + + /** Returns {@code true} if this result is from a cache. */ + public boolean cacheHit() { + if (executeResponse == null) { + return true; + } + + return executeResponse.getCachedResult(); + } + + /** + * Returns the underlying {@link ExecuteResponse} or {@code null} if this result is from a + * cache. + */ + @Nullable + public ExecuteResponse getResponse() { + return executeResponse; + } + } + + /** Lookup the remote cache for the given {@link RemoteAction}. {@code null} if not found. */ + @Nullable + public RemoteActionResult lookupCache(RemoteAction action) + throws IOException, InterruptedException { + ActionResult actionResult = + remoteCache.downloadActionResult( + action.remoteActionExecutionContext, action.actionKey, /* inlineOutErr= */ false); + + if (actionResult == null) { + return null; + } + + return RemoteActionResult.createFromCache(actionResult); + } + + /** Downloads outputs of a remotely executed action from remote cache. */ + @Nullable + public InMemoryOutput downloadOutputs(RemoteAction action, RemoteActionResult result) + throws InterruptedException, IOException, ExecException { + RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode; + boolean downloadOutputs = + shouldDownloadAllSpawnOutputs( + remoteOutputsMode, + /* exitCode = */ result.actionResult.getExitCode(), + hasFilesToDownload(action.spawn.getOutputFiles(), filesToDownload)); + InMemoryOutput inMemoryOutput = null; + if (downloadOutputs) { + remoteCache.download( + action.remoteActionExecutionContext, + remotePathResolver, + result.actionResult, + action.spawnExecutionContext.getFileOutErr(), + action.spawnExecutionContext::lockOutputFiles); + } else { + PathFragment inMemoryOutputPath = getInMemoryOutputPath(action.spawn); + inMemoryOutput = + remoteCache.downloadMinimal( + action.remoteActionExecutionContext, + remotePathResolver, + result.actionResult, + action.spawn.getOutputFiles(), + inMemoryOutputPath, + action.spawnExecutionContext.getFileOutErr(), + action.spawnExecutionContext.getMetadataInjector(), + action.spawnExecutionContext::lockOutputFiles); + } + + return inMemoryOutput; + } + + /** Upload outputs of a remote action which was executed locally to remote cache. */ + public void uploadOutputs(RemoteAction action) + throws InterruptedException, IOException, ExecException { + Collection outputFiles = + action.spawn.getOutputFiles().stream() + .map((inp) -> execRoot.getRelative(inp.getExecPath())) + .collect(ImmutableList.toImmutableList()); + remoteCache.upload( + action.remoteActionExecutionContext, + remotePathResolver, + action.actionKey, + action.action, + action.command, + outputFiles, + action.spawnExecutionContext.getFileOutErr()); + } + + /** + * Upload inputs of a remote action to remote cache if they are not presented already. + * + *

Must be called before calling {@link #execute}. + */ + public void uploadInputsIfNotPresent(RemoteAction action) + throws IOException, InterruptedException { + Preconditions.checkState(remoteCache instanceof RemoteExecutionCache); + RemoteExecutionCache remoteExecutionCache = (RemoteExecutionCache) remoteCache; + // Upload the command and all the inputs into the remote cache. + Map additionalInputs = Maps.newHashMapWithExpectedSize(2); + additionalInputs.put(action.actionKey.getDigest(), action.action); + additionalInputs.put(action.commandHash, action.command); + remoteExecutionCache.ensureInputsPresent( + action.remoteActionExecutionContext, action.merkleTree, additionalInputs); + } + + /** + * Executes the remote action remotely and returns the result. + * + * @param acceptCachedResult tells remote execution server whether it should used cached result. + * @param observer receives status updates during the execution. + */ + public RemoteActionResult execute( + RemoteAction action, boolean acceptCachedResult, OperationObserver observer) + throws IOException, InterruptedException { + Preconditions.checkNotNull(remoteExecutor, "remoteExecutor"); + + ExecuteRequest.Builder requestBuilder = + ExecuteRequest.newBuilder() + .setInstanceName(remoteOptions.remoteInstanceName) + .setActionDigest(action.actionKey.getDigest()) + .setSkipCacheLookup(!acceptCachedResult); + if (remoteOptions.remoteResultCachePriority != 0) { + requestBuilder + .getResultsCachePolicyBuilder() + .setPriority(remoteOptions.remoteResultCachePriority); + } + if (remoteOptions.remoteExecutionPriority != 0) { + requestBuilder.getExecutionPolicyBuilder().setPriority(remoteOptions.remoteExecutionPriority); + } + + ExecuteRequest request = requestBuilder.build(); + + ExecuteResponse reply = + remoteExecutor.executeRemotely(action.remoteActionExecutionContext, request, observer); + + return RemoteActionResult.createFromResponse(reply); + } + + /** A value classes representing downloaded server logs. */ + public static class ServerLogs { + public int logCount; + public Path directory; + @Nullable public Path lastLogPath; + } + + /** Downloads server logs from a remotely executed action if any. */ + public ServerLogs maybeDownloadServerLogs(RemoteAction action, ExecuteResponse resp, Path logDir) + throws InterruptedException, IOException { + ServerLogs serverLogs = new ServerLogs(); + serverLogs.directory = logDir.getRelative(action.getActionId()); + + ActionResult actionResult = resp.getResult(); + if (resp.getServerLogsCount() > 0 + && (actionResult.getExitCode() != 0 || resp.getStatus().getCode() != Code.OK.value())) { + for (Map.Entry e : resp.getServerLogsMap().entrySet()) { + if (e.getValue().getHumanReadable()) { + serverLogs.lastLogPath = serverLogs.directory.getRelative(e.getKey()); + serverLogs.logCount++; + getFromFuture( + remoteCache.downloadFile( + action.remoteActionExecutionContext, + serverLogs.lastLogPath, + e.getValue().getDigest())); + } + } + } + + return serverLogs; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java index f239fe3fb04163..ba889b9c135cbb 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteRepositoryRemoteExecutor.java @@ -13,6 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.remote; +import static com.google.devtools.build.lib.remote.util.Utils.buildAction; + import build.bazel.remote.execution.v2.Action; import build.bazel.remote.execution.v2.ActionResult; import build.bazel.remote.execution.v2.Command; @@ -129,8 +131,7 @@ public ExecutionResult execute( Digest commandHash = digestUtil.compute(command); MerkleTree merkleTree = MerkleTree.build(inputFiles, digestUtil); Action action = - RemoteSpawnRunner.buildAction( - commandHash, merkleTree.getRootDigest(), platform, timeout, acceptCached); + buildAction(commandHash, merkleTree.getRootDigest(), platform, timeout, acceptCached); Digest actionDigest = digestUtil.compute(action); ActionKey actionKey = new ActionKey(actionDigest); ActionResult actionResult; diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 322321e81bf303..d6b3e1c92361a1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -14,21 +14,11 @@ package com.google.devtools.build.lib.remote; import static com.google.common.base.Strings.isNullOrEmpty; +import static com.google.devtools.build.lib.profiler.ProfilerTask.REMOTE_DOWNLOAD; import static com.google.devtools.build.lib.remote.util.Utils.createSpawnResult; -import static com.google.devtools.build.lib.remote.util.Utils.getInMemoryOutputPath; -import static com.google.devtools.build.lib.remote.util.Utils.hasFilesToDownload; -import static com.google.devtools.build.lib.remote.util.Utils.shouldDownloadAllSpawnOutputs; -import build.bazel.remote.execution.v2.Action; -import build.bazel.remote.execution.v2.ActionResult; -import build.bazel.remote.execution.v2.Command; -import build.bazel.remote.execution.v2.Digest; -import build.bazel.remote.execution.v2.Platform; -import build.bazel.remote.execution.v2.RequestMetadata; -import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; import com.google.common.base.Throwables; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.FileArtifactValue; @@ -38,7 +28,6 @@ import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; -import com.google.devtools.build.lib.analysis.platform.PlatformUtils; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; @@ -48,25 +37,17 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; +import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteAction; +import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult; import com.google.devtools.build.lib.remote.common.CacheNotFoundException; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; -import com.google.devtools.build.lib.remote.common.RemotePathResolver; -import com.google.devtools.build.lib.remote.merkletree.MerkleTree; import com.google.devtools.build.lib.remote.options.RemoteOptions; -import com.google.devtools.build.lib.remote.options.RemoteOutputsMode; -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.Utils; import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; -import java.util.Collection; import java.util.HashSet; import java.util.NoSuchElementException; import java.util.Set; -import java.util.SortedMap; import javax.annotation.Nullable; /** A remote {@link SpawnCache} implementation. */ @@ -76,45 +57,21 @@ final class RemoteSpawnCache implements SpawnCache { private final Path execRoot; private final RemoteOptions options; private final boolean verboseFailures; - - private final RemoteCache remoteCache; - private final String buildRequestId; - private final String commandId; - @Nullable private final Reporter cmdlineReporter; - private final Set reportedErrors = new HashSet<>(); - - private final DigestUtil digestUtil; - private final RemotePathResolver remotePathResolver; - - /** - * If {@link RemoteOutputsMode#TOPLEVEL} is specified it contains the artifacts that should be - * downloaded. - */ - private final ImmutableSet filesToDownload; + private final RemoteExecutionService remoteExecutionService; RemoteSpawnCache( Path execRoot, RemoteOptions options, boolean verboseFailures, - RemoteCache remoteCache, - String buildRequestId, - String commandId, @Nullable Reporter cmdlineReporter, - DigestUtil digestUtil, - ImmutableSet filesToDownload, - RemotePathResolver remotePathResolver) { + RemoteExecutionService remoteExecutionService) { this.execRoot = execRoot; this.options = options; this.verboseFailures = verboseFailures; - this.remoteCache = remoteCache; this.cmdlineReporter = cmdlineReporter; - this.buildRequestId = buildRequestId; - this.commandId = commandId; - this.digestUtil = digestUtil; - this.filesToDownload = Preconditions.checkNotNull(filesToDownload, "filesToDownload"); - this.remotePathResolver = remotePathResolver; + this.remoteExecutionService = remoteExecutionService; } @Override @@ -129,37 +86,11 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) Stopwatch totalTime = Stopwatch.createStarted(); - SortedMap inputMap = remotePathResolver.getInputMapping(context); - MerkleTree merkleTree = - MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil); + RemoteAction action = remoteExecutionService.buildRemoteAction(spawn, context); SpawnMetrics.Builder spawnMetrics = SpawnMetrics.Builder.forRemoteExec() - .setInputBytes(merkleTree.getInputBytes()) - .setInputFiles(merkleTree.getInputFiles()); - Digest merkleTreeRoot = merkleTree.getRootDigest(); - - // Get the remote platform properties. - Platform platform = PlatformUtils.getPlatformProto(spawn, options); - - Command command = - RemoteSpawnRunner.buildCommand( - spawn.getOutputFiles(), - spawn.getArguments(), - spawn.getEnvironment(), - platform, - remotePathResolver); - RemoteOutputsMode remoteOutputsMode = options.remoteOutputsMode; - Action action = - RemoteSpawnRunner.buildAction( - digestUtil.compute(command), merkleTreeRoot, platform, context.getTimeout(), true); - // Look up action cache, and reuse the action output if it is found. - ActionKey actionKey = digestUtil.computeActionKey(action); - - RequestMetadata metadata = - TracingMetadataUtils.buildMetadata( - buildRequestId, commandId, actionKey.getDigest().getHash(), spawn.getResourceOwner()); - RemoteActionExecutionContext remoteActionExecutionContext = - RemoteActionExecutionContext.create(metadata); + .setInputBytes(action.getInputBytes()) + .setInputFiles(action.getInputFiles()); Profiler prof = Profiler.instance(); if (options.remoteAcceptCached @@ -168,55 +99,24 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) // Metadata will be available in context.current() until we detach. // This is done via a thread-local variable. try { - ActionResult result; + RemoteActionResult result; try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { - result = - remoteCache.downloadActionResult( - remoteActionExecutionContext, actionKey, /* inlineOutErr= */ false); + result = remoteExecutionService.lookupCache(action); } // In case the remote cache returned a failed action (exit code != 0) we treat it as a // cache miss if (result != null && result.getExitCode() == 0) { - InMemoryOutput inMemoryOutput = null; - boolean downloadOutputs = - shouldDownloadAllSpawnOutputs( - remoteOutputsMode, - /* exitCode = */ 0, - hasFilesToDownload(spawn.getOutputFiles(), filesToDownload)); Stopwatch fetchTime = Stopwatch.createStarted(); - if (downloadOutputs) { - try (SilentCloseable c = - prof.profile(ProfilerTask.REMOTE_DOWNLOAD, "download outputs")) { - remoteCache.download( - remoteActionExecutionContext, - remotePathResolver, - result, - context.getFileOutErr(), - context::lockOutputFiles); - } - } else { - PathFragment inMemoryOutputPath = getInMemoryOutputPath(spawn); - // inject output metadata - try (SilentCloseable c = - prof.profile(ProfilerTask.REMOTE_DOWNLOAD, "download outputs minimal")) { - inMemoryOutput = - remoteCache.downloadMinimal( - remoteActionExecutionContext, - remotePathResolver, - result, - spawn.getOutputFiles(), - inMemoryOutputPath, - context.getFileOutErr(), - context.getMetadataInjector(), - context::lockOutputFiles); - } + InMemoryOutput inMemoryOutput; + try (SilentCloseable c = prof.profile(REMOTE_DOWNLOAD, "download outputs")) { + inMemoryOutput = remoteExecutionService.downloadOutputs(action, result); } fetchTime.stop(); totalTime.stop(); spawnMetrics .setFetchTime(fetchTime.elapsed()) .setTotalTime(totalTime.elapsed()) - .setNetworkTime(remoteActionExecutionContext.getNetworkTime().getDuration()); + .setNetworkTime(action.getNetworkTime().getDuration()); SpawnResult spawnResult = createSpawnResult( result.getExitCode(), @@ -285,17 +185,8 @@ public void store(SpawnResult result) throws ExecException, InterruptedException } } - Collection files = - RemoteSpawnRunner.resolveActionInputs(execRoot, spawn.getOutputFiles()); try (SilentCloseable c = prof.profile(ProfilerTask.UPLOAD_TIME, "upload outputs")) { - remoteCache.upload( - remoteActionExecutionContext, - remotePathResolver, - actionKey, - action, - command, - files, - context.getFileOutErr()); + remoteExecutionService.uploadOutputs(action); } catch (IOException e) { String errorMessage; if (!verboseFailures) { @@ -316,7 +207,7 @@ public void store(SpawnResult result) throws ExecException, InterruptedException public void close() {} private void checkForConcurrentModifications() throws IOException { - for (ActionInput input : inputMap.values()) { + for (ActionInput input : action.getInputMap().values()) { if (input instanceof VirtualActionInput) { continue; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index a5a0e563671f8b..0dc4c8ab57ddfe 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -19,35 +19,17 @@ import static com.google.devtools.build.lib.profiler.ProfilerTask.REMOTE_EXECUTION; import static com.google.devtools.build.lib.profiler.ProfilerTask.UPLOAD_TIME; import static com.google.devtools.build.lib.remote.util.Utils.createSpawnResult; -import static com.google.devtools.build.lib.remote.util.Utils.getFromFuture; -import static com.google.devtools.build.lib.remote.util.Utils.getInMemoryOutputPath; -import static com.google.devtools.build.lib.remote.util.Utils.hasFilesToDownload; -import static com.google.devtools.build.lib.remote.util.Utils.shouldDownloadAllSpawnOutputs; - -import build.bazel.remote.execution.v2.Action; -import build.bazel.remote.execution.v2.ActionResult; -import build.bazel.remote.execution.v2.Command; -import build.bazel.remote.execution.v2.Digest; + import build.bazel.remote.execution.v2.ExecuteOperationMetadata; -import build.bazel.remote.execution.v2.ExecuteRequest; import build.bazel.remote.execution.v2.ExecuteResponse; import build.bazel.remote.execution.v2.ExecutedActionMetadata; import build.bazel.remote.execution.v2.ExecutionStage.Value; -import build.bazel.remote.execution.v2.LogFile; -import build.bazel.remote.execution.v2.Platform; -import build.bazel.remote.execution.v2.RequestMetadata; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Stopwatch; -import com.google.common.base.Strings; import com.google.common.base.Throwables; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Maps; import com.google.common.util.concurrent.ListeningScheduledExecutorService; import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.Spawn; @@ -56,7 +38,6 @@ import com.google.devtools.build.lib.actions.SpawnResult.Status; import com.google.devtools.build.lib.actions.Spawns; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; -import com.google.devtools.build.lib.analysis.platform.PlatformUtils; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; @@ -67,16 +48,11 @@ import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; import com.google.devtools.build.lib.profiler.SilentCloseable; +import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteAction; +import com.google.devtools.build.lib.remote.RemoteExecutionService.RemoteActionResult; +import com.google.devtools.build.lib.remote.RemoteExecutionService.ServerLogs; import com.google.devtools.build.lib.remote.common.OperationObserver; -import com.google.devtools.build.lib.remote.common.RemoteActionExecutionContext; -import com.google.devtools.build.lib.remote.common.RemoteCacheClient.ActionKey; -import com.google.devtools.build.lib.remote.common.RemoteExecutionClient; -import com.google.devtools.build.lib.remote.common.RemotePathResolver; -import com.google.devtools.build.lib.remote.merkletree.MerkleTree; import com.google.devtools.build.lib.remote.options.RemoteOptions; -import com.google.devtools.build.lib.remote.options.RemoteOutputsMode; -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.Utils; import com.google.devtools.build.lib.remote.util.Utils.InMemoryOutput; import com.google.devtools.build.lib.sandbox.SandboxHelpers; @@ -87,21 +63,15 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.longrunning.Operation; -import com.google.protobuf.Message; import com.google.protobuf.Timestamp; import com.google.protobuf.util.Durations; import com.google.protobuf.util.Timestamps; import io.grpc.Status.Code; import java.io.IOException; import java.time.Duration; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.SortedMap; -import java.util.TreeSet; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import javax.annotation.Nullable; @@ -114,22 +84,10 @@ public class RemoteSpawnRunner implements SpawnRunner { private final RemoteOptions remoteOptions; private final ExecutionOptions executionOptions; private final boolean verboseFailures; - @Nullable private final Reporter cmdlineReporter; - private final RemoteExecutionCache remoteCache; - private final RemoteExecutionClient remoteExecutor; private final RemoteRetrier retrier; - private final String buildRequestId; - private final String commandId; - private final DigestUtil digestUtil; private final Path logDir; - private final RemotePathResolver remotePathResolver; - - /** - * If {@link RemoteOutputsMode#TOPLEVEL} is specified it contains the artifacts that should be - * downloaded. - */ - private final ImmutableSet filesToDownload; + private final RemoteExecutionService remoteExecutionService; // Used to ensure that a warning is reported only once. private final AtomicBoolean warningReported = new AtomicBoolean(); @@ -140,29 +98,17 @@ public class RemoteSpawnRunner implements SpawnRunner { ExecutionOptions executionOptions, boolean verboseFailures, @Nullable Reporter cmdlineReporter, - String buildRequestId, - String commandId, - RemoteExecutionCache remoteCache, - RemoteExecutionClient remoteExecutor, ListeningScheduledExecutorService retryService, - DigestUtil digestUtil, Path logDir, - ImmutableSet filesToDownload, - RemotePathResolver remotePathResolver) { + RemoteExecutionService remoteExecutionService) { this.execRoot = execRoot; this.remoteOptions = remoteOptions; this.executionOptions = executionOptions; - this.remoteCache = Preconditions.checkNotNull(remoteCache, "remoteCache"); - this.remoteExecutor = Preconditions.checkNotNull(remoteExecutor, "remoteExecutor"); this.verboseFailures = verboseFailures; this.cmdlineReporter = cmdlineReporter; - this.buildRequestId = buildRequestId; - this.commandId = commandId; this.retrier = createExecuteRetrier(remoteOptions, retryService); - this.digestUtil = digestUtil; this.logDir = logDir; - this.filesToDownload = Preconditions.checkNotNull(filesToDownload, "filesToDownload"); - this.remotePathResolver = remotePathResolver; + this.remoteExecutionService = remoteExecutionService; } @Override @@ -210,64 +156,32 @@ public void reportExecutingIfNot() { @Override public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) throws ExecException, InterruptedException, IOException { + Preconditions.checkArgument( + Spawns.mayBeExecutedRemotely(spawn), "Spawn can't be executed remotely. This is a bug."); + Stopwatch totalTime = Stopwatch.createStarted(); boolean spawnCacheableRemotely = Spawns.mayBeCachedRemotely(spawn); boolean uploadLocalResults = remoteOptions.remoteUploadLocalResults && spawnCacheableRemotely; boolean acceptCachedResult = remoteOptions.remoteAcceptCached && spawnCacheableRemotely; context.report(ProgressStatus.SCHEDULING, getName()); - RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode; - SortedMap inputMap = remotePathResolver.getInputMapping(context); - final MerkleTree merkleTree = - MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil); + RemoteAction action = remoteExecutionService.buildRemoteAction(spawn, context); SpawnMetrics.Builder spawnMetrics = SpawnMetrics.Builder.forRemoteExec() - .setInputBytes(merkleTree.getInputBytes()) - .setInputFiles(merkleTree.getInputFiles()); - maybeWriteParamFilesLocally(spawn); + .setInputBytes(action.getInputBytes()) + .setInputFiles(action.getInputFiles()); - // Get the remote platform properties. - Platform platform = PlatformUtils.getPlatformProto(spawn, remoteOptions); - - Command command = - buildCommand( - spawn.getOutputFiles(), - spawn.getArguments(), - spawn.getEnvironment(), - platform, - remotePathResolver); - Digest commandHash = digestUtil.compute(command); - Action action = - buildAction( - commandHash, - merkleTree.getRootDigest(), - platform, - context.getTimeout(), - spawnCacheableRemotely); + maybeWriteParamFilesLocally(spawn); spawnMetrics.setParseTime(totalTime.elapsed()); - Preconditions.checkArgument( - Spawns.mayBeExecutedRemotely(spawn), "Spawn can't be executed remotely. This is a bug."); - // Look up action cache, and reuse the action output if it is found. - ActionKey actionKey = digestUtil.computeActionKey(action); - - RequestMetadata metadata = - TracingMetadataUtils.buildMetadata( - buildRequestId, commandId, actionKey.getDigest().getHash(), spawn.getResourceOwner()); - RemoteActionExecutionContext remoteActionExecutionContext = - RemoteActionExecutionContext.create(metadata); Profiler prof = Profiler.instance(); try { // Try to lookup the action in the action cache. - ActionResult cachedResult; + RemoteActionResult cachedResult; try (SilentCloseable c = prof.profile(ProfilerTask.REMOTE_CACHE_CHECK, "check cache hit")) { - cachedResult = - acceptCachedResult - ? remoteCache.downloadActionResult( - remoteActionExecutionContext, actionKey, /* inlineOutErr= */ false) - : null; + cachedResult = acceptCachedResult ? remoteExecutionService.lookupCache(action) : null; } if (cachedResult != null) { if (cachedResult.getExitCode() != 0) { @@ -278,14 +192,12 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) } else { try { return downloadAndFinalizeSpawnResult( - remoteActionExecutionContext, + action, cachedResult, /* cacheHit= */ true, spawn, - context, - remoteOutputsMode, totalTime, - () -> remoteActionExecutionContext.getNetworkTime().getDuration(), + () -> action.getNetworkTime().getDuration(), spawnMetrics); } catch (BulkTransferException e) { if (!e.onlyCausedByCacheNotFoundException()) { @@ -298,64 +210,31 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) } } } catch (IOException e) { - return execLocallyAndUploadOrFail( - remoteActionExecutionContext, - spawn, - context, - inputMap, - actionKey, - action, - command, - uploadLocalResults, - e); + return execLocallyAndUploadOrFail(action, spawn, context, uploadLocalResults, e); } - ExecuteRequest.Builder requestBuilder = - ExecuteRequest.newBuilder() - .setInstanceName(remoteOptions.remoteInstanceName) - .setActionDigest(actionKey.getDigest()) - .setSkipCacheLookup(!acceptCachedResult); - if (remoteOptions.remoteResultCachePriority != 0) { - requestBuilder - .getResultsCachePolicyBuilder() - .setPriority(remoteOptions.remoteResultCachePriority); - } - if (remoteOptions.remoteExecutionPriority != 0) { - requestBuilder.getExecutionPolicyBuilder().setPriority(remoteOptions.remoteExecutionPriority); - } + AtomicBoolean useCachedResult = new AtomicBoolean(acceptCachedResult); try { return retrier.execute( () -> { - ExecuteRequest request = requestBuilder.build(); - // Upload the command and all the inputs into the remote cache. try (SilentCloseable c = prof.profile(UPLOAD_TIME, "upload missing inputs")) { - Map additionalInputs = Maps.newHashMapWithExpectedSize(2); - additionalInputs.put(actionKey.getDigest(), action); - additionalInputs.put(commandHash, command); - Duration networkTimeStart = - remoteActionExecutionContext.getNetworkTime().getDuration(); + Duration networkTimeStart = action.getNetworkTime().getDuration(); Stopwatch uploadTime = Stopwatch.createStarted(); - remoteCache.ensureInputsPresent( - remoteActionExecutionContext, merkleTree, additionalInputs); + remoteExecutionService.uploadInputsIfNotPresent(action); // subtract network time consumed here to ensure wall clock during upload is not // double // counted, and metrics time computation does not exceed total time spawnMetrics.setUploadTime( uploadTime .elapsed() - .minus( - remoteActionExecutionContext - .getNetworkTime() - .getDuration() - .minus(networkTimeStart))); + .minus(action.getNetworkTime().getDuration().minus(networkTimeStart))); } ExecutingStatusReporter reporter = new ExecutingStatusReporter(context); - ExecuteResponse reply; + RemoteActionResult result; try (SilentCloseable c = prof.profile(REMOTE_EXECUTION, "execute remotely")) { - reply = - remoteExecutor.executeRemotely(remoteActionExecutionContext, request, reporter); + result = remoteExecutionService.execute(action, useCachedResult.get(), reporter); } // In case of replies from server contains metadata, but none of them has EXECUTING // status. @@ -363,50 +242,37 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) reporter.reportExecutingIfNot(); FileOutErr outErr = context.getFileOutErr(); - String message = reply.getMessage(); - ActionResult actionResult = reply.getResult(); - if ((actionResult.getExitCode() != 0 || reply.getStatus().getCode() != Code.OK.value()) - && !message.isEmpty()) { + String message = result.getMessage(); + if (!result.success() && !message.isEmpty()) { outErr.printErr(message + "\n"); } - spawnMetricsAccounting(spawnMetrics, actionResult.getExecutionMetadata()); + spawnMetricsAccounting(spawnMetrics, result.getExecutionMetadata()); try (SilentCloseable c = prof.profile(REMOTE_DOWNLOAD, "download server logs")) { - maybeDownloadServerLogs(remoteActionExecutionContext, reply, actionKey); + maybeDownloadServerLogs(action, result.getResponse()); } try { return downloadAndFinalizeSpawnResult( - remoteActionExecutionContext, - actionResult, - reply.getCachedResult(), + action, + result, + result.cacheHit(), spawn, - context, - remoteOutputsMode, totalTime, - () -> remoteActionExecutionContext.getNetworkTime().getDuration(), + () -> action.getNetworkTime().getDuration(), spawnMetrics); } catch (BulkTransferException e) { if (e.onlyCausedByCacheNotFoundException()) { // No cache hit, so if we retry this execution, we must no longer accept // cached results, it must be reexecuted - requestBuilder.setSkipCacheLookup(true); + useCachedResult.set(false); } throw e; } }); } catch (IOException e) { - return execLocallyAndUploadOrFail( - remoteActionExecutionContext, - spawn, - context, - inputMap, - actionKey, - action, - command, - uploadLocalResults, - e); + return execLocallyAndUploadOrFail(action, spawn, context, uploadLocalResults, e); } } @@ -456,56 +322,29 @@ static void spawnMetricsAccounting( } private SpawnResult downloadAndFinalizeSpawnResult( - RemoteActionExecutionContext remoteActionExecutionContext, - ActionResult actionResult, + RemoteAction action, + RemoteActionResult result, boolean cacheHit, Spawn spawn, - SpawnExecutionContext context, - RemoteOutputsMode remoteOutputsMode, Stopwatch totalTime, Supplier networkTime, SpawnMetrics.Builder spawnMetrics) throws ExecException, IOException, InterruptedException { - boolean downloadOutputs = - shouldDownloadAllSpawnOutputs( - remoteOutputsMode, - /* exitCode = */ actionResult.getExitCode(), - hasFilesToDownload(spawn.getOutputFiles(), filesToDownload)); - InMemoryOutput inMemoryOutput = null; Duration networkTimeStart = networkTime.get(); Stopwatch fetchTime = Stopwatch.createStarted(); - if (downloadOutputs) { - try (SilentCloseable c = Profiler.instance().profile(REMOTE_DOWNLOAD, "download outputs")) { - remoteCache.download( - remoteActionExecutionContext, - remotePathResolver, - actionResult, - context.getFileOutErr(), - context::lockOutputFiles); - } - } else { - PathFragment inMemoryOutputPath = getInMemoryOutputPath(spawn); - try (SilentCloseable c = - Profiler.instance().profile(REMOTE_DOWNLOAD, "download outputs minimal")) { - inMemoryOutput = - remoteCache.downloadMinimal( - remoteActionExecutionContext, - remotePathResolver, - actionResult, - spawn.getOutputFiles(), - inMemoryOutputPath, - context.getFileOutErr(), - context.getMetadataInjector(), - context::lockOutputFiles); - } + + InMemoryOutput inMemoryOutput; + try (SilentCloseable c = Profiler.instance().profile(REMOTE_DOWNLOAD, "download outputs")) { + inMemoryOutput = remoteExecutionService.downloadOutputs(action, result); } + fetchTime.stop(); totalTime.stop(); Duration networkTimeEnd = networkTime.get(); // subtract network time consumed here to ensure wall clock during fetch is not double // counted, and metrics time computation does not exceed total time return createSpawnResult( - actionResult.getExitCode(), + result.getExitCode(), cacheHit, getName(), inMemoryOutput, @@ -540,30 +379,18 @@ private void maybeWriteParamFilesLocally(Spawn spawn) throws IOException { } } - private void maybeDownloadServerLogs( - RemoteActionExecutionContext context, ExecuteResponse resp, ActionKey actionKey) + private void maybeDownloadServerLogs(RemoteAction action, ExecuteResponse resp) throws InterruptedException { - ActionResult result = resp.getResult(); - if (resp.getServerLogsCount() > 0 - && (result.getExitCode() != 0 || resp.getStatus().getCode() != Code.OK.value())) { - Path parent = logDir.getRelative(actionKey.getDigest().getHash()); - Path logPath = null; - int logCount = 0; - for (Map.Entry e : resp.getServerLogsMap().entrySet()) { - if (e.getValue().getHumanReadable()) { - logPath = parent.getRelative(e.getKey()); - logCount++; - try { - getFromFuture(remoteCache.downloadFile(context, logPath, e.getValue().getDigest())); - } catch (IOException ex) { - reportOnce(Event.warn("Failed downloading server logs from the remote cache.")); - } - } - } - if (logCount > 0 && verboseFailures) { + try { + ServerLogs serverLogs = remoteExecutionService.maybeDownloadServerLogs(action, resp, logDir); + if (serverLogs.logCount > 0 && verboseFailures) { report( - Event.info("Server logs of failing action:\n " + (logCount > 1 ? parent : logPath))); + Event.info( + "Server logs of failing action:\n " + + (serverLogs.logCount > 1 ? serverLogs.directory : serverLogs.lastLogPath))); } + } catch (IOException e) { + reportOnce(Event.warn("Failed downloading server logs from the remote cache.")); } } @@ -581,13 +408,9 @@ private SpawnResult execLocally(Spawn spawn, SpawnExecutionContext context) } private SpawnResult execLocallyAndUploadOrFail( - RemoteActionExecutionContext remoteActionExecutionContext, + RemoteAction action, Spawn spawn, SpawnExecutionContext context, - SortedMap inputMap, - ActionKey actionKey, - Action action, - Command command, boolean uploadLocalResults, IOException cause) throws ExecException, InterruptedException, IOException { @@ -597,26 +420,12 @@ private SpawnResult execLocallyAndUploadOrFail( throw new InterruptedException(); } if (remoteOptions.remoteLocalFallback && !RemoteRetrierUtils.causedByExecTimeout(cause)) { - return execLocallyAndUpload( - remoteActionExecutionContext, - spawn, - context, - inputMap, - actionKey, - action, - command, - uploadLocalResults); + return execLocallyAndUpload(action, spawn, context, uploadLocalResults); } - return handleError( - remoteActionExecutionContext, cause, context.getFileOutErr(), actionKey, context); + return handleError(action, cause); } - private SpawnResult handleError( - RemoteActionExecutionContext remoteActionExecutionContext, - IOException exception, - FileOutErr outErr, - ActionKey actionKey, - SpawnExecutionContext context) + private SpawnResult handleError(RemoteAction action, IOException exception) throws ExecException, InterruptedException, IOException { boolean remoteCacheFailed = BulkTransferException.isOnlyCausedByCacheNotFoundException(exception); @@ -624,16 +433,11 @@ private SpawnResult handleError( ExecutionStatusException e = (ExecutionStatusException) exception.getCause(); if (e.getResponse() != null) { ExecuteResponse resp = e.getResponse(); - maybeDownloadServerLogs(remoteActionExecutionContext, resp, actionKey); + maybeDownloadServerLogs(action, resp); if (resp.hasResult()) { try { - // We try to download all (partial) results even on server error, for debuggability. - remoteCache.download( - remoteActionExecutionContext, - remotePathResolver, - resp.getResult(), - outErr, - context::lockOutputFiles); + remoteExecutionService.downloadOutputs( + action, RemoteActionResult.createFromResponse(resp)); } catch (BulkTransferException bulkTransferEx) { exception.addSuppressed(bulkTransferEx); } @@ -693,67 +497,6 @@ private SpawnResult handleError( .build(); } - static Action buildAction( - Digest command, - Digest inputRoot, - @Nullable Platform platform, - Duration timeout, - boolean cacheable) { - - Action.Builder action = Action.newBuilder(); - action.setCommandDigest(command); - action.setInputRootDigest(inputRoot); - if (!timeout.isZero()) { - action.setTimeout(com.google.protobuf.Duration.newBuilder().setSeconds(timeout.getSeconds())); - } - if (!cacheable) { - action.setDoNotCache(true); - } - if (platform != null) { - action.setPlatform(platform); - } - return action.build(); - } - - static Command buildCommand( - Collection outputs, - List arguments, - ImmutableMap env, - @Nullable Platform platform, - RemotePathResolver remotePathResolver) { - Command.Builder command = Command.newBuilder(); - ArrayList outputFiles = new ArrayList<>(); - ArrayList outputDirectories = new ArrayList<>(); - for (ActionInput output : outputs) { - String pathString = remotePathResolver.localPathToOutputPath(output); - if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) { - outputDirectories.add(pathString); - } else { - outputFiles.add(pathString); - } - } - Collections.sort(outputFiles); - Collections.sort(outputDirectories); - command.addAllOutputFiles(outputFiles); - command.addAllOutputDirectories(outputDirectories); - - if (platform != null) { - command.setPlatform(platform); - } - command.addAllArguments(arguments); - // Sorting the environment pairs by variable name. - TreeSet variables = new TreeSet<>(env.keySet()); - for (String var : variables) { - command.addEnvironmentVariablesBuilder().setName(var).setValue(env.get(var)); - } - - String workingDirectory = remotePathResolver.getWorkingDirectory(); - if (!Strings.isNullOrEmpty(workingDirectory)) { - command.setWorkingDirectory(workingDirectory); - } - return command.build(); - } - private Map getInputCtimes(SortedMap inputMap) { HashMap ctimes = new HashMap<>(); for (Map.Entry e : inputMap.entrySet()) { @@ -776,18 +519,11 @@ private Map getInputCtimes(SortedMap inpu @VisibleForTesting SpawnResult execLocallyAndUpload( - RemoteActionExecutionContext remoteActionExecutionContext, - Spawn spawn, - SpawnExecutionContext context, - SortedMap inputMap, - ActionKey actionKey, - Action action, - Command command, - boolean uploadLocalResults) + RemoteAction action, Spawn spawn, SpawnExecutionContext context, boolean uploadLocalResults) throws ExecException, IOException, InterruptedException { - Map ctimesBefore = getInputCtimes(inputMap); + Map ctimesBefore = getInputCtimes(action.getInputMap()); SpawnResult result = execLocally(spawn, context); - Map ctimesAfter = getInputCtimes(inputMap); + Map ctimesAfter = getInputCtimes(action.getInputMap()); uploadLocalResults = uploadLocalResults && Status.SUCCESS.equals(result.status()) && result.exitCode() == 0; if (!uploadLocalResults) { @@ -801,16 +537,8 @@ SpawnResult execLocallyAndUpload( } } - Collection outputFiles = resolveActionInputs(execRoot, spawn.getOutputFiles()); try (SilentCloseable c = Profiler.instance().profile(UPLOAD_TIME, "upload outputs")) { - remoteCache.upload( - remoteActionExecutionContext, - remotePathResolver, - actionKey, - action, - command, - outputFiles, - context.getFileOutErr()); + remoteExecutionService.uploadOutputs(action); } catch (IOException e) { if (verboseFailures) { report(Event.debug("Upload to remote cache failed: " + e.getMessage())); @@ -833,17 +561,6 @@ private void report(Event evt) { } } - /** - * Resolve a collection of {@link com.google.devtools.build.lib.actions.ActionInput}s to {@link - * Path}s. - */ - static Collection resolveActionInputs( - Path execRoot, Collection actionInputs) { - return actionInputs.stream() - .map((inp) -> execRoot.getRelative(inp.getExecPath())) - .collect(ImmutableList.toImmutableList()); - } - private static RemoteRetrier createExecuteRetrier( RemoteOptions options, ListeningScheduledExecutorService retryService) { return new ExecuteRetrier( diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java index f5ce5077c5c4a0..8991267c4246b0 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java +++ b/src/main/java/com/google/devtools/build/lib/remote/util/Utils.java @@ -15,8 +15,10 @@ import static java.util.stream.Collectors.joining; +import build.bazel.remote.execution.v2.Action; import build.bazel.remote.execution.v2.ActionResult; import build.bazel.remote.execution.v2.Digest; +import build.bazel.remote.execution.v2.Platform; import com.google.common.base.Ascii; import com.google.common.base.Preconditions; import com.google.common.base.Throwables; @@ -397,6 +399,27 @@ public static void verifyBlobContents(Digest expected, Digest actual) throws IOE } } + public static Action buildAction( + Digest command, + Digest inputRoot, + @Nullable Platform platform, + java.time.Duration timeout, + boolean cacheable) { + Action.Builder action = Action.newBuilder(); + action.setCommandDigest(command); + action.setInputRootDigest(inputRoot); + if (!timeout.isZero()) { + action.setTimeout(Duration.newBuilder().setSeconds(timeout.getSeconds())); + } + if (!cacheable) { + action.setDoNotCache(true); + } + if (platform != null) { + action.setPlatform(platform); + } + return action.build(); + } + /** An in-memory output file. */ public static final class InMemoryOutput { private final ActionInput output; diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 52c90933251f2d..69b54bde4811e0 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -104,6 +104,7 @@ public class RemoteSpawnCacheTest { private FileSystem fs; private DigestUtil digestUtil; private Path execRoot; + private RemotePathResolver remotePathResolver; private SimpleSpawn simpleSpawn; private FakeActionInputFileCache fakeFileCache; @Mock private RemoteCache remoteCache; @@ -200,17 +201,19 @@ private static SimpleSpawn simpleSpawnWithExecutionInfo( } private RemoteSpawnCache remoteSpawnCacheWithOptions(RemoteOptions options) { + RemoteExecutionService remoteExecutionService = + new RemoteExecutionService( + execRoot, + remotePathResolver, + BUILD_REQUEST_ID, + COMMAND_ID, + digestUtil, + options, + remoteCache, + null, + ImmutableSet.of()); return new RemoteSpawnCache( - execRoot, - options, - /* verboseFailures=*/ true, - remoteCache, - BUILD_REQUEST_ID, - COMMAND_ID, - reporter, - digestUtil, - /* filesToDownload= */ ImmutableSet.of(), - RemotePathResolver.createDefault(execRoot)); + execRoot, options, /* verboseFailures=*/ true, reporter, remoteExecutionService); } @Before @@ -219,6 +222,7 @@ public final void setUp() throws Exception { fs = new InMemoryFileSystem(new JavaClock(), DigestHashFunction.SHA256); digestUtil = new DigestUtil(DigestHashFunction.SHA256); execRoot = fs.getPath("/exec/root"); + remotePathResolver = RemotePathResolver.createDefault(execRoot); FileSystemUtils.createDirectoryAndParents(execRoot); fakeFileCache = new FakeActionInputFileCache(execRoot); simpleSpawn = simpleSpawnWithExecutionInfo(ImmutableMap.of()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java index e8df2436ae38e5..37bb35d731956e 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java @@ -296,15 +296,7 @@ public void cachableSpawnsShouldBeCached_localFallback() throws Exception { assertThat(result.status()).isEqualTo(Status.SUCCESS); verify(localRunner).exec(eq(spawn), eq(policy)); verify(runner) - .execLocallyAndUpload( - any(), - eq(spawn), - eq(policy), - any(), - any(), - any(), - any(), - /* uploadLocalResults= */ eq(true)); + .execLocallyAndUpload(any(), eq(spawn), eq(policy), /* uploadLocalResults= */ eq(true)); verify(cache).upload(any(), any(), any(), any(), any(), any(), any()); } @@ -336,15 +328,7 @@ public void failedLocalActionShouldNotBeUploaded() throws Exception { verify(localRunner).exec(eq(spawn), eq(policy)); verify(runner) - .execLocallyAndUpload( - any(), - eq(spawn), - eq(policy), - any(), - any(), - any(), - any(), - /* uploadLocalResults= */ eq(true)); + .execLocallyAndUpload(any(), eq(spawn), eq(policy), /* uploadLocalResults= */ eq(true)); verify(cache, never()).upload(any(), any(), any(), any(), any(), any(), any()); } @@ -386,15 +370,7 @@ public void treatFailedCachedActionAsCacheMiss_local() throws Exception { verify(localRunner).exec(eq(spawn), eq(policy)); verify(runner) - .execLocallyAndUpload( - any(), - eq(spawn), - eq(policy), - any(), - any(), - any(), - any(), - /* uploadLocalResults= */ eq(true)); + .execLocallyAndUpload(any(), eq(spawn), eq(policy), /* uploadLocalResults= */ eq(true)); verify(cache).upload(any(), any(), any(), any(), any(), any(), any()); verify(cache, never()) .download( @@ -1100,24 +1076,30 @@ public void testMaterializeParamFilesIsImpliedBySubcommands() throws Exception { } private void testParamFilesAreMaterializedForFlag(String flag) throws Exception { + RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); ExecutionOptions executionOptions = Options.parse(ExecutionOptions.class, flag).getOptions(); executionOptions.materializeParamFiles = true; + RemoteExecutionService remoteExecutionService = + new RemoteExecutionService( + execRoot, + RemotePathResolver.createDefault(execRoot), + "build-req-id", + "command-id", + digestUtil, + remoteOptions, + cache, + executor, + ImmutableSet.of()); RemoteSpawnRunner runner = new RemoteSpawnRunner( execRoot, - Options.getDefaults(RemoteOptions.class), + remoteOptions, executionOptions, true, /*cmdlineReporter=*/ null, - "build-req-id", - "command-id", - cache, - executor, retryService, - digestUtil, logDir, - /* filesToDownload= */ ImmutableSet.of(), - RemotePathResolver.createDefault(execRoot)); + remoteExecutionService); ExecuteResponse succeeded = ExecuteResponse.newBuilder() @@ -1640,20 +1622,25 @@ private RemoteSpawnRunner newSpawnRunner( @Nullable Reporter reporter, ImmutableSet topLevelOutputs, RemotePathResolver remotePathResolver) { + RemoteExecutionService remoteExecutionService = + new RemoteExecutionService( + execRoot, + remotePathResolver, + "build-req-id", + "command-id", + digestUtil, + remoteOptions, + cache, + executor, + topLevelOutputs); return new RemoteSpawnRunner( execRoot, remoteOptions, Options.getDefaults(ExecutionOptions.class), verboseFailures, reporter, - "build-req-id", - "command-id", - cache, - executor, retryService, - digestUtil, logDir, - topLevelOutputs, - remotePathResolver); + remoteExecutionService); } } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java index b87f98c662efab..2bd17d5c34218f 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerWithGrpcRemoteExecutorTest.java @@ -296,6 +296,17 @@ public int maxConcurrency() { uploader); RemoteExecutionCache remoteCache = new RemoteExecutionCache(cacheProtocol, remoteOptions, DIGEST_UTIL); + RemoteExecutionService remoteExecutionService = + new RemoteExecutionService( + execRoot, + RemotePathResolver.createDefault(execRoot), + "build-req-id", + "command-id", + DIGEST_UTIL, + remoteOptions, + remoteCache, + executor, + /* filesToDownload= */ ImmutableSet.of()); client = new RemoteSpawnRunner( execRoot, @@ -303,15 +314,9 @@ public int maxConcurrency() { Options.getDefaults(ExecutionOptions.class), /* verboseFailures= */ true, /*cmdlineReporter=*/ null, - "build-req-id", - "command-id", - remoteCache, - executor, retryService, - DIGEST_UTIL, logDir, - /* filesToDownload= */ ImmutableSet.of(), - RemotePathResolver.createDefault(execRoot)); + remoteExecutionService); inputDigest = fakeFileCache.createScratchInput(simpleSpawn.getInputFiles().getSingleton(), "xyz");