diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/exec')
7 files changed, 25 insertions, 18 deletions
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 4641df0dc2..ffef030501 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 @@ -34,8 +34,8 @@ import com.google.devtools.build.lib.util.CommandFailureUtils; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; +import java.time.Duration; import java.util.SortedMap; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; @@ -69,7 +69,7 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte if (actionExecutionContext.reportsSubcommands()) { actionExecutionContext.reportSubcommand(spawn); } - final int timeoutSeconds = Spawns.getTimeoutSeconds(spawn); + final Duration timeout = Spawns.getTimeout(spawn); SpawnExecutionPolicy policy = new SpawnExecutionPolicy() { private final int id = execCount.incrementAndGet(); @@ -104,8 +104,8 @@ public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionConte } @Override - public long getTimeoutMillis() { - return TimeUnit.SECONDS.toMillis(timeoutSeconds); + public Duration getTimeout() { + return timeout; } @Override diff --git a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java index d8c6d774ee..ecea21a1da 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java +++ b/src/main/java/com/google/devtools/build/lib/exec/ExecutionOptions.java @@ -24,6 +24,7 @@ import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsBase; +import java.time.Duration; import java.util.Map; /** @@ -203,7 +204,7 @@ public class ExecutionOptions extends OptionsBase { + "moderate, long and eternal (in that order). In either form, a value of -1 tells " + "blaze to use its default timeouts for that category." ) - public Map<TestTimeout, Integer> testTimeout; + public Map<TestTimeout, Duration> testTimeout; @Option( name = "resource_autosense", diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java index c2819766cc..b98ca2ee59 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; +import java.time.Duration; import java.util.SortedMap; /** @@ -169,7 +170,7 @@ public interface SpawnRunner { void lockOutputFiles() throws InterruptedException; /** Returns the timeout that should be applied for the given {@link Spawn} instance. */ - long getTimeoutMillis(); + Duration getTimeout(); /** The files to which to write stdout and stderr. */ FileOutErr getFileOutErr(); diff --git a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java index acbf5c4963..8edc5217fb 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/StandaloneTestStrategy.java @@ -112,7 +112,7 @@ public class StandaloneTestStrategy extends TestStrategy { executionInfo.put(ExecutionRequirements.NO_CACHE, ""); } // This key is only understood by StandaloneSpawnStrategy. - executionInfo.put("timeout", "" + getTimeout(action)); + executionInfo.put("timeout", "" + getTimeout(action).getSeconds()); executionInfo.putAll(action.getTestProperties().getExecutionInfo()); ResourceSet localResourceUsage = diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java b/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java index 09bf0e04b3..b6db013173 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/TestPolicy.java @@ -17,6 +17,7 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.rules.test.TestRunnerAction; import com.google.devtools.build.lib.util.UserUtils; import com.google.devtools.build.lib.vfs.PathFragment; +import java.time.Duration; import java.util.HashMap; import java.util.Map; @@ -58,7 +59,7 @@ public class TestPolicy { public Map<String, String> computeTestEnvironment( TestRunnerAction testAction, Map<String, String> clientEnv, - int timeoutInSeconds, + Duration timeout, PathFragment relativeRunfilesDir, PathFragment tmpDir) { Map<String, String> env = new HashMap<>(); @@ -95,7 +96,7 @@ public class TestPolicy { // Setup any test-specific env variables; note that this does not overwrite existing values for // TEST_RANDOM_SEED or TEST_SIZE if they're already set. - testAction.setupEnvVariables(env, timeoutInSeconds); + testAction.setupEnvVariables(env, timeout); return env; } } diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java index d12b700576..d37b56621a 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java @@ -48,6 +48,7 @@ import java.io.Closeable; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.time.Duration; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -235,7 +236,7 @@ public abstract class TestStrategy implements TestActionContext { * the "categorical timeouts" which are based on the --test_timeout flag. A rule picks its timeout * but ends up with the same effective value as all other rules in that bucket. */ - protected final int getTimeout(TestRunnerAction testAction) { + protected final Duration getTimeout(TestRunnerAction testAction) { return executionOptions.testTimeout.get(testAction.getTestProperties().getTimeout()); } 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 2619ef5249..b2c9d3c965 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 @@ -42,6 +42,7 @@ import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; import java.io.OutputStream; +import java.time.Duration; import java.util.ArrayList; import java.util.EnumMap; import java.util.List; @@ -236,14 +237,13 @@ public final class LocalSpawnRunner implements SpawnRunner { stepLog(INFO, "running locally"); setState(State.LOCAL_ACTION_RUNNING); - int timeoutSeconds = (int) (policy.getTimeoutMillis() / 1000); Command cmd; OutputStream stdOut = ByteStreams.nullOutputStream(); OutputStream stdErr = ByteStreams.nullOutputStream(); if (useProcessWrapper) { List<String> cmdLine = new ArrayList<>(); cmdLine.add(processWrapper); - cmdLine.add("--timeout=" + timeoutSeconds); + cmdLine.add("--timeout=" + policy.getTimeout().getSeconds()); cmdLine.add("--kill_delay=" + localExecutionOptions.localSigkillGraceSeconds); cmdLine.add("--stdout=" + getPathOrDevNull(outErr.getOutputPath())); cmdLine.add("--stderr=" + getPathOrDevNull(outErr.getErrorPath())); @@ -259,7 +259,10 @@ public final class LocalSpawnRunner implements SpawnRunner { spawn.getArguments().toArray(new String[0]), localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), execRoot, productName), execRoot.getPathFile(), - policy.getTimeoutMillis()); + // TODO(ulfjack): Command throws if timeouts are unsupported and timeout >= 0. For + // consistency, we should change it to not throw (and not enforce a timeout) if + // timeout <= 0 instead. + policy.getTimeout().isZero() ? -1 : policy.getTimeout().toMillis()); } long startTime = System.currentTimeMillis(); @@ -289,9 +292,9 @@ public final class LocalSpawnRunner implements SpawnRunner { } setState(State.SUCCESS); - long wallTime = System.currentTimeMillis() - startTime; + long wallTimeMillis = System.currentTimeMillis() - startTime; boolean wasTimeout = result.getTerminationStatus().timedout() - || (useProcessWrapper && wasTimeout(timeoutSeconds, wallTime)); + || (useProcessWrapper && wasTimeout(policy.getTimeout(), wallTimeMillis)); Status status = wasTimeout ? Status.TIMEOUT : Status.SUCCESS; int exitCode = status == Status.TIMEOUT ? POSIX_TIMEOUT_EXIT_CODE @@ -300,7 +303,7 @@ public final class LocalSpawnRunner implements SpawnRunner { .setStatus(status) .setExitCode(exitCode) .setExecutorHostname(hostName) - .setWallTimeMillis(wallTime) + .setWallTimeMillis(wallTimeMillis) .build(); } @@ -308,8 +311,8 @@ public final class LocalSpawnRunner implements SpawnRunner { return path == null ? "/dev/null" : path.getPathString(); } - private boolean wasTimeout(int timeoutSeconds, long wallTimeMillis) { - return timeoutSeconds > 0 && wallTimeMillis / 1000.0 > timeoutSeconds; + private boolean wasTimeout(Duration timeout, long wallTimeMillis) { + return !timeout.isZero() && wallTimeMillis > timeout.toMillis(); } } |