From 25120df651ae7d0ac9e85cbbea54921d1f2c4ae4 Mon Sep 17 00:00:00 2001 From: Kush Chakraborty Date: Thu, 8 Dec 2016 19:53:41 +0000 Subject: Initial code for Persistent Java Test Runner. At this point this does nothing more than re-run the exact same test without having to re-start the test runner. In future iterations the aim is to be able to re-run tests with modified code, without having to re-start the test runner. To test out the WorkerTestStrategy simply use --test_strategy=experimental_worker for a test with bazel. -- PiperOrigin-RevId: 141465929 MOS_MIGRATED_REVID=141465929 --- .../lib/rules/test/StandaloneTestStrategy.java | 55 +++--- .../com/google/devtools/build/lib/worker/BUILD | 1 + .../lib/worker/WorkerActionContextProvider.java | 25 ++- .../devtools/build/lib/worker/WorkerFilesHash.java | 41 ++++ .../devtools/build/lib/worker/WorkerOptions.java | 15 ++ .../build/lib/worker/WorkerSpawnStrategy.java | 36 ++-- .../build/lib/worker/WorkerTestStrategy.java | 211 +++++++++++++++++++++ 7 files changed, 333 insertions(+), 51 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java create mode 100644 src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java (limited to 'src/main/java/com/google/devtools/build') diff --git a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java index bbef175a7d..05eeb7abac 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/rules/test/StandaloneTestStrategy.java @@ -55,7 +55,7 @@ public class StandaloneTestStrategy extends TestStrategy { public static final String COLLECT_COVERAGE = "external/bazel_tools/tools/test/collect_coverage.sh"; - private final Path workspace; + protected final Path workspace; public StandaloneTestStrategy( OptionsClassProvider requestOptions, @@ -93,25 +93,6 @@ public class StandaloneTestStrategy extends TestStrategy { TestRunnerAction.ResolvedPaths resolvedPaths = action.resolve(execRoot); Map env = getEnv(action, execRoot, runfilesDir, testTmpDir, resolvedPaths.getXmlOutputPath()); - - Map info = new HashMap<>(); - - // This key is only understood by StandaloneSpawnStrategy. - info.put("timeout", "" + getTimeout(action)); - info.putAll(action.getTestProperties().getExecutionInfo()); - - Artifact testSetup = action.getRuntimeArtifact(TEST_SETUP_BASENAME); - Spawn spawn = - new BaseSpawn( - getArgs(testSetup.getExecPathString(), COLLECT_COVERAGE, action), - env, - info, - new RunfilesSupplierImpl( - runfilesDir.asFragment(), action.getExecutionSettings().getRunfiles()), - action, - action.getTestProperties().getLocalResourceUsage(executionOptions.usingLocalTestJobs()), - ImmutableSet.of(resolvedPaths.getXmlOutputPath().relativeTo(execRoot))); - Executor executor = actionExecutionContext.getExecutor(); try { @@ -128,7 +109,12 @@ public class StandaloneTestStrategy extends TestStrategy { .getTestStderr()); ResourceHandle handle = ResourceManager.instance().acquireResources(action, resources)) { TestResultData data = - execute(actionExecutionContext.withFileOutErr(fileOutErr), spawn, action); + execute( + actionExecutionContext.withFileOutErr(fileOutErr), + env, + action, + execRoot, + runfilesDir); appendStderr(fileOutErr.getOutputPath(), fileOutErr.getErrorPath()); finalizeTest(actionExecutionContext, action, data); } @@ -173,14 +159,35 @@ public class StandaloneTestStrategy extends TestStrategy { return vars; } - private TestResultData execute( - ActionExecutionContext actionExecutionContext, Spawn spawn, TestRunnerAction action) - throws ExecException, InterruptedException { + protected TestResultData execute( + ActionExecutionContext actionExecutionContext, + Map environment, + TestRunnerAction action, + Path execRoot, + Path runfilesDir) + throws ExecException, InterruptedException, IOException { Executor executor = actionExecutionContext.getExecutor(); Closeable streamed = null; Path testLogPath = action.getTestLog().getPath(); TestResultData.Builder builder = TestResultData.newBuilder(); + Map info = new HashMap<>(); + // This key is only understood by StandaloneSpawnStrategy. + info.put("timeout", "" + getTimeout(action)); + info.putAll(action.getTestProperties().getExecutionInfo()); + + Artifact testSetup = action.getRuntimeArtifact(TEST_SETUP_BASENAME); + TestRunnerAction.ResolvedPaths resolvedPaths = action.resolve(execRoot); + Spawn spawn = + new BaseSpawn( + getArgs(testSetup.getExecPathString(), COLLECT_COVERAGE, action), + environment, + info, + new RunfilesSupplierImpl( + runfilesDir.asFragment(), action.getExecutionSettings().getRunfiles()), + action, + action.getTestProperties().getLocalResourceUsage(executionOptions.usingLocalTestJobs()), + ImmutableSet.of(resolvedPaths.getXmlOutputPath().relativeTo(execRoot))); long startTime = executor.getClock().currentTimeMillis(); SpawnActionContext spawnActionContext = executor.getSpawnActionContext(action.getMnemonic()); try { diff --git a/src/main/java/com/google/devtools/build/lib/worker/BUILD b/src/main/java/com/google/devtools/build/lib/worker/BUILD index 89b9776385..8e2c21fcb0 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/BUILD +++ b/src/main/java/com/google/devtools/build/lib/worker/BUILD @@ -18,6 +18,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/sandbox", "//src/main/java/com/google/devtools/build/lib/standalone", "//src/main/java/com/google/devtools/common/options", + "//src/main/protobuf:test_status_java_proto", "//src/main/protobuf:worker_protocol_java_proto", "//third_party:apache_commons_pool2", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java index e8cd891060..e15af06b3e 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerActionContextProvider.java @@ -14,10 +14,12 @@ package com.google.devtools.build.lib.worker; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMultimap; import com.google.devtools.build.lib.actions.ActionContextProvider; import com.google.devtools.build.lib.actions.Executor.ActionContext; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.exec.ExecutionOptions; +import com.google.devtools.build.lib.rules.test.TestActionContext; import com.google.devtools.build.lib.runtime.CommandEnvironment; /** @@ -28,14 +30,21 @@ final class WorkerActionContextProvider extends ActionContextProvider { public WorkerActionContextProvider( CommandEnvironment env, BuildRequest buildRequest, WorkerPool workers) { - this.strategies = - ImmutableList.of( - new WorkerSpawnStrategy( - env.getDirectories(), - workers, - buildRequest.getOptions(ExecutionOptions.class).verboseFailures, - buildRequest.getOptions(WorkerOptions.class).workerMaxRetries, - buildRequest.getOptions(WorkerOptions.class).workerVerbose)); + int maxRetries = buildRequest.getOptions(WorkerOptions.class).workerMaxRetries; + ImmutableMultimap.Builder extraFlags = ImmutableMultimap.builder(); + extraFlags.putAll(buildRequest.getOptions(WorkerOptions.class).workerExtraFlags); + + WorkerSpawnStrategy workerSpawnStrategy = + new WorkerSpawnStrategy( + env.getDirectories(), + workers, + buildRequest.getOptions(ExecutionOptions.class).verboseFailures, + maxRetries, + buildRequest.getOptions(WorkerOptions.class).workerVerbose, + extraFlags.build()); + TestActionContext workerTestStrategy = + new WorkerTestStrategy(env, buildRequest, workers, maxRetries, extraFlags.build()); + this.strategies = ImmutableList.of(workerSpawnStrategy, workerTestStrategy); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java new file mode 100644 index 0000000000..08ab7f5e88 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerFilesHash.java @@ -0,0 +1,41 @@ +// Copyright 2016 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.worker; + +import com.google.common.hash.HashCode; +import com.google.common.hash.Hasher; +import com.google.common.hash.Hashing; +import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionInput; +import java.io.IOException; +import java.nio.charset.Charset; + +/** + * Calculates the hash based on the files, which should be unchanged on disk for a worker to get + * reused. + */ +public class WorkerFilesHash { + + public static HashCode getWorkerFilesHash( + Iterable toolFiles, ActionExecutionContext actionExecutionContext) + throws IOException { + Hasher hasher = Hashing.sha256().newHasher(); + for (ActionInput tool : toolFiles) { + hasher.putString(tool.getExecPathString(), Charset.defaultCharset()); + hasher.putBytes(actionExecutionContext.getActionInputFileCache().getDigest(tool)); + } + return hasher.hash(); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerOptions.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerOptions.java index d8fc26c776..ab3ffb88a1 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerOptions.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerOptions.java @@ -13,9 +13,12 @@ // limitations under the License. package com.google.devtools.build.lib.worker; +import com.google.devtools.common.options.Converters; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsBase; +import java.util.List; +import java.util.Map.Entry; /** * Options related to worker processes. @@ -72,6 +75,18 @@ public class WorkerOptions extends OptionsBase { ) public boolean workerVerbose; + @Option( + name = "worker_extra_flag", + converter = Converters.AssignmentConverter.class, + defaultValue = "", + category = "strategy", + help = + "Extra command-flags that will be passed to worker processes in addition to " + + "--persistent_worker, keyed by mnemonic (e.g. --worker_extra_flag=Javac=--debug.", + allowMultiple = true + ) + public List> workerExtraFlags; + @Option( name = "worker_sandboxing", defaultValue = "false", diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java index 594258afe5..08d24b2652 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java @@ -16,13 +16,13 @@ package com.google.devtools.build.lib.worker; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.common.base.Charsets; +import com.google.common.base.MoreObjects; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.common.collect.Multimap; import com.google.common.hash.HashCode; -import com.google.common.hash.Hasher; -import com.google.common.hash.Hashing; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputFileCache; @@ -173,6 +173,7 @@ public final class WorkerSpawnStrategy implements SandboxedSpawnActionContext { private final Path execRoot; private final boolean verboseFailures; private final int maxRetries; + private final Multimap extraFlags; private final boolean workerVerbose; public WorkerSpawnStrategy( @@ -180,12 +181,15 @@ public final class WorkerSpawnStrategy implements SandboxedSpawnActionContext { WorkerPool workers, boolean verboseFailures, int maxRetries, - boolean workerVerbose) { + boolean workerVerbose, + Multimap extraFlags) { + Preconditions.checkNotNull(workers); this.workers = Preconditions.checkNotNull(workers); this.execRoot = blazeDirs.getExecRoot(); this.verboseFailures = verboseFailures; this.maxRetries = maxRetries; this.workerVerbose = workerVerbose; + this.extraFlags = extraFlags; } @Override @@ -243,16 +247,21 @@ public final class WorkerSpawnStrategy implements SandboxedSpawnActionContext { FileOutErr outErr = actionExecutionContext.getFileOutErr(); - ImmutableList args = ImmutableList.builder() - .addAll(spawn.getArguments().subList(0, spawn.getArguments().size() - 1)) - .add("--persistent_worker") - .build(); + ImmutableList args = + ImmutableList.builder() + .addAll(spawn.getArguments().subList(0, spawn.getArguments().size() - 1)) + .add("--persistent_worker") + .addAll( + MoreObjects.firstNonNull( + extraFlags.get(spawn.getMnemonic()), ImmutableList.of())) + .build(); ImmutableMap env = spawn.getEnvironment(); try { ActionInputFileCache inputFileCache = actionExecutionContext.getActionInputFileCache(); - HashCode workerFilesHash = combineActionInputHashes(spawn.getToolFiles(), inputFileCache); + HashCode workerFilesHash = WorkerFilesHash.getWorkerFilesHash( + spawn.getToolFiles(), actionExecutionContext); Map inputFiles = new SpawnHelpers(execRoot).getMounts(spawn, actionExecutionContext); Set outputFiles = SandboxHelpers.getOutputFiles(spawn); @@ -329,17 +338,6 @@ public final class WorkerSpawnStrategy implements SandboxedSpawnActionContext { } } - private HashCode combineActionInputHashes( - Iterable toolFiles, ActionInputFileCache actionInputFileCache) - throws IOException { - Hasher hasher = Hashing.sha256().newHasher(); - for (ActionInput tool : toolFiles) { - hasher.putString(tool.getExecPathString(), Charset.defaultCharset()); - hasher.putBytes(actionInputFileCache.getDigest(tool)); - } - return hasher.hash(); - } - private WorkResponse execInWorker( EventHandler eventHandler, WorkerKey key, diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java new file mode 100644 index 0000000000..c090c2e23c --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerTestStrategy.java @@ -0,0 +1,211 @@ +// Copyright 2016 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.worker; + +import com.google.common.base.MoreObjects; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Multimap; +import com.google.common.hash.HashCode; +import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.ExecutionStrategy; +import com.google.devtools.build.lib.actions.Executor; +import com.google.devtools.build.lib.actions.TestExecException; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.rules.test.StandaloneTestStrategy; +import com.google.devtools.build.lib.rules.test.TestActionContext; +import com.google.devtools.build.lib.rules.test.TestRunnerAction; +import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.view.test.TestStatus.BlazeTestStatus; +import com.google.devtools.build.lib.view.test.TestStatus.TestCase; +import com.google.devtools.build.lib.view.test.TestStatus.TestResultData; +import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest; +import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse; +import com.google.devtools.common.options.OptionsClassProvider; +import java.io.IOException; +import java.util.List; +import java.util.Map; + +/** + * Runs TestRunnerAction actions in a worker. This is still experimental WIP. + * Do not use this strategy to run tests.
+ * + * TODO(kush): List to things to cosider:
+ * 1. Figure out if/how to honor the actions's execution info: + * action.getTestProperties().getExecutionInfo()
+ * 2. Figure out how to stream intermediate output when running in a Worker or block streamed + * outputs for this strategy.
+ * 3. Figure out how to add timeout facility.
+ */ +@ExecutionStrategy(contextType = TestActionContext.class, name = { "experimental_worker" }) +public class WorkerTestStrategy extends StandaloneTestStrategy { + private final WorkerPool workerPool; + private final int maxRetries; + private final Multimap extraFlags; + + public WorkerTestStrategy( + CommandEnvironment env, + OptionsClassProvider requestOptions, + WorkerPool workerPool, + int maxRetries, + Multimap extraFlags) { + super( + requestOptions, + env.getBlazeWorkspace().getBinTools(), + env.getClientEnv(), + env.getWorkspace()); + this.workerPool = workerPool; + this.maxRetries = maxRetries; + this.extraFlags = extraFlags; + } + + @Override + protected TestResultData execute( + ActionExecutionContext actionExecutionContext, + Map environment, + TestRunnerAction action, + Path execRoot, + Path runfilesDir) + throws ExecException, InterruptedException, IOException { + List startupArgs = getStartUpArgs(action); + + return execInWorker( + actionExecutionContext, + environment, + action, + startupArgs, + execRoot, + maxRetries); + } + + private TestResultData execInWorker( + ActionExecutionContext actionExecutionContext, + Map environment, + TestRunnerAction action, + List startupArgs, + Path execRoot, + int retriesLeft) + throws ExecException, InterruptedException, IOException { + Executor executor = actionExecutionContext.getExecutor(); + TestResultData.Builder builder = TestResultData.newBuilder(); + + Path testLogPath = action.getTestLog().getPath(); + Worker worker = null; + WorkerKey key = null; + long startTime = executor.getClock().currentTimeMillis(); + try { + HashCode workerFilesHash = WorkerFilesHash.getWorkerFilesHash( + action.getTools(), actionExecutionContext); + key = + new WorkerKey( + startupArgs, + environment, + execRoot, + action.getMnemonic(), + workerFilesHash, + ImmutableMap.of(), + ImmutableSet.of(), + /*mustBeSandboxed=*/false); + worker = workerPool.borrowObject(key); + + WorkRequest request = WorkRequest.getDefaultInstance(); + request.writeDelimitedTo(worker.getOutputStream()); + worker.getOutputStream().flush(); + + WorkResponse response = WorkResponse.parseDelimitedFrom(worker.getInputStream()); + actionExecutionContext.getFileOutErr().getErrorStream().write( + response.getOutputBytes().toByteArray()); + + long duration = executor.getClock().currentTimeMillis() - startTime; + builder.addTestTimes(duration); + builder.setRunDurationMillis(duration); + if (response.getExitCode() == 0) { + builder + .setTestPassed(true) + .setStatus(BlazeTestStatus.PASSED) + .setCachable(true) + .setPassedLog(testLogPath.getPathString()); + } else { + builder + .setTestPassed(false) + .setStatus(BlazeTestStatus.FAILED) + .addFailedLogs(testLogPath.getPathString()); + } + TestCase details = parseTestResult( + action.resolve(actionExecutionContext.getExecutor().getExecRoot()).getXmlOutputPath()); + if (details != null) { + builder.setTestCase(details); + } + + return builder.build(); + } catch (IOException | InterruptedException e) { + if (e instanceof InterruptedException) { + // The user pressed Ctrl-C. Get out here quick. + retriesLeft = 0; + } + + if (worker != null) { + workerPool.invalidateObject(key, worker); + worker = null; + } + if (retriesLeft > 0) { + // The worker process failed, but we still have some retries left. Let's retry with a fresh + // worker. + executor + .getEventHandler() + .handle( + Event.warn( + key.getMnemonic() + + " worker failed (" + + e + + "), invalidating and retrying with new worker...")); + return execInWorker( + actionExecutionContext, + environment, + action, + startupArgs, + execRoot, + retriesLeft - 1); + } else { + throw new TestExecException(e.getMessage()); + } + } finally { + if (worker != null) { + workerPool.returnObject(key, worker); + } + } + } + + private List getStartUpArgs(TestRunnerAction action) throws ExecException { + Artifact testSetup = action.getRuntimeArtifact(TEST_SETUP_BASENAME); + List args = getArgs(testSetup.getExecPathString(), "", action); + ImmutableList.Builder startupArgs = ImmutableList.builder(); + // Add test setup with no echo to prevent stdout corruption. + startupArgs.add(args.get(0)).add("--no_echo"); + // Add remaining of the original args. + startupArgs.addAll(args.subList(1, args.size())); + // Make the Test runner run persistently. + startupArgs.add("--persistent_test_runner"); + // Add additional flags requested for this invocation. + startupArgs.addAll(MoreObjects.firstNonNull( + extraFlags.get(action.getMnemonic()), ImmutableList.of())); + return startupArgs.build(); + } +} -- cgit v1.2.3