Make workers restart on flags that affect their creation/behaviour.
Also refactors the related code to being cleaner.
RELNOTES: None.
PiperOrigin-RevId: 374365649
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 1080013..1008d0f 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/worker/BUILD
@@ -26,7 +26,6 @@
"//src/main/java/com/google/devtools/build/lib/exec:spawn_runner",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry",
"//src/main/java/com/google/devtools/build/lib/exec/local",
- "//src/main/java/com/google/devtools/build/lib/exec/local:options",
"//src/main/java/com/google/devtools/build/lib/runtime/commands/events",
"//src/main/java/com/google/devtools/build/lib/sandbox",
"//src/main/java/com/google/devtools/build/lib/shell",
diff --git a/src/main/java/com/google/devtools/build/lib/worker/SimpleWorkerPool.java b/src/main/java/com/google/devtools/build/lib/worker/SimpleWorkerPool.java
index 02249e1..4c67418 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/SimpleWorkerPool.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/SimpleWorkerPool.java
@@ -15,6 +15,7 @@
import com.google.common.base.Throwables;
import java.io.IOException;
+import java.util.Objects;
import javax.annotation.concurrent.ThreadSafe;
import org.apache.commons.pool2.impl.GenericKeyedObjectPool;
import org.apache.commons.pool2.impl.GenericKeyedObjectPoolConfig;
@@ -28,8 +29,38 @@
@ThreadSafe
final class SimpleWorkerPool extends GenericKeyedObjectPool<WorkerKey, Worker> {
- public SimpleWorkerPool(WorkerFactory factory, GenericKeyedObjectPoolConfig<Worker> config) {
- super(factory, config);
+ public SimpleWorkerPool(WorkerFactory factory, int max) {
+ super(factory, makeConfig(max));
+ }
+
+ static SimpleWorkerPoolConfig makeConfig(int max) {
+ SimpleWorkerPoolConfig config = new SimpleWorkerPoolConfig();
+
+ // It's better to re-use a worker as often as possible and keep it hot, in order to profit
+ // from JIT optimizations as much as possible.
+ config.setLifo(true);
+
+ // Keep a fixed number of workers running per key.
+ config.setMaxIdlePerKey(max);
+ config.setMaxTotalPerKey(max);
+ config.setMinIdlePerKey(max);
+
+ // Don't limit the total number of worker processes, as otherwise the pool might be full of
+ // workers for one WorkerKey and can't accommodate a worker for another WorkerKey.
+ config.setMaxTotal(-1);
+
+ // Wait for a worker to become ready when a thread needs one.
+ config.setBlockWhenExhausted(true);
+
+ // Always test the liveliness of worker processes.
+ config.setTestOnBorrow(true);
+ config.setTestOnCreate(true);
+ config.setTestOnReturn(true);
+
+ // No eviction of idle workers.
+ config.setTimeBetweenEvictionRunsMillis(-1);
+
+ return config;
}
@Override
@@ -51,4 +82,66 @@
throw new RuntimeException("unexpected", t);
}
}
+
+ /**
+ * Our own configuration class for the {@code SimpleWorkerPool} that correctly implements {@code
+ * equals()} and {@code hashCode()}.
+ */
+ static final class SimpleWorkerPoolConfig extends GenericKeyedObjectPoolConfig<Worker> {
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ SimpleWorkerPoolConfig that = (SimpleWorkerPoolConfig) o;
+ return getBlockWhenExhausted() == that.getBlockWhenExhausted()
+ && getFairness() == that.getFairness()
+ && getJmxEnabled() == that.getJmxEnabled()
+ && getLifo() == that.getLifo()
+ && getMaxWaitMillis() == that.getMaxWaitMillis()
+ && getMinEvictableIdleTimeMillis() == that.getMinEvictableIdleTimeMillis()
+ && getNumTestsPerEvictionRun() == that.getNumTestsPerEvictionRun()
+ && getSoftMinEvictableIdleTimeMillis() == that.getSoftMinEvictableIdleTimeMillis()
+ && getTestOnBorrow() == that.getTestOnBorrow()
+ && getTestOnCreate() == that.getTestOnCreate()
+ && getTestOnReturn() == that.getTestOnReturn()
+ && getTestWhileIdle() == that.getTestWhileIdle()
+ && getTimeBetweenEvictionRunsMillis() == that.getTimeBetweenEvictionRunsMillis()
+ && getMaxIdlePerKey() == that.getMaxIdlePerKey()
+ && getMaxTotal() == that.getMaxTotal()
+ && getMaxTotalPerKey() == that.getMaxTotalPerKey()
+ && getMinIdlePerKey() == that.getMinIdlePerKey()
+ && Objects.equals(getEvictionPolicyClassName(), that.getEvictionPolicyClassName())
+ && Objects.equals(getJmxNameBase(), that.getJmxNameBase())
+ && Objects.equals(getJmxNamePrefix(), that.getJmxNamePrefix());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ getBlockWhenExhausted(),
+ getFairness(),
+ getJmxEnabled(),
+ getLifo(),
+ getMaxWaitMillis(),
+ getMinEvictableIdleTimeMillis(),
+ getNumTestsPerEvictionRun(),
+ getSoftMinEvictableIdleTimeMillis(),
+ getTestOnBorrow(),
+ getTestOnCreate(),
+ getTestOnReturn(),
+ getTestWhileIdle(),
+ getTimeBetweenEvictionRunsMillis(),
+ getMaxIdlePerKey(),
+ getMaxTotal(),
+ getMaxTotalPerKey(),
+ getMinIdlePerKey(),
+ getEvictionPolicyClassName(),
+ getJmxNameBase(),
+ getJmxNamePrefix());
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java
index 1b3d25a..b6a189e 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java
@@ -18,6 +18,7 @@
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Objects;
import java.util.Optional;
import java.util.TreeSet;
import java.util.concurrent.atomic.AtomicInteger;
@@ -35,23 +36,19 @@
// request_id (which is indistinguishable from 0 in proto3).
private static final AtomicInteger pidCounter = new AtomicInteger(1);
- private WorkerOptions workerOptions;
private final Path workerBaseDir;
private Reporter reporter;
+ private final boolean workerSandboxing;
- public WorkerFactory(WorkerOptions workerOptions, Path workerBaseDir) {
- this.workerOptions = workerOptions;
+ public WorkerFactory(Path workerBaseDir, boolean workerSandboxing) {
this.workerBaseDir = workerBaseDir;
+ this.workerSandboxing = workerSandboxing;
}
public void setReporter(Reporter reporter) {
this.reporter = reporter;
}
- public void setOptions(WorkerOptions workerOptions) {
- this.workerOptions = workerOptions;
- }
-
@Override
public Worker create(WorkerKey key) {
int workerId = pidCounter.getAndIncrement();
@@ -60,7 +57,7 @@
workerBaseDir.getRelative(workTypeName + "-" + workerId + "-" + key.getMnemonic() + ".log");
Worker worker;
- boolean sandboxed = workerOptions.workerSandboxing || key.isSpeculative();
+ boolean sandboxed = workerSandboxing || key.isSpeculative();
if (sandboxed) {
Path workDir = getSandboxedWorkerPath(key, workerId);
worker = new SandboxedWorker(key, workerId, workDir, logFile);
@@ -70,7 +67,7 @@
} else {
worker = new SingleplexWorker(key, workerId, key.getExecRoot(), logFile);
}
- if (workerOptions.workerVerbose) {
+ if (reporter != null) {
reporter.handle(
Event.info(
String.format(
@@ -91,9 +88,7 @@
.getRelative(workspaceName);
}
- /**
- * Use the DefaultPooledObject implementation.
- */
+ /** Use the DefaultPooledObject implementation. */
@Override
public PooledObject<Worker> wrap(Worker worker) {
return new DefaultPooledObject<>(worker);
@@ -102,7 +97,7 @@
/** When a worker process is discarded, destroy its process, too. */
@Override
public void destroyObject(WorkerKey key, PooledObject<Worker> p) {
- if (workerOptions.workerVerbose) {
+ if (reporter != null) {
int workerId = p.getObject().getWorkerId();
reporter.handle(
Event.info(
@@ -122,7 +117,7 @@
Worker worker = p.getObject();
Optional<Integer> exitValue = worker.getExitValue();
if (exitValue.isPresent()) {
- if (workerOptions.workerVerbose && worker.diedUnexpectedly()) {
+ if (reporter != null && worker.diedUnexpectedly()) {
String msg =
String.format(
"%s %s (id %d) has unexpectedly died with exit code %d.",
@@ -140,7 +135,7 @@
boolean filesChanged =
!key.getWorkerFilesCombinedHash().equals(worker.getWorkerFilesCombinedHash());
- if (workerOptions.workerVerbose && reporter != null && filesChanged) {
+ if (reporter != null && filesChanged) {
StringBuilder msg = new StringBuilder();
msg.append(
String.format(
@@ -167,4 +162,21 @@
return !filesChanged;
}
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof WorkerFactory)) {
+ return false;
+ }
+ WorkerFactory that = (WorkerFactory) o;
+ return workerSandboxing == that.workerSandboxing && workerBaseDir.equals(that.workerBaseDir);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(workerBaseDir, workerSandboxing);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
index a34c9bd..4ce5465 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java
@@ -15,9 +15,9 @@
import static com.google.common.base.Preconditions.checkNotNull;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.common.eventbus.Subscribe;
import com.google.devtools.build.lib.buildtool.buildevent.BuildCompleteEvent;
import com.google.devtools.build.lib.buildtool.buildevent.BuildStartingEvent;
@@ -33,23 +33,16 @@
import com.google.devtools.build.lib.sandbox.SandboxHelpers;
import com.google.devtools.build.lib.sandbox.SandboxOptions;
import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.worker.WorkerOptions.MultiResourceConverter;
+import com.google.devtools.build.lib.worker.WorkerPool.WorkerPoolConfig;
import com.google.devtools.common.options.OptionsBase;
import java.io.IOException;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import javax.annotation.Nonnull;
/** A module that adds the WorkerActionContextProvider to the available action context providers. */
public class WorkerModule extends BlazeModule {
private CommandEnvironment env;
private WorkerFactory workerFactory;
- private WorkerPool workerPool;
- private WorkerOptions options;
- private ImmutableMap<String, Integer> workerPoolConfig;
- private ImmutableMap<String, Integer> multiplexPoolConfig;
+ @VisibleForTesting WorkerPool workerPool;
@Override
public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) {
@@ -68,21 +61,31 @@
@Subscribe
public void cleanStarting(CleanStartingEvent event) {
if (workerPool != null) {
- this.options = event.getOptionsProvider().getOptions(WorkerOptions.class);
- workerFactory.setReporter(env.getReporter());
- workerFactory.setOptions(options);
+ WorkerOptions options = event.getOptionsProvider().getOptions(WorkerOptions.class);
+ workerFactory.setReporter(options.workerVerbose ? env.getReporter() : null);
shutdownPool(
- "Clean command is running, shutting down worker pool...", /* alwaysLog= */ false);
+ "Clean command is running, shutting down worker pool...",
+ /* alwaysLog= */ false,
+ options.workerVerbose);
}
}
+ /**
+ * Handles updating worker factories and pools when a build starts. If either the workerDir or the
+ * sandboxing flag has changed, we need to recreate the factory, and we clear out logs at the same
+ * time. If options affecting the pools have changed, we just change the pools.
+ */
@Subscribe
public void buildStarting(BuildStartingEvent event) {
- options = event.getRequest().getOptions(WorkerOptions.class);
+ WorkerOptions options = event.getRequest().getOptions(WorkerOptions.class);
+ if (workerFactory != null) {
+ workerFactory.setReporter(options.workerVerbose ? env.getReporter() : null);
+ }
+ Path workerDir =
+ env.getOutputBase().getRelative(env.getRuntime().getProductName() + "-workers");
- if (workerFactory == null) {
- Path workerDir =
- env.getOutputBase().getRelative(env.getRuntime().getProductName() + "-workers");
+ WorkerFactory newWorkerFactory = new WorkerFactory(workerDir, options.workerSandboxing);
+ if (!newWorkerFactory.equals(workerFactory)) {
try {
if (!workerDir.createDirectory()) {
// Clean out old log files.
@@ -102,59 +105,40 @@
.handle(Event.error("Could not create base directory for workers: " + workerDir));
}
- workerFactory = new WorkerFactory(options, workerDir);
+ shutdownPool(
+ "Worker factory configuration has changed, restarting worker pool...",
+ /* alwaysLog= */ true,
+ options.workerVerbose);
+ workerFactory = newWorkerFactory;
+ workerFactory.setReporter(options.workerVerbose ? env.getReporter() : null);
}
- workerFactory.setReporter(env.getReporter());
- workerFactory.setOptions(options);
-
- ImmutableMap<String, Integer> newConfig = createConfigFromOptions(options.workerMaxInstances);
- ImmutableMap<String, Integer> newMultiplexConfig =
- createConfigFromOptions(options.workerMaxMultiplexInstances);
+ WorkerPoolConfig newConfig =
+ new WorkerPoolConfig(
+ workerFactory,
+ options.workerMaxInstances,
+ options.workerMaxMultiplexInstances,
+ options.highPriorityWorkers);
// If the config changed compared to the last run, we have to create a new pool.
- if ((workerPoolConfig != null && !workerPoolConfig.equals(newConfig))
- || (multiplexPoolConfig != null && !multiplexPoolConfig.equals(newMultiplexConfig))) {
+ if (workerPool == null || !newConfig.equals(workerPool.getWorkerPoolConfig())) {
shutdownPool(
- "Worker configuration has changed, restarting worker pool...", /* alwaysLog= */ true);
+ "Worker pool configuration has changed, restarting worker pool...",
+ /* alwaysLog= */ true,
+ options.workerVerbose);
}
if (workerPool == null) {
- workerPoolConfig = newConfig;
- multiplexPoolConfig = newMultiplexConfig;
- workerPool =
- new WorkerPool(
- workerFactory, workerPoolConfig, multiplexPoolConfig, options.highPriorityWorkers);
+ workerPool = new WorkerPool(newConfig);
}
}
- /**
- * Creates a configuration for a worker pool from the options given. If the same mnemonic occurs
- * more than once in the options, the last value passed wins.
- */
- @Nonnull
- private static ImmutableMap<String, Integer> createConfigFromOptions(
- List<Map.Entry<String, Integer>> options) {
- LinkedHashMap<String, Integer> newConfigBuilder = new LinkedHashMap<>();
- for (Map.Entry<String, Integer> entry : options) {
- newConfigBuilder.put(entry.getKey(), entry.getValue());
- }
-
- if (!newConfigBuilder.containsKey("")) {
- // Empty string gives the number of workers for any type of worker not explicitly specified.
- // If no value is given, use the default, 2.
- newConfigBuilder.put("", MultiResourceConverter.DEFAULT_VALUE);
- }
-
- return ImmutableMap.copyOf(newConfigBuilder);
- }
-
@Override
public void registerSpawnStrategies(
SpawnStrategyRegistry.Builder registryBuilder, CommandEnvironment env) {
checkNotNull(workerPool);
SandboxOptions sandboxOptions = env.getOptions().getOptions(SandboxOptions.class);
- options = env.getOptions().getOptions(WorkerOptions.class);
+ WorkerOptions options = env.getOptions().getOptions(WorkerOptions.class);
LocalEnvProvider localEnvProvider = LocalEnvProvider.forCurrentOs(env.getClientEnv());
WorkerSpawnRunner spawnRunner =
new WorkerSpawnRunner(
@@ -178,17 +162,21 @@
@Subscribe
public void buildComplete(BuildCompleteEvent event) {
+ WorkerOptions options = env.getOptions().getOptions(WorkerOptions.class);
if (options != null && options.workerQuitAfterBuild) {
- shutdownPool("Build completed, shutting down worker pool...", /* alwaysLog= */ false);
+ shutdownPool(
+ "Build completed, shutting down worker pool...",
+ /* alwaysLog= */ false,
+ options.workerVerbose);
}
}
/** Shuts down the worker pool and sets {#code workerPool} to null. */
- private void shutdownPool(String reason, boolean alwaysLog) {
+ private void shutdownPool(String reason, boolean alwaysLog, boolean workerVerbose) {
Preconditions.checkArgument(!reason.isEmpty());
if (workerPool != null) {
- if ((options != null && options.workerVerbose) || alwaysLog) {
+ if (workerVerbose || alwaysLog) {
env.getReporter().handle(Event.info(reason));
}
workerPool.close();
@@ -199,7 +187,6 @@
@Override
public void afterCommand() {
this.env = null;
- this.options = null;
if (this.workerFactory != null) {
this.workerFactory.setReporter(null);
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerPool.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerPool.java
index ba630a5..7bb8f64 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerPool.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerPool.java
@@ -16,9 +16,15 @@
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.worker.WorkerOptions.MultiResourceConverter;
import java.io.IOException;
+import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger;
+import javax.annotation.Nonnull;
import javax.annotation.concurrent.ThreadSafe;
import org.apache.commons.pool2.impl.GenericKeyedObjectPool;
@@ -45,70 +51,66 @@
private final AtomicInteger highPriorityWorkersInUse = new AtomicInteger(0);
/** Which mnemonics create high-priority workers. */
private final ImmutableSet<String> highPriorityWorkerMnemonics;
+
+ private final WorkerPoolConfig workerPoolConfig;
/** Map of singleplex worker pools, one per mnemonic. */
private final ImmutableMap<String, SimpleWorkerPool> workerPools;
/** Map of multiplex worker pools, one per mnemonic. */
private final ImmutableMap<String, SimpleWorkerPool> multiplexPools;
+ public WorkerPool(WorkerPoolConfig workerPoolConfig) {
+ this.workerPoolConfig = workerPoolConfig;
+
+ highPriorityWorkerMnemonics =
+ ImmutableSet.copyOf((Iterable<String>) workerPoolConfig.getHighPriorityWorkers());
+
+ Map<String, Integer> config = createConfigFromOptions(workerPoolConfig.getWorkerMaxInstances());
+ Map<String, Integer> multiplexConfig =
+ createConfigFromOptions(workerPoolConfig.getWorkerMaxMultiplexInstances());
+
+ workerPools =
+ createWorkerPools(workerPoolConfig.getWorkerFactory(), config, DEFAULT_MAX_WORKERS);
+ multiplexPools =
+ createWorkerPools(
+ workerPoolConfig.getWorkerFactory(), multiplexConfig, DEFAULT_MAX_MULTIPLEX_WORKERS);
+ }
+
+ public WorkerPoolConfig getWorkerPoolConfig() {
+ return workerPoolConfig;
+ }
+
/**
- * @param factory worker factory
- * @param config pool configuration; max number of workers per WorkerKey for each mnemonic; the
- * empty string key specifies the default maximum
- * @param multiplexConfig like {@code config}, but for multiplex workers
- * @param highPriorityWorkers mnemonics of high priority workers
+ * Creates a configuration for a worker pool from the options given. If the same mnemonic occurs
+ * more than once in the options, the last value passed wins.
*/
- public WorkerPool(
- WorkerFactory factory,
- Map<String, Integer> config,
- Map<String, Integer> multiplexConfig,
- Iterable<String> highPriorityWorkers) {
- highPriorityWorkerMnemonics = ImmutableSet.copyOf(highPriorityWorkers);
- workerPools = createWorkerPools(factory, config, DEFAULT_MAX_WORKERS);
- multiplexPools = createWorkerPools(factory, multiplexConfig, DEFAULT_MAX_MULTIPLEX_WORKERS);
+ @Nonnull
+ private static ImmutableMap<String, Integer> createConfigFromOptions(
+ List<Entry<String, Integer>> options) {
+ LinkedHashMap<String, Integer> newConfigBuilder = new LinkedHashMap<>();
+ for (Map.Entry<String, Integer> entry : options) {
+ newConfigBuilder.put(entry.getKey(), entry.getValue());
+ }
+
+ if (!newConfigBuilder.containsKey("")) {
+ // Empty string gives the number of workers for any type of worker not explicitly specified.
+ // If no value is given, use the default, 2.
+ newConfigBuilder.put("", MultiResourceConverter.DEFAULT_VALUE);
+ }
+
+ return ImmutableMap.copyOf(newConfigBuilder);
}
private static ImmutableMap<String, SimpleWorkerPool> createWorkerPools(
WorkerFactory factory, Map<String, Integer> config, int defaultMaxWorkers) {
ImmutableMap.Builder<String, SimpleWorkerPool> workerPoolsBuilder = ImmutableMap.builder();
config.forEach(
- (key, value) ->
- workerPoolsBuilder.put(key, new SimpleWorkerPool(factory, makeConfig(value))));
+ (key, value) -> workerPoolsBuilder.put(key, new SimpleWorkerPool(factory, value)));
if (!config.containsKey("")) {
- workerPoolsBuilder.put("", new SimpleWorkerPool(factory, makeConfig(defaultMaxWorkers)));
+ workerPoolsBuilder.put("", new SimpleWorkerPool(factory, defaultMaxWorkers));
}
return workerPoolsBuilder.build();
}
- private static WorkerPoolConfig makeConfig(int max) {
- WorkerPoolConfig config = new WorkerPoolConfig();
-
- // It's better to re-use a worker as often as possible and keep it hot, in order to profit
- // from JIT optimizations as much as possible.
- config.setLifo(true);
-
- // Keep a fixed number of workers running per key.
- config.setMaxIdlePerKey(max);
- config.setMaxTotalPerKey(max);
- config.setMinIdlePerKey(max);
-
- // Don't limit the total number of worker processes, as otherwise the pool might be full of
- // workers for one WorkerKey and can't accommodate a worker for another WorkerKey.
- config.setMaxTotal(-1);
-
- // Wait for a worker to become ready when a thread needs one.
- config.setBlockWhenExhausted(true);
-
- // Always test the liveliness of worker processes.
- config.setTestOnBorrow(true);
- config.setTestOnCreate(true);
- config.setTestOnReturn(true);
-
- // No eviction of idle workers.
- config.setTimeBetweenEvictionRunsMillis(-1);
-
- return config;
- }
-
private SimpleWorkerPool getPool(WorkerKey key) {
if (key.isMultiplex()) {
return multiplexPools.getOrDefault(key.getMnemonic(), multiplexPools.get(""));
@@ -197,4 +199,59 @@
workerPools.values().forEach(GenericKeyedObjectPool::close);
multiplexPools.values().forEach(GenericKeyedObjectPool::close);
}
+
+ static class WorkerPoolConfig {
+ private final WorkerFactory workerFactory;
+ private final List<Entry<String, Integer>> workerMaxInstances;
+ private final List<Entry<String, Integer>> workerMaxMultiplexInstances;
+ private final List<String> highPriorityWorkers;
+
+ WorkerPoolConfig(
+ WorkerFactory workerFactory,
+ List<Entry<String, Integer>> workerMaxInstances,
+ List<Entry<String, Integer>> workerMaxMultiplexInstances,
+ List<String> highPriorityWorkers) {
+ this.workerFactory = workerFactory;
+ this.workerMaxInstances = workerMaxInstances;
+ this.workerMaxMultiplexInstances = workerMaxMultiplexInstances;
+ this.highPriorityWorkers = highPriorityWorkers;
+ }
+
+ public WorkerFactory getWorkerFactory() {
+ return workerFactory;
+ }
+
+ public List<Entry<String, Integer>> getWorkerMaxInstances() {
+ return workerMaxInstances;
+ }
+
+ public List<Entry<String, Integer>> getWorkerMaxMultiplexInstances() {
+ return workerMaxMultiplexInstances;
+ }
+
+ public List<String> getHighPriorityWorkers() {
+ return highPriorityWorkers;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (!(o instanceof WorkerPoolConfig)) {
+ return false;
+ }
+ WorkerPoolConfig that = (WorkerPoolConfig) o;
+ return workerFactory.equals(that.workerFactory)
+ && workerMaxInstances.equals(that.workerMaxInstances)
+ && workerMaxMultiplexInstances.equals(that.workerMaxMultiplexInstances)
+ && highPriorityWorkers.equals(that.highPriorityWorkers);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(
+ workerFactory, workerMaxInstances, workerMaxMultiplexInstances, highPriorityWorkers);
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolConfig.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolConfig.java
deleted file mode 100644
index 20d40df..0000000
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolConfig.java
+++ /dev/null
@@ -1,79 +0,0 @@
-// 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 java.util.Objects;
-import org.apache.commons.pool2.impl.GenericKeyedObjectPoolConfig;
-
-/**
- * Our own configuration class for the {@code WorkerPool} that correctly implements {@code equals()}
- * and {@code hashCode()}.
- */
-final class WorkerPoolConfig extends GenericKeyedObjectPoolConfig<Worker> {
- @Override
- public boolean equals(Object o) {
- if (this == o) {
- return true;
- }
- if (o == null || getClass() != o.getClass()) {
- return false;
- }
- WorkerPoolConfig that = (WorkerPoolConfig) o;
- return getBlockWhenExhausted() == that.getBlockWhenExhausted()
- && getFairness() == that.getFairness()
- && getJmxEnabled() == that.getJmxEnabled()
- && getLifo() == that.getLifo()
- && getMaxWaitMillis() == that.getMaxWaitMillis()
- && getMinEvictableIdleTimeMillis() == that.getMinEvictableIdleTimeMillis()
- && getNumTestsPerEvictionRun() == that.getNumTestsPerEvictionRun()
- && getSoftMinEvictableIdleTimeMillis() == that.getSoftMinEvictableIdleTimeMillis()
- && getTestOnBorrow() == that.getTestOnBorrow()
- && getTestOnCreate() == that.getTestOnCreate()
- && getTestOnReturn() == that.getTestOnReturn()
- && getTestWhileIdle() == that.getTestWhileIdle()
- && getTimeBetweenEvictionRunsMillis() == that.getTimeBetweenEvictionRunsMillis()
- && getMaxIdlePerKey() == that.getMaxIdlePerKey()
- && getMaxTotal() == that.getMaxTotal()
- && getMaxTotalPerKey() == that.getMaxTotalPerKey()
- && getMinIdlePerKey() == that.getMinIdlePerKey()
- && Objects.equals(getEvictionPolicyClassName(), that.getEvictionPolicyClassName())
- && Objects.equals(getJmxNameBase(), that.getJmxNameBase())
- && Objects.equals(getJmxNamePrefix(), that.getJmxNamePrefix());
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(
- getBlockWhenExhausted(),
- getFairness(),
- getJmxEnabled(),
- getLifo(),
- getMaxWaitMillis(),
- getMinEvictableIdleTimeMillis(),
- getNumTestsPerEvictionRun(),
- getSoftMinEvictableIdleTimeMillis(),
- getTestOnBorrow(),
- getTestOnCreate(),
- getTestOnReturn(),
- getTestWhileIdle(),
- getTimeBetweenEvictionRunsMillis(),
- getMaxIdlePerKey(),
- getMaxTotal(),
- getMaxTotalPerKey(),
- getMinIdlePerKey(),
- getEvictionPolicyClassName(),
- getJmxNameBase(),
- getJmxNamePrefix());
- }
-}
diff --git a/src/test/java/com/google/devtools/build/lib/worker/BUILD b/src/test/java/com/google/devtools/build/lib/worker/BUILD
index 88afc7f..83935fc 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/worker/BUILD
@@ -61,14 +61,18 @@
srcs = glob(["*Test.java"]),
deps = [
":testutil",
+ "//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
+ "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
+ "//src/main/java/com/google/devtools/build/lib/analysis:server_directories",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_runner",
"//src/main/java/com/google/devtools/build/lib/exec/local",
"//src/main/java/com/google/devtools/build/lib/sandbox",
+ "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:resource_converter",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
diff --git a/src/test/java/com/google/devtools/build/lib/worker/ExampleWorker.java b/src/test/java/com/google/devtools/build/lib/worker/ExampleWorker.java
index 96e2e86..50439bd 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/ExampleWorker.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/ExampleWorker.java
@@ -86,6 +86,7 @@
if (request == null) {
break;
}
+
currentRequest = request;
inputs.clear();
for (Input input : request.getInputsList()) {
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolConfigTest.java b/src/test/java/com/google/devtools/build/lib/worker/SimpleWorkerPoolConfigTest.java
similarity index 85%
rename from src/test/java/com/google/devtools/build/lib/worker/WorkerPoolConfigTest.java
rename to src/test/java/com/google/devtools/build/lib/worker/SimpleWorkerPoolConfigTest.java
index 27b7aac..a314a9c 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolConfigTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/SimpleWorkerPoolConfigTest.java
@@ -14,20 +14,18 @@
package com.google.devtools.build.lib.worker;
import com.google.common.testing.EqualsTester;
-
+import com.google.devtools.build.lib.worker.SimpleWorkerPool.SimpleWorkerPoolConfig;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/**
- * Test WorkerPoolConfig.
- */
+/** Test SimpleWorkerPoolConfig. */
@RunWith(JUnit4.class)
-public class WorkerPoolConfigTest {
+public class SimpleWorkerPoolConfigTest {
@Test
public void testEquals() throws Exception {
- WorkerPoolConfig config1a = new WorkerPoolConfig();
+ SimpleWorkerPoolConfig config1a = new SimpleWorkerPoolConfig();
config1a.setLifo(true);
config1a.setMaxIdlePerKey(4);
config1a.setMaxTotalPerKey(4);
@@ -39,7 +37,7 @@
config1a.setTestOnReturn(true);
config1a.setTimeBetweenEvictionRunsMillis(-1);
- WorkerPoolConfig config1b = new WorkerPoolConfig();
+ SimpleWorkerPoolConfig config1b = new SimpleWorkerPoolConfig();
config1b.setLifo(true);
config1b.setMaxIdlePerKey(4);
config1b.setMaxTotalPerKey(4);
@@ -51,7 +49,7 @@
config1b.setTestOnReturn(true);
config1b.setTimeBetweenEvictionRunsMillis(-1);
- WorkerPoolConfig config2a = new WorkerPoolConfig();
+ SimpleWorkerPoolConfig config2a = new SimpleWorkerPoolConfig();
config2a.setLifo(true);
config2a.setMaxIdlePerKey(1);
config2a.setMaxTotalPerKey(1);
@@ -63,7 +61,7 @@
config2a.setTestOnReturn(true);
config2a.setTimeBetweenEvictionRunsMillis(-1);
- WorkerPoolConfig config2b = new WorkerPoolConfig();
+ SimpleWorkerPoolConfig config2b = new SimpleWorkerPoolConfig();
config2b.setLifo(true);
config2b.setMaxIdlePerKey(1);
config2b.setMaxTotalPerKey(1);
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java
index fa37349..81671d0 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java
@@ -41,7 +41,8 @@
@Test
public void sandboxedWorkerPathEndsWithWorkspaceName() throws Exception {
Path workerBaseDir = fs.getPath("/outputbase/bazel-workers");
- WorkerFactory workerFactory = new WorkerFactory(new WorkerOptions(), workerBaseDir);
+ final WorkerOptions workerOptions = new WorkerOptions();
+ WorkerFactory workerFactory = new WorkerFactory(workerBaseDir, workerOptions.workerSandboxing);
WorkerKey workerKey = createWorkerKey(/* mustBeSandboxed */ true, /* proxied */ false);
Path sandboxedWorkerPath = workerFactory.getSandboxedWorkerPath(workerKey, 1);
@@ -66,7 +67,8 @@
@Test
public void workerCreationTypeCheck() throws Exception {
Path workerBaseDir = fs.getPath("/outputbase/bazel-workers");
- WorkerFactory workerFactory = new WorkerFactory(new WorkerOptions(), workerBaseDir);
+ final WorkerOptions workerOptions = new WorkerOptions();
+ WorkerFactory workerFactory = new WorkerFactory(workerBaseDir, workerOptions.workerSandboxing);
WorkerKey sandboxedWorkerKey = createWorkerKey(/* mustBeSandboxed */ true, /* proxied */ false);
Worker sandboxedWorker = workerFactory.create(sandboxedWorkerKey);
assertThat(sandboxedWorker.getClass()).isEqualTo(SandboxedWorker.class);
@@ -88,7 +90,8 @@
@Test
public void testMultiplexWorkersShareLogfiles() throws Exception {
Path workerBaseDir = fs.getPath("/outputbase/bazel-workers");
- WorkerFactory workerFactory = new WorkerFactory(new WorkerOptions(), workerBaseDir);
+ final WorkerOptions workerOptions = new WorkerOptions();
+ WorkerFactory workerFactory = new WorkerFactory(workerBaseDir, workerOptions.workerSandboxing);
WorkerKey workerKey1 = createWorkerKey(/* mustBeSandboxed */ false, /* proxied */ true, "arg1");
Worker proxiedWorker1a = workerFactory.create(workerKey1);
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java
new file mode 100644
index 0000000..a33d1fb
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java
@@ -0,0 +1,269 @@
+// 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.worker;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.lib.actions.ExecutionRequirements.WorkerProtocolFormat.JSON;
+import static org.mockito.Mockito.when;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+import com.google.common.eventbus.EventBus;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.analysis.ServerDirectories;
+import com.google.devtools.build.lib.buildtool.BuildRequest;
+import com.google.devtools.build.lib.buildtool.buildevent.BuildStartingEvent;
+import com.google.devtools.build.lib.clock.BlazeClock;
+import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.events.StoredEventHandler;
+import com.google.devtools.build.lib.runtime.BlazeRuntime;
+import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.util.AbruptExitException;
+import com.google.devtools.build.lib.vfs.DigestHashFunction;
+import com.google.devtools.build.lib.vfs.FileSystem;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.devtools.common.options.OptionsParsingResult;
+import java.io.IOException;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+
+/** Tests for WorkerModule. I bet you didn't see that coming, eh? */
+@RunWith(JUnit4.class)
+public class WorkerModuleTest {
+ @Rule public final MockitoRule mockito = MockitoJUnit.rule();
+ @Mock CommandEnvironment env;
+ @Mock BuildRequest request;
+ @Mock OptionsParsingResult startupOptionsProvider;
+
+ private FileSystem fs;
+ private StoredEventHandler storedEventHandler;
+
+ @Before
+ public void setUp() {
+ fs = new InMemoryFileSystem(BlazeClock.instance(), DigestHashFunction.SHA256);
+ }
+
+ @Test
+ public void buildStarting_createsPools()
+ throws AbruptExitException, IOException, InterruptedException {
+ WorkerModule module = new WorkerModule();
+ WorkerOptions options = WorkerOptions.DEFAULTS;
+ when(request.getOptions(WorkerOptions.class)).thenReturn(options);
+ setupEnvironment("/outputRoot");
+
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).isEmpty();
+ assertThat(fs.getPath("/outputRoot/outputBase/bazel-workers").exists()).isTrue();
+ assertThat(module.workerPool).isNotNull();
+ WorkerKey workerKey = TestUtils.createWorkerKey(JSON, fs);
+ Worker worker = module.workerPool.borrowObject(workerKey);
+ assertThat(worker.workerKey).isEqualTo(workerKey);
+ }
+
+ @Test
+ public void buildStarting_restartsOnSandboxChanges() throws IOException, AbruptExitException {
+ WorkerModule module = new WorkerModule();
+ WorkerOptions options = WorkerOptions.DEFAULTS;
+ when(request.getOptions(WorkerOptions.class)).thenReturn(options);
+ setupEnvironment("/outputRoot");
+
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).isEmpty();
+
+ Path workerDir = fs.getPath("/outputRoot/outputBase/bazel-workers");
+ Path aLog = workerDir.getRelative("f.log");
+ aLog.createSymbolicLink(PathFragment.EMPTY_FRAGMENT);
+ WorkerPool oldPool = module.workerPool;
+ options.workerSandboxing = !options.workerSandboxing;
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).hasSize(1);
+ assertThat(storedEventHandler.getEvents().get(0).getMessage())
+ .contains("Worker factory configuration has changed");
+ assertThat(module.workerPool).isNotSameInstanceAs(oldPool);
+ assertThat(aLog.exists()).isFalse();
+ }
+
+ @Test
+ public void buildStarting_workersDestroyedOnRestart()
+ throws IOException, AbruptExitException, InterruptedException {
+ WorkerModule module = new WorkerModule();
+ WorkerOptions options = WorkerOptions.DEFAULTS;
+ options.workerVerbose = true;
+ when(request.getOptions(WorkerOptions.class)).thenReturn(options);
+ setupEnvironment("/outputRoot");
+
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ WorkerKey workerKey = TestUtils.createWorkerKey(JSON, fs);
+ Worker worker = module.workerPool.borrowObject(workerKey);
+ assertThat(worker.workerKey).isEqualTo(workerKey);
+ assertThat(storedEventHandler.getEvents()).hasSize(1);
+ assertThat(storedEventHandler.getEvents().get(0).getMessage())
+ .contains("Created new sandboxed dummy worker");
+ storedEventHandler.clear();
+
+ Path workerDir = fs.getPath("/outputRoot/outputBase/bazel-workers");
+ Path aLog = workerDir.getRelative("f.log");
+ aLog.createSymbolicLink(PathFragment.EMPTY_FRAGMENT);
+ WorkerPool oldPool = module.workerPool;
+ options.workerSandboxing = !options.workerSandboxing;
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).hasSize(1);
+ assertThat(storedEventHandler.getEvents().get(0).getMessage())
+ .contains("Worker factory configuration has changed");
+ assertThat(module.workerPool).isNotSameInstanceAs(oldPool);
+ assertThat(aLog.exists()).isFalse();
+ }
+
+ @Test
+ public void buildStarting_restartsOnOutputbaseChanges() throws IOException, AbruptExitException {
+ WorkerModule module = new WorkerModule();
+ WorkerOptions options = WorkerOptions.DEFAULTS;
+ when(request.getOptions(WorkerOptions.class)).thenReturn(options);
+ setupEnvironment("/outputRoot");
+
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).isEmpty();
+
+ // Log file from old root, doesn't get cleaned
+ Path workerDir = fs.getPath("/outputRoot/outputBase/bazel-workers");
+ Path oldLog = workerDir.getRelative("f.log");
+ oldLog.createSymbolicLink(PathFragment.EMPTY_FRAGMENT);
+
+ WorkerPool oldPool = module.workerPool;
+ setupEnvironment("/otherRootDir");
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).hasSize(1);
+ assertThat(storedEventHandler.getEvents().get(0).getMessage())
+ .contains("Worker factory configuration has changed");
+ assertThat(module.workerPool).isNotSameInstanceAs(oldPool);
+ assertThat(fs.getPath("/otherRootDir/outputBase/bazel-workers").exists()).isTrue();
+ assertThat(oldLog.exists()).isTrue();
+ }
+
+ @Test
+ public void buildStarting_restartsOnHiPrioChanges() throws IOException, AbruptExitException {
+ WorkerModule module = new WorkerModule();
+ WorkerOptions options = WorkerOptions.DEFAULTS;
+ when(request.getOptions(WorkerOptions.class)).thenReturn(options);
+ setupEnvironment("/outputRoot");
+
+ module.beforeCommand(env);
+ // Check that new pools/factories are made with default options
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).isEmpty();
+
+ // Logs are only cleared on factory reset, not on pool reset, so this file should survive
+ Path workerDir = fs.getPath("/outputRoot/outputBase/bazel-workers");
+ Path oldLog = workerDir.getRelative("f.log");
+ oldLog.createSymbolicLink(PathFragment.EMPTY_FRAGMENT);
+
+ WorkerPool oldPool = module.workerPool;
+ options.highPriorityWorkers = ImmutableList.of("foo");
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).hasSize(1);
+ assertThat(storedEventHandler.getEvents().get(0).getMessage())
+ .contains("Worker pool configuration has changed");
+ assertThat(module.workerPool).isNotSameInstanceAs(oldPool);
+ assertThat(oldLog.exists()).isTrue();
+ }
+
+ @Test
+ public void buildStarting_restartsOnNumMultiplexWorkersChanges()
+ throws IOException, AbruptExitException {
+ WorkerModule module = new WorkerModule();
+ WorkerOptions options = WorkerOptions.DEFAULTS;
+ when(request.getOptions(WorkerOptions.class)).thenReturn(options);
+ setupEnvironment("/outputRoot");
+
+ module.beforeCommand(env);
+ // Check that new pools/factories are made with default options
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).isEmpty();
+
+ WorkerPool oldPool = module.workerPool;
+ options.workerMaxMultiplexInstances = Lists.newArrayList(Maps.immutableEntry("foo", 42));
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).hasSize(1);
+ assertThat(storedEventHandler.getEvents().get(0).getMessage())
+ .contains("Worker pool configuration has changed");
+ assertThat(module.workerPool).isNotSameInstanceAs(oldPool);
+ }
+
+ @Test
+ public void buildStarting_restartsOnNumWorkersChanges() throws IOException, AbruptExitException {
+ WorkerModule module = new WorkerModule();
+ WorkerOptions options = WorkerOptions.DEFAULTS;
+
+ when(request.getOptions(WorkerOptions.class)).thenReturn(options);
+ setupEnvironment("/outputRoot");
+
+ module.beforeCommand(env);
+ // Check that new pools/factories are made with default options
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).isEmpty();
+
+ WorkerPool oldPool = module.workerPool;
+ options.workerMaxInstances = Lists.newArrayList(Maps.immutableEntry("bar", 3));
+ module.beforeCommand(env);
+ module.buildStarting(new BuildStartingEvent(env, request));
+ assertThat(storedEventHandler.getEvents()).hasSize(1);
+ assertThat(storedEventHandler.getEvents().get(0).getMessage())
+ .contains("Worker pool configuration has changed");
+ assertThat(module.workerPool).isNotSameInstanceAs(oldPool);
+ }
+
+ private void setupEnvironment(String rootDir) throws IOException, AbruptExitException {
+ storedEventHandler = new StoredEventHandler();
+ Path root = fs.getPath(rootDir);
+ Path outputBase = root.getRelative("outputBase");
+ outputBase.createDirectoryAndParents();
+ when(env.getOutputBase()).thenReturn(outputBase);
+ Path workspace = fs.getPath("/workspace");
+ when(env.getWorkingDirectory()).thenReturn(workspace);
+ ServerDirectories serverDirectories =
+ new ServerDirectories(
+ root.getRelative("userroot/install"), outputBase, root.getRelative("userroot"));
+ BlazeRuntime blazeRuntime =
+ new BlazeRuntime.Builder()
+ .setProductName("bazel")
+ .setServerDirectories(serverDirectories)
+ .setStartupOptionsProvider(startupOptionsProvider)
+ .build();
+ when(env.getRuntime()).thenReturn(blazeRuntime);
+ when(env.getDirectories())
+ .thenReturn(new BlazeDirectories(serverDirectories, null, null, "blaze"));
+ EventBus eventBus = new EventBus();
+ when(env.getEventBus()).thenReturn(eventBus);
+ when(env.getReporter()).thenReturn(new Reporter(eventBus, storedEventHandler));
+ }
+}
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java
index 036ca7b..20cb532 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolTest.java
@@ -23,15 +23,17 @@
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.devtools.build.lib.worker.WorkerPool.WorkerPoolConfig;
import java.io.IOException;
import java.lang.Thread.State;
+import java.util.Map.Entry;
import org.apache.commons.pool2.impl.DefaultPooledObject;
import org.junit.Before;
import org.junit.Rule;
@@ -60,14 +62,13 @@
public void setUp() throws Exception {
fileSystem = new InMemoryFileSystem(BlazeClock.instance(), DigestHashFunction.SHA256);
doAnswer(
- arg -> {
- return new DefaultPooledObject<>(
- new TestWorker(
- arg.getArgument(0),
- workerIds++,
- fileSystem.getPath("/workDir"),
- fileSystem.getPath("/logDir")));
- })
+ arg ->
+ new DefaultPooledObject<>(
+ new TestWorker(
+ arg.getArgument(0),
+ workerIds++,
+ fileSystem.getPath("/workDir"),
+ fileSystem.getPath("/logDir"))))
.when(factoryMock)
.makeObject(any());
when(factoryMock.validateObject(any(), any())).thenReturn(true);
@@ -77,10 +78,8 @@
public void testBorrow_createsWhenNeeded() throws Exception {
WorkerPool workerPool =
new WorkerPool(
- factoryMock,
- ImmutableMap.of("mnem", 2, "", 1),
- ImmutableMap.of(),
- Lists.newArrayList());
+ new WorkerPoolConfig(
+ factoryMock, entryList("mnem", 2, "", 1), entryList(), Lists.newArrayList()));
WorkerKey workerKey = createWorkerKey(fileSystem, "mnem", false);
Worker worker1 = workerPool.borrowObject(workerKey);
Worker worker2 = workerPool.borrowObject(workerKey);
@@ -93,10 +92,8 @@
public void testBorrow_reusesWhenPossible() throws Exception {
WorkerPool workerPool =
new WorkerPool(
- factoryMock,
- ImmutableMap.of("mnem", 2, "", 1),
- ImmutableMap.of(),
- Lists.newArrayList());
+ new WorkerPoolConfig(
+ factoryMock, entryList("mnem", 2, "", 1), entryList(), Lists.newArrayList()));
WorkerKey workerKey = createWorkerKey(fileSystem, "mnem", false);
Worker worker1 = workerPool.borrowObject(workerKey);
workerPool.returnObject(workerKey, worker1);
@@ -109,10 +106,8 @@
public void testBorrow_usesDefault() throws Exception {
WorkerPool workerPool =
new WorkerPool(
- factoryMock,
- ImmutableMap.of("mnem", 2, "", 1),
- ImmutableMap.of(),
- Lists.newArrayList());
+ new WorkerPoolConfig(
+ factoryMock, entryList("mnem", 2, "", 1), entryList(), Lists.newArrayList()));
WorkerKey workerKey1 = createWorkerKey(fileSystem, "mnem", false);
Worker worker1 = workerPool.borrowObject(workerKey1);
Worker worker1a = workerPool.borrowObject(workerKey1);
@@ -129,10 +124,8 @@
public void testBorrow_pooledByKey() throws Exception {
WorkerPool workerPool =
new WorkerPool(
- factoryMock,
- ImmutableMap.of("mnem", 2, "", 1),
- ImmutableMap.of(),
- Lists.newArrayList());
+ new WorkerPoolConfig(
+ factoryMock, entryList("mnem", 2, "", 1), entryList(), Lists.newArrayList()));
WorkerKey workerKey1 = createWorkerKey(fileSystem, "mnem", false);
Worker worker1 = workerPool.borrowObject(workerKey1);
Worker worker1a = workerPool.borrowObject(workerKey1);
@@ -149,10 +142,11 @@
public void testBorrow_separateMultiplexWorkers() throws Exception {
WorkerPool workerPool =
new WorkerPool(
- factoryMock,
- ImmutableMap.of("mnem", 1, "", 1),
- ImmutableMap.of("mnem", 2, "", 1),
- Lists.newArrayList());
+ new WorkerPoolConfig(
+ factoryMock,
+ entryList("mnem", 1, "", 1),
+ entryList("mnem", 2, "", 1),
+ Lists.newArrayList()));
WorkerKey workerKey = createWorkerKey(fileSystem, "mnem", false);
Worker worker1 = workerPool.borrowObject(workerKey);
assertThat(worker1.getWorkerId()).isEqualTo(1);
@@ -175,10 +169,11 @@
public void testBorrow_allowsOneHiPrio() throws Exception {
WorkerPool workerPool =
new WorkerPool(
- factoryMock,
- ImmutableMap.of("loprio", 2, "hiprio", 2, "", 1),
- ImmutableMap.of(),
- ImmutableList.of("hiprio"));
+ new WorkerPoolConfig(
+ factoryMock,
+ entryList("loprio", 2, "hiprio", 2, "", 1),
+ entryList(),
+ ImmutableList.of("hiprio")));
WorkerKey workerKey1 = createWorkerKey(fileSystem, "hiprio", false);
Worker worker1 = workerPool.borrowObject(workerKey1);
assertThat(worker1.getWorkerId()).isEqualTo(1);
@@ -194,10 +189,11 @@
public void testBorrow_twoHiPrioBlocks() throws Exception {
WorkerPool workerPool =
new WorkerPool(
- factoryMock,
- ImmutableMap.of("loprio", 2, "hiprio", 2, "", 1),
- ImmutableMap.of(),
- ImmutableList.of("hiprio"));
+ new WorkerPoolConfig(
+ factoryMock,
+ entryList("loprio", 2, "hiprio", 2, "", 1),
+ entryList(),
+ ImmutableList.of("hiprio")));
WorkerKey workerKey1 = createWorkerKey(fileSystem, "hiprio", false);
Worker worker1 = workerPool.borrowObject(workerKey1);
Worker worker1a = workerPool.borrowObject(workerKey1);
@@ -238,4 +234,21 @@
verify(factoryMock, times(2)).makeObject(workerKey1);
verify(factoryMock, times(1)).makeObject(workerKey2);
}
+
+ private static ImmutableList<Entry<String, Integer>> entryList() {
+ return ImmutableList.of();
+ }
+
+ private static ImmutableList<Entry<String, Integer>> entryList(
+ String key1, int value1, String key2, int value2) {
+ return ImmutableList.of(Maps.immutableEntry(key1, value1), Maps.immutableEntry(key2, value2));
+ }
+
+ private static ImmutableList<Entry<String, Integer>> entryList(
+ String key1, int value1, String key2, int value2, String key3, int value3) {
+ return ImmutableList.of(
+ Maps.immutableEntry(key1, value1),
+ Maps.immutableEntry(key2, value2),
+ Maps.immutableEntry(key3, value3));
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java
index fcdfea1..dbe71a4 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerSpawnRunnerTest.java
@@ -47,6 +47,7 @@
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.devtools.build.lib.worker.WorkerPool.WorkerPoolConfig;
import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
import java.io.IOException;
@@ -85,20 +86,21 @@
private WorkerPool createWorkerPool() {
return new WorkerPool(
- new WorkerFactory(options, fs.getPath("/workerBase")) {
- @Override
- public Worker create(WorkerKey key) {
- return worker;
- }
+ new WorkerPoolConfig(
+ new WorkerFactory(fs.getPath("/workerBase"), options.workerSandboxing) {
+ @Override
+ public Worker create(WorkerKey key) {
+ return worker;
+ }
- @Override
- public boolean validateObject(WorkerKey key, PooledObject<Worker> p) {
- return true;
- }
- },
- ImmutableMap.of(),
- ImmutableMap.of(),
- ImmutableList.of());
+ @Override
+ public boolean validateObject(WorkerKey key, PooledObject<Worker> p) {
+ return true;
+ }
+ },
+ ImmutableList.of(),
+ ImmutableList.of(),
+ ImmutableList.of()));
}
@Test
