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");