diff options
4 files changed, 255 insertions, 45 deletions
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 3c444a2577..8f997536e9 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 @@ -79,6 +79,36 @@ final class WorkerFactory extends BaseKeyedPooledObjectFactory<WorkerKey, Worker @Override public boolean validateObject(WorkerKey key, PooledObject<Worker> p) { Worker worker = p.getObject(); - return key.getWorkerFilesHash().equals(worker.getWorkerFilesHash()) && worker.isAlive(); + + boolean hashMatches = key.getWorkerFilesHash().equals(worker.getWorkerFilesHash()); + boolean workerIsAlive = worker.isAlive(); + boolean workerIsStillValid = hashMatches && workerIsAlive; + + if (reporter != null && !workerIsStillValid) { + StringBuilder msg = new StringBuilder(); + msg.append(key.getMnemonic()); + msg.append(" worker (id "); + msg.append(p.getObject().getWorkerId()); + msg.append(") can no longer be used, because"); + + if (!workerIsAlive) { + msg.append(" its process terminated itself or got killed"); + } + + if (!hashMatches) { + if (!workerIsAlive) { + msg.append(" and"); + } + msg.append(" its files have changed on disk ["); + msg.append(worker.getWorkerFilesHash()); + msg.append(" -> "); + msg.append(key.getWorkerFilesHash()); + msg.append("]"); + } + + reporter.handle(Event.warn(msg.toString())); + } + + return workerIsStillValid; } } 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 ef429461a3..63bade2086 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 @@ -29,21 +29,20 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.OptionsBase; -import org.apache.commons.pool2.impl.GenericKeyedObjectPoolConfig; - import java.io.IOException; /** * A module that adds the WorkerActionContextProvider to the available action context providers. */ public class WorkerModule extends BlazeModule { - private WorkerFactory workerFactory; - private WorkerPool workerPool; - private CommandEnvironment env; private BuildRequest buildRequest; private boolean verbose; + private WorkerFactory workerFactory; + private WorkerPool workerPool; + private WorkerPoolConfig workerPoolConfig; + @Override public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) { return command.builds() @@ -55,6 +54,13 @@ public class WorkerModule extends BlazeModule { public void beforeCommand(Command command, CommandEnvironment env) { this.env = env; env.getEventBus().register(this); + } + + @Subscribe + public void buildStarting(BuildStartingEvent event) { + buildRequest = event.getRequest(); + WorkerOptions options = buildRequest.getOptions(WorkerOptions.class); + verbose = options.workerVerbose; if (workerFactory == null) { Path logDir = env.getOutputBase().getRelative("worker-logs"); @@ -79,44 +85,50 @@ public class WorkerModule extends BlazeModule { } workerFactory.setReporter(env.getReporter()); + workerFactory.setVerbose(options.workerVerbose); + + WorkerPoolConfig newConfig = createWorkerPoolConfig(options); + + // If the config changed compared to the last run, we have to create a new pool. + if (workerPoolConfig != null && !workerPoolConfig.equals(newConfig)) { + shutdownPool("Worker configuration has changed, restarting worker pool..."); + } if (workerPool == null) { - GenericKeyedObjectPoolConfig config = new GenericKeyedObjectPoolConfig(); + workerPoolConfig = newConfig; + workerPool = new WorkerPool(workerFactory, 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); + private WorkerPoolConfig createWorkerPoolConfig(WorkerOptions options) { + WorkerPoolConfig config = new WorkerPoolConfig(); - // Check for & deal with idle workers every 5 seconds. - config.setTimeBetweenEvictionRunsMillis(5 * 1000); + // 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); - // Always test the liveliness of worker processes. - config.setTestOnBorrow(true); - config.setTestOnCreate(true); - config.setTestOnReturn(true); - config.setTestWhileIdle(true); + // Keep a fixed number of workers running per key. + config.setMaxIdlePerKey(options.workerMaxInstances); + config.setMaxTotalPerKey(options.workerMaxInstances); + config.setMinIdlePerKey(options.workerMaxInstances); - // Don't limit the total number of worker processes, as otherwise the pool might be full of - // e.g. Java workers and could never accommodate another request for a different kind of - // worker. - config.setMaxTotal(-1); + // Don't limit the total number of worker processes, as otherwise the pool might be full of + // e.g. Java workers and could never accommodate another request for a different kind of + // worker. + config.setMaxTotal(-1); - workerPool = new WorkerPool(workerFactory, config); - } - } + // Wait for a worker to become ready when a thread needs one. + config.setBlockWhenExhausted(true); - @Subscribe - public void buildStarting(BuildStartingEvent event) { - Preconditions.checkNotNull(workerPool); + // Always test the liveliness of worker processes. + config.setTestOnBorrow(true); + config.setTestOnCreate(true); + config.setTestOnReturn(true); - this.buildRequest = event.getRequest(); + // No eviction of idle workers. + config.setTimeBetweenEvictionRunsMillis(-1); - WorkerOptions options = buildRequest.getOptions(WorkerOptions.class); - workerPool.setMaxTotalPerKey(options.workerMaxInstances); - workerPool.setMaxIdlePerKey(options.workerMaxInstances); - workerPool.setMinIdlePerKey(options.workerMaxInstances); - workerFactory.setVerbose(options.workerVerbose); - this.verbose = options.workerVerbose; + return config; } @Override @@ -136,16 +148,10 @@ public class WorkerModule extends BlazeModule { @Subscribe public void buildComplete(BuildCompleteEvent event) { - if (workerPool != null && buildRequest != null + if (buildRequest != null && buildRequest.getOptions(WorkerOptions.class) != null && buildRequest.getOptions(WorkerOptions.class).workerQuitAfterBuild) { - if (verbose) { - env - .getReporter() - .handle(Event.info("Build completed, shutting down worker pool...")); - } - workerPool.close(); - workerPool = null; + shutdownPool("Build completed, shutting down worker pool..."); } } @@ -154,11 +160,18 @@ public class WorkerModule extends BlazeModule { // for them to finish. @Subscribe public void buildInterrupted(BuildInterruptedEvent event) { + shutdownPool("Build interrupted, shutting down worker pool..."); + } + + /** + * Shuts down the worker pool and sets {#code workerPool} to null. + */ + private void shutdownPool(String reason) { + Preconditions.checkArgument(!reason.isEmpty()); + if (workerPool != null) { if (verbose) { - env - .getReporter() - .handle(Event.info("Build interrupted, shutting down worker pool...")); + env.getReporter().handle(Event.info(reason)); } workerPool.close(); workerPool = null; @@ -170,5 +183,9 @@ public class WorkerModule extends BlazeModule { this.env = null; this.buildRequest = null; this.verbose = false; + + if (this.workerFactory != null) { + this.workerFactory.setReporter(null); + } } } 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 new file mode 100644 index 0000000000..b56e7c912b --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerPoolConfig.java @@ -0,0 +1,80 @@ +// 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 org.apache.commons.pool2.impl.GenericKeyedObjectPoolConfig; + +import java.util.Objects; + +/** + * Our own configuration class for the {@code WorkerPool} that correctly implements {@code equals()} + * and {@code hashCode()}. + */ +final class WorkerPoolConfig extends GenericKeyedObjectPoolConfig { + @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/WorkerPoolConfigTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolConfigTest.java new file mode 100644 index 0000000000..27b7aac2e0 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerPoolConfigTest.java @@ -0,0 +1,83 @@ +// 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.testing.EqualsTester; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** + * Test WorkerPoolConfig. + */ +@RunWith(JUnit4.class) +public class WorkerPoolConfigTest { + + @Test + public void testEquals() throws Exception { + WorkerPoolConfig config1a = new WorkerPoolConfig(); + config1a.setLifo(true); + config1a.setMaxIdlePerKey(4); + config1a.setMaxTotalPerKey(4); + config1a.setMinIdlePerKey(4); + config1a.setMaxTotal(-1); + config1a.setBlockWhenExhausted(true); + config1a.setTestOnBorrow(true); + config1a.setTestOnCreate(false); + config1a.setTestOnReturn(true); + config1a.setTimeBetweenEvictionRunsMillis(-1); + + WorkerPoolConfig config1b = new WorkerPoolConfig(); + config1b.setLifo(true); + config1b.setMaxIdlePerKey(4); + config1b.setMaxTotalPerKey(4); + config1b.setMinIdlePerKey(4); + config1b.setMaxTotal(-1); + config1b.setBlockWhenExhausted(true); + config1b.setTestOnBorrow(true); + config1b.setTestOnCreate(false); + config1b.setTestOnReturn(true); + config1b.setTimeBetweenEvictionRunsMillis(-1); + + WorkerPoolConfig config2a = new WorkerPoolConfig(); + config2a.setLifo(true); + config2a.setMaxIdlePerKey(1); + config2a.setMaxTotalPerKey(1); + config2a.setMinIdlePerKey(1); + config2a.setMaxTotal(-1); + config2a.setBlockWhenExhausted(true); + config2a.setTestOnBorrow(true); + config2a.setTestOnCreate(false); + config2a.setTestOnReturn(true); + config2a.setTimeBetweenEvictionRunsMillis(-1); + + WorkerPoolConfig config2b = new WorkerPoolConfig(); + config2b.setLifo(true); + config2b.setMaxIdlePerKey(1); + config2b.setMaxTotalPerKey(1); + config2b.setMinIdlePerKey(1); + config2b.setMaxTotal(-1); + config2b.setBlockWhenExhausted(true); + config2b.setTestOnBorrow(true); + config2b.setTestOnCreate(false); + config2b.setTestOnReturn(true); + config2b.setTimeBetweenEvictionRunsMillis(-1); + + new EqualsTester() + .addEqualityGroup(config1a, config1b) + .addEqualityGroup(config2a, config2b) + .testEquals(); + } +} |