diff options
Diffstat (limited to 'src/main/java/com/google')
6 files changed, 70 insertions, 72 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java index 8040946774..c130de86c2 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java +++ b/src/main/java/com/google/devtools/build/lib/actions/SpawnResult.java @@ -33,12 +33,12 @@ import javax.annotation.Nullable; public interface SpawnResult { /** The status of the attempted Spawn execution. */ public enum Status { - /** - * Subprocess executed successfully, but may have returned a non-zero exit code. See - * {@link #exitCode} for the actual exit code. - */ + /** Subprocess executed successfully, and returned a zero exit code. */ SUCCESS(true), + /** Subprocess executed successfully, but returned a non-zero exit code. */ + NON_ZERO_EXIT(true), + /** Subprocess execution timed out. */ TIMEOUT(true), @@ -49,46 +49,22 @@ public interface SpawnResult { OUT_OF_MEMORY(true), /** - * Subprocess did not execute for an unknown reason - only use this if none of the more specific - * status codes apply. + * Subprocess did not execute. This error is not catastrophic - Bazel will try to continue the + * build if keep_going is enabled, possibly attempt to rerun the same spawn, and attempt to run + * other actions. */ EXECUTION_FAILED, - /** The attempted subprocess was disallowed by a user setting. */ - LOCAL_ACTION_NOT_ALLOWED(true), - - /** The Spawn referred to an non-existent absolute or relative path. */ - COMMAND_NOT_FOUND, - /** - * One of the Spawn inputs was a directory. For backwards compatibility, some - * {@link SpawnRunner} implementations may attempt to run the subprocess anyway. Note that this - * leads to incremental correctness issues, as Bazel does not track dependencies on directories. + * Subprocess did not execute. Do not retry, and exit immediately even if keep_going is enabled. */ - DIRECTORY_AS_INPUT_DISALLOWED(true), + EXECUTION_FAILED_CATASTROPHICALLY, /** - * Too many input files - remote execution systems may refuse to execute subprocesses with an - * excessive number of input files. + * Subprocess did not execute in a way that indicates that the user can fix it. For example, a + * remote system may have denied the execution due to too many inputs or too large inputs. */ - TOO_MANY_INPUT_FILES(true), - - /** - * Total size of inputs is too large - remote execution systems may refuse to execute - * subprocesses if the total size of all inputs exceeds a limit. - */ - INPUTS_TOO_LARGE(true), - - /** - * One of the input files to the Spawn was modified during the build - some {@link SpawnRunner} - * implementations cache checksums and may detect such modifications on a best effort basis. - */ - FILE_MODIFIED_DURING_BUILD(true), - - /** - * The {@link SpawnRunner} was unable to establish a required network connection. - */ - CONNECTION_FAILED, + EXECUTION_DENIED(true), /** * The remote execution system is overloaded and had to refuse execution for this Spawn. @@ -99,18 +75,7 @@ public interface SpawnResult { * The result of the remotely executed Spawn could not be retrieved due to errors in the remote * caching layer. */ - REMOTE_CACHE_FAILED, - - /** - * The remote execution system did not allow the request due to missing authorization or - * authentication. - */ - NOT_AUTHORIZED(true), - - /** - * The Spawn was malformed. - */ - INVALID_ARGUMENT; + REMOTE_CACHE_FAILED; private final boolean isUserError; @@ -128,21 +93,30 @@ public interface SpawnResult { } /** - * Returns whether the spawn was actually run, regardless of the exit code. I.e., returns if - * status == SUCCESS || status == TIMEOUT || status == OUT_OF_MEMORY. Returns false if there were - * errors that prevented the spawn from being run, such as network errors, missing local files, - * errors setting up sandboxing, etc. + * Returns whether the spawn was actually run, regardless of the exit code. I.e., returns + * {@code true} if {@link #status} is any of {@link Status#SUCCESS}, {@link Status#NON_ZERO_EXIT}, + * {@link Status#TIMEOUT} or {@link Status#OUT_OF_MEMORY}. + * + * <p>Returns false if there were errors that prevented the spawn from being run, such as network + * errors, missing local files, errors setting up sandboxing, etc. */ boolean setupSuccess(); + /** Returns true if the status was {@link Status#EXECUTION_FAILED_CATASTROPHICALLY}. */ boolean isCatastrophe(); /** The status of the attempted Spawn execution. */ Status status(); /** - * The exit code of the subprocess if the subprocess was executed. Check {@link #status} for - * {@link Status#SUCCESS} before calling this method. + * The exit code of the subprocess if the subprocess was executed. Should only be called if + * {@link #status} returns {@link Status#SUCCESS} or {@link Status#NON_ZERO_EXIT}. + * + * <p>This method must return a non-zero exit code if the status is {@link Status#TIMEOUT} or + * {@link Status#OUT_OF_MEMORY}. It is recommended to return 128 + 14 when the status is + * {@link Status#TIMEOUT}, which corresponds to the Unix signal SIGALRM. + * + * <p>This method may throw {@link IllegalStateException} if called for any other status. */ int exitCode(); @@ -208,12 +182,15 @@ public interface SpawnResult { @Override public boolean setupSuccess() { - return status == Status.SUCCESS || status == Status.TIMEOUT || status == Status.OUT_OF_MEMORY; + return status == Status.SUCCESS + || status == Status.NON_ZERO_EXIT + || status == Status.TIMEOUT + || status == Status.OUT_OF_MEMORY; } @Override public boolean isCatastrophe() { - return false; + return status == Status.EXECUTION_FAILED_CATASTROPHICALLY; } @Override @@ -295,6 +272,14 @@ public interface SpawnResult { private boolean cacheHit; public SpawnResult build() { + if (status == Status.SUCCESS) { + Preconditions.checkArgument(exitCode == 0); + } + if (status == Status.NON_ZERO_EXIT + || status == Status.TIMEOUT + || status == Status.OUT_OF_MEMORY) { + Preconditions.checkArgument(exitCode != 0); + } return new SimpleSpawnResult(this); } diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index 0f1cf259c0..1512a0139c 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -105,7 +105,7 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte throw new EnvironmentalExecException("Unexpected IO error.", e); } - if ((spawnResult.status() != Status.SUCCESS) || (spawnResult.exitCode() != 0)) { + if (spawnResult.status() != Status.SUCCESS) { String cwd = actionExecutionContext.getExecRoot().getPathString(); String message = CommandFailureUtils.describeCommandFailure( diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java index e33c010263..e99b78b48e 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java @@ -239,7 +239,7 @@ public final class LocalSpawnRunner implements SpawnRunner { ("Action type " + actionType + " is not allowed to run locally due to regex filter: " + localExecutionOptions.allowedLocalAction + "\n").getBytes(UTF_8)); return new SpawnResult.Builder() - .setStatus(Status.LOCAL_ACTION_NOT_ALLOWED) + .setStatus(Status.EXECUTION_DENIED) .setExitCode(LOCAL_EXEC_ERROR) .setExecutorHostname(hostName) .build(); @@ -322,11 +322,14 @@ public final class LocalSpawnRunner implements SpawnRunner { boolean wasTimeout = result.getTerminationStatus().timedOut() || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTime)); - Status status = wasTimeout ? Status.TIMEOUT : Status.SUCCESS; int exitCode = - status == Status.TIMEOUT + wasTimeout ? POSIX_TIMEOUT_EXIT_CODE : result.getTerminationStatus().getRawExitCode(); + Status status = + wasTimeout + ? Status.TIMEOUT + : (exitCode == 0 ? Status.SUCCESS : Status.NON_ZERO_EXIT); return new SpawnResult.Builder() .setStatus(status) .setExitCode(exitCode) 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 71e0139aef..8dae278958 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 @@ -65,6 +65,8 @@ import javax.annotation.Nullable; /** A client for the remote execution service. */ @ThreadSafe class RemoteSpawnRunner implements SpawnRunner { + private static final int POSIX_TIMEOUT_EXIT_CODE = /*SIGNAL_BASE=*/128 + /*SIGALRM=*/14; + private final Path execRoot; private final RemoteOptions options; // TODO(olaola): This will be set on a per-action basis instead. @@ -197,9 +199,10 @@ class RemoteSpawnRunner implements SpawnRunner { private SpawnResult downloadRemoteResults(ActionResult result, FileOutErr outErr) throws ExecException, IOException, InterruptedException { remoteCache.download(result, execRoot, outErr); + int exitCode = result.getExitCode(); return new SpawnResult.Builder() - .setStatus(Status.SUCCESS) // Even if the action failed with non-zero exit code. - .setExitCode(result.getExitCode()) + .setStatus(exitCode == 0 ? Status.SUCCESS : Status.NON_ZERO_EXIT) + .setExitCode(exitCode) .build(); } @@ -219,19 +222,22 @@ class RemoteSpawnRunner implements SpawnRunner { private SpawnResult handleError(IOException cause, FileOutErr outErr) throws IOException, ExecException { - final Status status; if (cause instanceof TimeoutException) { - status = Status.TIMEOUT; // TODO(buchgr): provide stdout/stderr from the action that timed out. // Remove the unsuported message once remote execution tests no longer check for it. try (OutputStream out = outErr.getOutputStream()) { String msg = "Log output for timeouts is not yet supported in remote execution.\n"; out.write(msg.getBytes(StandardCharsets.UTF_8)); } - return new SpawnResult.Builder().setStatus(status).build(); - } else if (cause instanceof RetryException + return new SpawnResult.Builder() + .setStatus(Status.TIMEOUT) + .setExitCode(POSIX_TIMEOUT_EXIT_CODE) + .build(); + } + final Status status; + if (cause instanceof RetryException && ((RetryException) cause).causedByStatusCode(Code.UNAVAILABLE)) { - status = Status.CONNECTION_FAILED; + status = Status.EXECUTION_FAILED_CATASTROPHICALLY; } else if (cause instanceof CacheNotFoundException) { status = Status.REMOTE_CACHE_FAILED; } else { @@ -240,8 +246,8 @@ class RemoteSpawnRunner implements SpawnRunner { throw new SpawnExecException( Throwables.getStackTraceAsString(cause), new SpawnResult.Builder() - .setExitCode(ExitCode.REMOTE_ERROR.getNumericExitCode()) .setStatus(status) + .setExitCode(ExitCode.REMOTE_ERROR.getNumericExitCode()) .build(), /* forciblyRunRemotely= */ false, /* catastrophe= */ true); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java index e82f1878d7..5612c4423e 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/AbstractSandboxSpawnRunner.java @@ -167,11 +167,14 @@ abstract class AbstractSandboxSpawnRunner implements SpawnRunner { Duration wallTime = Duration.ofMillis(System.currentTimeMillis() - startTime); boolean wasTimeout = wasTimeout(timeout, wallTime); - Status status = wasTimeout ? Status.TIMEOUT : Status.SUCCESS; int exitCode = - status == Status.TIMEOUT + wasTimeout ? POSIX_TIMEOUT_EXIT_CODE : result.getTerminationStatus().getRawExitCode(); + Status status = + wasTimeout + ? Status.TIMEOUT + : (exitCode == 0) ? Status.SUCCESS : Status.NON_ZERO_EXIT; return new SpawnResult.Builder() .setStatus(status) .setExitCode(exitCode) diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index f7853c936d..31e68be8f8 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java @@ -159,9 +159,10 @@ final class WorkerSpawnRunner implements SpawnRunner { FileOutErr outErr = policy.getFileOutErr(); response.getOutputBytes().writeTo(outErr.getErrorStream()); + int exitCode = response.getExitCode(); return new SpawnResult.Builder() - .setExitCode(response.getExitCode()) - .setStatus(SpawnResult.Status.SUCCESS) + .setExitCode(exitCode) + .setStatus(exitCode == 0 ? SpawnResult.Status.SUCCESS : SpawnResult.Status.NON_ZERO_EXIT) .setWallTime(wallTime) .build(); } |