Cleanup: decouple the WorkerFactory from WorkerPoolConfig.
PiperOrigin-RevId: 626031767
Change-Id: I89996e782ab7daf85e1d2edbc5f2b5c806699767
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 86b38bf..e9004dd 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
@@ -48,7 +48,7 @@
private static final String STALE_TRASH = "_stale_trash";
private CommandEnvironment env;
- private WorkerFactory workerFactory;
+ @VisibleForTesting WorkerFactory workerFactory;
private AsynchronousTreeDeleter treeDeleter;
WorkerPoolConfig config;
@@ -160,13 +160,12 @@
WorkerPoolConfig newConfig =
new WorkerPoolConfig(
- workerFactory,
options.useNewWorkerPool,
options.workerMaxInstances,
options.workerMaxMultiplexInstances);
// If the config changed compared to the last run, we have to create a new pool.
- if (workerPool == null || !newConfig.equals(config)) {
+ if (!newConfig.equals(config)) {
shutdownPool(
"Worker pool configuration has changed, restarting worker pool...",
/* alwaysLog= */ true,
@@ -175,9 +174,9 @@
if (workerPool == null) {
if (options.useNewWorkerPool) {
- workerPool = new WorkerPoolImpl(newConfig);
+ workerPool = new WorkerPoolImpl(workerFactory, newConfig);
} else {
- workerPool = new WorkerPoolImplLegacy(newConfig);
+ workerPool = new WorkerPoolImplLegacy(workerFactory, newConfig);
}
config = newConfig;
// If workerPool is restarted then we should recreate metrics.
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
index 2160ff2..8ebb9e3 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolConfig.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolConfig.java
@@ -24,17 +24,14 @@
* workers.
*/
public class WorkerPoolConfig {
- private final WorkerFactory workerFactory;
private final boolean useNewWorkerPool;
private final List<Entry<String, Integer>> workerMaxInstances;
private final List<Entry<String, Integer>> workerMaxMultiplexInstances;
public WorkerPoolConfig(
- WorkerFactory workerFactory,
boolean useNewWorkerPool,
List<Entry<String, Integer>> workerMaxInstances,
List<Entry<String, Integer>> workerMaxMultiplexInstances) {
- this.workerFactory = workerFactory;
this.useNewWorkerPool = useNewWorkerPool;
this.workerMaxInstances = workerMaxInstances;
this.workerMaxMultiplexInstances = workerMaxMultiplexInstances;
@@ -42,20 +39,14 @@
@VisibleForTesting
public WorkerPoolConfig(
- WorkerFactory workerFactory,
List<Entry<String, Integer>> workerMaxInstances,
List<Entry<String, Integer>> workerMaxMultiplexInstances) {
this(
- workerFactory,
/* useNewWorkerPool= */ false,
workerMaxInstances,
workerMaxMultiplexInstances);
}
- public WorkerFactory getWorkerFactory() {
- return workerFactory;
- }
-
public List<Entry<String, Integer>> getWorkerMaxInstances() {
return workerMaxInstances;
}
@@ -72,15 +63,13 @@
if (!(o instanceof WorkerPoolConfig that)) {
return false;
}
- return workerFactory.equals(that.workerFactory)
- && useNewWorkerPool == that.useNewWorkerPool
+ return useNewWorkerPool == that.useNewWorkerPool
&& workerMaxInstances.equals(that.workerMaxInstances)
&& workerMaxMultiplexInstances.equals(that.workerMaxMultiplexInstances);
}
@Override
public int hashCode() {
- return Objects.hash(
- workerFactory, useNewWorkerPool, workerMaxInstances, workerMaxMultiplexInstances);
+ return Objects.hash(useNewWorkerPool, workerMaxInstances, workerMaxMultiplexInstances);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImpl.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImpl.java
index 6b90aa1..e79ef12 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImpl.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImpl.java
@@ -72,8 +72,8 @@
private final ImmutableMap<String, Integer> multiplexMaxInstances;
private final ConcurrentHashMap<WorkerKey, WorkerKeyPool> pools = new ConcurrentHashMap<>();
- public WorkerPoolImpl(WorkerPoolConfig config) {
- this.factory = config.getWorkerFactory();
+ public WorkerPoolImpl(WorkerFactory factory, WorkerPoolConfig config) {
+ this.factory = factory;
this.singleplexMaxInstances =
getMaxInstances(config.getWorkerMaxInstances(), DEFAULT_MAX_SINGLEPLEX_WORKERS);
this.multiplexMaxInstances =
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImplLegacy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImplLegacy.java
index c2b3b77..59eef40 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImplLegacy.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolImplLegacy.java
@@ -58,7 +58,7 @@
/** Map of multiplex worker pools, one per mnemonic. */
private final ImmutableMap<String, SimpleWorkerPool> multiplexPools;
- public WorkerPoolImplLegacy(WorkerPoolConfig workerPoolConfig) {
+ public WorkerPoolImplLegacy(WorkerFactory factory, WorkerPoolConfig workerPoolConfig) {
this.workerPoolConfig = workerPoolConfig;
ImmutableMap<String, Integer> config =
@@ -67,8 +67,8 @@
createConfigFromOptions(
workerPoolConfig.getWorkerMaxMultiplexInstances(), DEFAULT_MAX_MULTIPLEX_WORKERS);
- workerPools = createWorkerPools(workerPoolConfig.getWorkerFactory(), config);
- multiplexPools = createWorkerPools(workerPoolConfig.getWorkerFactory(), multiplexConfig);
+ workerPools = createWorkerPools(factory, config);
+ multiplexPools = createWorkerPools(factory, multiplexConfig);
}
public WorkerPoolConfig getWorkerPoolConfig() {
diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerLifecycleManagerTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerLifecycleManagerTest.java
index f50ec75..eeff482 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerLifecycleManagerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerLifecycleManagerTest.java
@@ -89,16 +89,16 @@
(WorkerPoolSupplier)
(factory, singleplexMaxInstances, multiplexMaxInstances) ->
new WorkerPoolImplLegacy(
- new WorkerPoolConfig(
- factory, singleplexMaxInstances, multiplexMaxInstances)),
+ factory,
+ new WorkerPoolConfig(singleplexMaxInstances, multiplexMaxInstances)),
workerFactorySupplier,
},
{
(WorkerPoolSupplier)
(factory, singleplexMaxInstances, multiplexMaxInstances) ->
new WorkerPoolImpl(
- new WorkerPoolConfig(
- factory, singleplexMaxInstances, multiplexMaxInstances)),
+ factory,
+ new WorkerPoolConfig(singleplexMaxInstances, multiplexMaxInstances)),
workerFactorySupplier,
}
});
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
index 8bfe6ac..ab09abf 100644
--- a/src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java
+++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerModuleTest.java
@@ -138,7 +138,7 @@
.contains("Worker factory configuration has changed");
assertThat(module.workerPool).isNotSameInstanceAs(oldPool);
WorkerKey workerKey = WorkerTestUtils.createWorkerKey(fs, "mnemonic", false);
- module.getWorkerPoolConfig().getWorkerFactory().create(workerKey);
+ module.workerFactory.create(workerKey);
assertThat(fs.getPath("/otherRootDir/outputBase/bazel-workers").exists()).isTrue();
assertThat(oldLog.exists()).isTrue();
}
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 18e037e..73940af 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
@@ -86,8 +86,8 @@
(WorkerPoolSupplier)
(factory) ->
new WorkerPoolImplLegacy(
+ factory,
new WorkerPoolConfig(
- factory,
/* workerMaxInstances= */ ImmutableList.of(
Maps.immutableEntry("mnem", 2), Maps.immutableEntry("", 1)),
/* workerMaxMultiplexInstances= */ ImmutableList.of(
@@ -98,8 +98,8 @@
(WorkerPoolSupplier)
(factory) ->
new WorkerPoolImpl(
+ factory,
new WorkerPoolConfig(
- factory,
/* workerMaxInstances= */ ImmutableList.of(
Maps.immutableEntry("mnem", 2)),
/* workerMaxMultiplexInstances= */ ImmutableList.of(
