From 9626bb4923c74c6d3c09b7438eb24b32191053df Mon Sep 17 00:00:00 2001 From: buchgr Date: Fri, 11 Aug 2017 13:54:04 +0200 Subject: [PATCH] remote: Refactor RemoteSpawnRunner to distinquish between remote executor, remote cache and local executor errors. This is a no-op refactoring and clears the way to integrate a change that no longer uploads to the remote cache if a previous remote cache interaction failed. RELNOTES: None. PiperOrigin-RevId: 164966432 --- .../build/lib/remote/RemoteSpawnRunner.java | 142 +++++++++++------- 1 file changed, 90 insertions(+), 52 deletions(-) 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 f6d79ad45cd579..f09d496a31ebc7 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 @@ -31,6 +31,7 @@ import com.google.devtools.build.lib.exec.SpawnRunner; import com.google.devtools.build.lib.remote.Digests.ActionKey; import com.google.devtools.build.lib.remote.TreeNodeRepository.TreeNode; +import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.remoteexecution.v1test.Action; @@ -107,73 +108,110 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) // Look up action cache, and reuse the action output if it is found. ActionKey actionKey = Digests.computeActionKey(action); + boolean acceptCachedResult = options.remoteAcceptCached && Spawns.mayBeCached(spawn); + boolean uploadLocalResults = options.remoteUploadLocalResults; + try { - boolean acceptCachedResult = options.remoteAcceptCached && Spawns.mayBeCached(spawn); - ActionResult result = + // Try to lookup the action in the action cache. + ActionResult cachedResult = acceptCachedResult ? remoteCache.getCachedActionResult(actionKey) : null; - if (result != null) { - // We don't cache failed actions, so we know the outputs exist. - // For now, download all outputs locally; in the future, we can reuse the digests to - // just update the TreeNodeRepository and continue the build. + if (cachedResult != null) { try { - remoteCache.download(result, execRoot, policy.getFileOutErr()); - return new SpawnResult.Builder() - .setStatus(Status.SUCCESS) // Even if the action failed with non-zero exit code. - .setExitCode(result.getExitCode()) - .build(); + return downloadRemoteResults(cachedResult, policy.getFileOutErr()); } catch (CacheNotFoundException e) { - acceptCachedResult = false; // Retry the action remotely and invalidate the results. + // Intentionally left empty. No cache hit, so we fall through to local or + // remote execution. } } + } catch (IOException e) { + return execLocallyOrFail(spawn, policy, inputMap, actionKey, + options.remoteUploadLocalResults, e); + } - if (remoteExecutor == null) { - return execLocally(spawn, policy, inputMap, options.remoteUploadLocalResults, - remoteCache, actionKey); - } + if (remoteExecutor == null) { + // Remote execution is disabled and so execute the spawn on the local machine. + return execLocally(spawn, policy, inputMap, uploadLocalResults, remoteCache, actionKey); + } + try { // Upload the command and all the inputs into the remote cache. remoteCache.ensureInputsPresent(repository, execRoot, inputRoot, command); - // TODO(olaola): set BuildInfo and input total bytes as well. - ExecuteRequest.Builder request = - ExecuteRequest.newBuilder() - .setInstanceName(options.remoteInstanceName) - .setAction(action) - .setTotalInputFileCount(inputMap.size()) - .setSkipCacheLookup(!acceptCachedResult); - ExecuteResponse reply = remoteExecutor.executeRemotely(request.build()); - result = reply.getResult(); - if (options.remoteLocalFallback && result.getExitCode() != 0) { - return execLocally(spawn, policy, inputMap, options.remoteUploadLocalResults, - remoteCache, actionKey); - } - remoteCache.download(result, execRoot, policy.getFileOutErr()); - return new SpawnResult.Builder() - .setStatus(Status.SUCCESS) // Even if the action failed with non-zero exit code. - .setExitCode(result.getExitCode()) - .build(); } catch (IOException e) { - if (options.remoteLocalFallback) { - return execLocally(spawn, policy, inputMap, options.remoteUploadLocalResults, - remoteCache, actionKey); - } + return execLocallyOrFail(spawn, policy, inputMap, actionKey, + options.remoteUploadLocalResults, e); + } - String message = ""; - if (e instanceof RetryException - && ((RetryException) e).causedByStatusCode(Code.UNAVAILABLE)) { - message = "The remote executor/cache is unavailable"; - } else if (e instanceof CacheNotFoundException) { - message = "Failed to download from remote cache"; - } else { - message = "Error in remote cache/executor"; - } - // TODO(olaola): reuse the ErrorMessage class for these errors. - if (verboseFailures) { - message += "\n" + Throwables.getStackTraceAsString(e); - } - throw new EnvironmentalExecException(message, e, true); + final ActionResult result; + try { + result = executeRemotely(action, inputMap.size(), acceptCachedResult); + } catch (IOException e) { + return execLocallyOrFail(spawn, policy, inputMap, actionKey, uploadLocalResults, e); + } + + boolean executionFailed = result.getExitCode() != 0; + if (options.remoteLocalFallback && executionFailed) { + return execLocally(spawn, policy, inputMap, options.remoteUploadLocalResults, + remoteCache, actionKey); + } + + try { + return downloadRemoteResults(result, policy.getFileOutErr()); + } catch (IOException e) { + return execLocallyOrFail(spawn, policy, inputMap, actionKey, + options.remoteUploadLocalResults, e); + } + } + + private SpawnResult downloadRemoteResults(ActionResult result, FileOutErr outErr) + throws ExecException, IOException, InterruptedException { + remoteCache.download(result, execRoot, outErr); + return new SpawnResult.Builder() + .setStatus(Status.SUCCESS) // Even if the action failed with non-zero exit code. + .setExitCode(result.getExitCode()) + .build(); + } + + private ActionResult executeRemotely(Action action, int numInputFiles, + boolean acceptCachedResult) throws IOException, InterruptedException { + // TODO(olaola): set BuildInfo and input total bytes as well. + ExecuteRequest.Builder request = + ExecuteRequest.newBuilder() + .setInstanceName(options.remoteInstanceName) + .setAction(action) + .setTotalInputFileCount(numInputFiles) + .setSkipCacheLookup(!acceptCachedResult); + ExecuteResponse reply = remoteExecutor.executeRemotely(request.build()); + return reply.getResult(); + } + + private SpawnResult execLocallyOrFail(Spawn spawn, SpawnExecutionPolicy policy, + SortedMap inputMap, ActionKey actionKey, + boolean uploadLocalResults, IOException cause) + throws ExecException, InterruptedException, IOException { + if (options.remoteLocalFallback) { + return execLocally(spawn, policy, inputMap, uploadLocalResults, + remoteCache, actionKey); + } + throw new EnvironmentalExecException(errorMessage(cause), cause, true); + } + + private String errorMessage(IOException e) { + String message = ""; + if (e instanceof RetryException + && ((RetryException) e).causedByStatusCode(Code.UNAVAILABLE)) { + message = "The remote executor/cache is unavailable"; + } else if (e instanceof CacheNotFoundException) { + message = "Failed to download from remote cache"; + } else { + message = "Error in remote cache/executor"; + } + // TODO(olaola): reuse the ErrorMessage class for these errors. + if (verboseFailures) { + message += "\n" + Throwables.getStackTraceAsString(e); } + return message; } static Action buildAction(