diff options
7 files changed, 40 insertions, 73 deletions
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java index aba3115ee84..63e2bfd0ae1 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java @@ -2,21 +2,19 @@ package com.yahoo.vespa.config.server; import com.yahoo.cloud.config.ConfigserverConfig; +import com.yahoo.component.Version; import com.yahoo.config.model.provision.Host; import com.yahoo.config.model.provision.Hosts; import com.yahoo.config.model.provision.InMemoryProvisioner; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.RegionName; -import com.yahoo.component.Version; import com.yahoo.config.provision.Zone; import com.yahoo.container.QrSearchersConfig; import com.yahoo.container.core.VipStatusConfig; import com.yahoo.container.handler.ClustersStatus; import com.yahoo.container.handler.VipStatus; -import com.yahoo.container.jdisc.config.HealthMonitorConfig; import com.yahoo.container.jdisc.state.StateMonitor; -import com.yahoo.jdisc.core.SystemTimer; import com.yahoo.path.Path; import com.yahoo.text.Utf8; import com.yahoo.vespa.config.server.deploy.DeployTester; @@ -24,7 +22,6 @@ import com.yahoo.vespa.config.server.rpc.RpcServer; import com.yahoo.vespa.config.server.version.VersionState; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; -import com.yahoo.vespa.flags.InMemoryFlagSource; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -74,7 +71,7 @@ public class ConfigServerBootstrapTest { RpcServer rpcServer = createRpcServer(configserverConfig); // Take a host away so that there are too few for the application, to verify we can still bootstrap provisioner.allocations().values().iterator().next().remove(0); - StateMonitor stateMonitor = new StateMonitor(); + StateMonitor stateMonitor = StateMonitor.createForTesting(); VipStatus vipStatus = createVipStatus(stateMonitor); ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState, stateMonitor, @@ -105,7 +102,7 @@ public class ConfigServerBootstrapTest { assertTrue(versionState.isUpgraded()); RpcServer rpcServer = createRpcServer(configserverConfig); - StateMonitor stateMonitor = new StateMonitor(); + StateMonitor stateMonitor = StateMonitor.createForTesting(); VipStatus vipStatus = createVipStatus(stateMonitor); ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState, stateMonitor, @@ -136,7 +133,7 @@ public class ConfigServerBootstrapTest { .resolve("sessions/2/services.xml")); RpcServer rpcServer = createRpcServer(configserverConfig); - StateMonitor stateMonitor = new StateMonitor(); + StateMonitor stateMonitor = StateMonitor.createForTesting(); VipStatus vipStatus = createVipStatus(stateMonitor); ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState, stateMonitor, @@ -180,7 +177,7 @@ public class ConfigServerBootstrapTest { curator.set(Path.fromString("/config/v2/tenants/" + applicationId.tenant().value() + "/sessions/2/version"), Utf8.toBytes("1.2.2")); RpcServer rpcServer = createRpcServer(configserverConfig); - StateMonitor stateMonitor = createStateMonitor(); + StateMonitor stateMonitor = StateMonitor.createForTesting(); VipStatus vipStatus = createVipStatus(stateMonitor); ConfigServerBootstrap bootstrap = new ConfigServerBootstrap(tester.applicationRepository(), rpcServer, versionState, stateMonitor, vipStatus, @@ -205,11 +202,6 @@ public class ConfigServerBootstrapTest { return new MockRpcServer(configserverConfig.rpcport(), temporaryFolder.newFolder()); } - private StateMonitor createStateMonitor() { - return new StateMonitor(new HealthMonitorConfig(new HealthMonitorConfig.Builder().initialStatus("initializing")), - new SystemTimer()); - } - private static ConfigserverConfig createConfigserverConfig(TemporaryFolder temporaryFolder) throws IOException { return createConfigserverConfig(temporaryFolder, true); } diff --git a/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java b/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java index e1b5b769906..876d05b7be9 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java +++ b/container-core/src/main/java/com/yahoo/container/handler/VipStatus.java @@ -50,7 +50,7 @@ public class VipStatus { /** For testing */ public VipStatus(QrSearchersConfig dispatchers, ClustersStatus clustersStatus) { - this(dispatchers, new VipStatusConfig.Builder().build(), clustersStatus, new StateMonitor()); + this(dispatchers, new VipStatusConfig.Builder().build(), clustersStatus, StateMonitor.createForTesting()); } @Inject diff --git a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java index 066118294a0..ccd0864b3ab 100644 --- a/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java +++ b/container-core/src/main/java/com/yahoo/container/jdisc/state/StateMonitor.java @@ -9,6 +9,7 @@ import com.yahoo.jdisc.application.MetricConsumer; import com.yahoo.jdisc.core.SystemTimer; import java.util.Map; +import java.util.Optional; import java.util.TreeSet; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.Executors; @@ -32,7 +33,7 @@ public class StateMonitor extends AbstractComponent { public enum Status {up, down, initializing} private final CopyOnWriteArrayList<StateMetricConsumer> consumers = new CopyOnWriteArrayList<>(); - private final ScheduledExecutorService executor; + private final Optional<ScheduledExecutorService> executor; private final Timer timer; private final long snapshotIntervalMs; private volatile long lastSnapshotTimeMs; @@ -40,41 +41,28 @@ public class StateMonitor extends AbstractComponent { private volatile Status status; private final TreeSet<String> valueNames = new TreeSet<>(); - /** For testing */ - public StateMonitor() { - this(new HealthMonitorConfig.Builder().build(), new SystemTimer()); - } - @Inject public StateMonitor(HealthMonitorConfig config, Timer timer) { - this(config, - timer, - runnable -> { - Thread thread = new Thread(runnable, "StateMonitor"); - thread.setDaemon(true); - return thread; - }); + this(config, timer, runnable -> { + Thread thread = new Thread(runnable, "StateMonitor"); + thread.setDaemon(true); + return thread; + }); } - StateMonitor(HealthMonitorConfig config, Timer timer, ThreadFactory threadFactory) { - this((long)(config.snapshot_interval() * TimeUnit.SECONDS.toMillis(1)), - Status.valueOf(config.initialStatus()), - timer, threadFactory, true); + public static StateMonitor createForTesting() { + return new StateMonitor(new HealthMonitorConfig.Builder().build(), new SystemTimer()); } - /* Public for testing only */ - public StateMonitor(long snapshotIntervalMS, Status status, Timer timer, ThreadFactory threadFactory, - boolean start) { + /** Non-private only for unit testing this class. */ + StateMonitor(HealthMonitorConfig config, Timer timer, ThreadFactory threadFactory) { this.timer = timer; - this.snapshotIntervalMs = snapshotIntervalMS; + this.snapshotIntervalMs = (long)(config.snapshot_interval() * TimeUnit.SECONDS.toMillis(1)); this.lastSnapshotTimeMs = timer.currentTimeMillis(); - this.status = status; - this.executor = Executors.newSingleThreadScheduledExecutor(threadFactory); - - if (start) { - executor.scheduleAtFixedRate(this::updateSnapshot, snapshotIntervalMS, - snapshotIntervalMS, TimeUnit.MILLISECONDS); - } + this.status = Status.valueOf(config.initialStatus()); + this.executor = Optional.ofNullable(threadFactory).map(Executors::newSingleThreadScheduledExecutor); + this.executor.ifPresent(exec -> exec.scheduleAtFixedRate(this::updateSnapshot, snapshotIntervalMs, + snapshotIntervalMs, TimeUnit.MILLISECONDS)); } /** Returns a metric consumer for jDisc which will write metrics back to this */ @@ -136,13 +124,16 @@ public class StateMonitor extends AbstractComponent { @Override public void deconstruct() { - executor.shutdown(); - try { - executor.awaitTermination(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { } + executor.ifPresent(exec -> { + exec.shutdown(); + try { + exec.awaitTermination(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + } - if (!executor.isTerminated()) { - log.warning("StateMonitor failed to terminate within 5 seconds of interrupt signal. Ignoring."); - } + if (!exec.isTerminated()) { + log.warning("StateMonitor failed to terminate within 5 seconds of interrupt signal. Ignoring."); + } + }); } } diff --git a/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java b/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java index d26d7f7f013..3f33efa1993 100644 --- a/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java +++ b/container-core/src/test/java/com/yahoo/container/handler/VipStatusTestCase.java @@ -4,7 +4,6 @@ package com.yahoo.container.handler; import com.yahoo.container.QrSearchersConfig; import com.yahoo.container.core.VipStatusConfig; import com.yahoo.container.jdisc.state.StateMonitor; -import com.yahoo.jdisc.core.SystemTimer; import org.junit.Test; import static org.junit.Assert.assertEquals; @@ -140,11 +139,9 @@ public class VipStatusTestCase { } private static StateMonitor createStateMonitor(StateMonitor.Status startState) { - return new StateMonitor(1000, startState, new SystemTimer(), runnable -> { - Thread thread = new Thread(runnable, "StateMonitor"); - thread.setDaemon(true); - return thread; - }, true); + StateMonitor stateMonitor = StateMonitor.createForTesting(); + stateMonitor.status(startState); + return stateMonitor; } private static void removeFromRotation(String[] clusters, VipStatus v) { diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java index 6b44115016f..57a9ba4abdb 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateHandlerTestBase.java @@ -22,14 +22,12 @@ import org.junit.Before; import java.io.InputStreamReader; import java.io.Reader; import java.nio.charset.StandardCharsets; -import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; /** * @author Simon Thoresen Hult @@ -59,8 +57,7 @@ public class StateHandlerTestBase { new HealthMonitorConfig.Builder() .snapshot_interval(TimeUnit.MILLISECONDS.toSeconds(SNAPSHOT_INTERVAL)) .initialStatus("up")); - ThreadFactory threadFactory = ignored -> mock(Thread.class); - this.monitor = new StateMonitor(healthMonitorConfig, timer, threadFactory); + this.monitor = new StateMonitor(healthMonitorConfig, timer, null); builder.guiceModules().install(new AbstractModule() { @Override diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateMonitorBenchmarkTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateMonitorBenchmarkTest.java index 3892f81b8b5..22a2fc274c7 100644 --- a/container-core/src/test/java/com/yahoo/container/jdisc/state/StateMonitorBenchmarkTest.java +++ b/container-core/src/test/java/com/yahoo/container/jdisc/state/StateMonitorBenchmarkTest.java @@ -2,12 +2,10 @@ package com.yahoo.container.jdisc.state; import com.google.inject.Provider; -import com.yahoo.container.jdisc.config.HealthMonitorConfig; import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.application.ContainerThread; import com.yahoo.jdisc.application.MetricConsumer; import com.yahoo.jdisc.application.MetricProvider; -import com.yahoo.jdisc.core.SystemTimer; import org.junit.Test; import java.util.ArrayList; @@ -32,8 +30,7 @@ public class StateMonitorBenchmarkTest { @Test public void requireThatHealthMonitorDoesNotBlockMetricThreads() throws Exception { - StateMonitor monitor = new StateMonitor(new HealthMonitorConfig(new HealthMonitorConfig.Builder()), - new SystemTimer()); + StateMonitor monitor = StateMonitor.createForTesting(); Provider<MetricConsumer> provider = MetricConsumerProviders.wrap(monitor); performUpdates(provider, 8); for (int i = 1; i <= NUM_THREADS; i *= 2) { diff --git a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviders.java b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviders.java index b77836774d7..dd96414bbc7 100644 --- a/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviders.java +++ b/container-disc/src/test/java/com/yahoo/container/jdisc/metric/MetricConsumerProviders.java @@ -4,11 +4,9 @@ package com.yahoo.container.jdisc.metric; import com.yahoo.component.ComponentId; import com.yahoo.component.provider.ComponentRegistry; import com.yahoo.container.jdisc.MetricConsumerFactory; -import com.yahoo.container.jdisc.config.HealthMonitorConfig; -import com.yahoo.metrics.MetricsPresentationConfig; import com.yahoo.container.jdisc.state.StateMonitor; import com.yahoo.jdisc.application.MetricConsumer; -import com.yahoo.jdisc.core.SystemTimer; +import com.yahoo.metrics.MetricsPresentationConfig; /** * @author Simon Thoresen Hult @@ -26,13 +24,13 @@ class MetricConsumerProviders { public static MetricConsumerProvider newInstance(MetricConsumerFactory... factories) { return new MetricConsumerProvider(newComponentRegistry(factories), new MetricsPresentationConfig(new MetricsPresentationConfig.Builder()), - newStateMonitor()); + StateMonitor.createForTesting()); } public static MetricConsumerProvider newDefaultInstance() { return new MetricConsumerProvider(newComponentRegistry(), new MetricsPresentationConfig(new MetricsPresentationConfig.Builder()), - newStateMonitor()); + StateMonitor.createForTesting()); } public static ComponentRegistry<MetricConsumerFactory> newComponentRegistry(MetricConsumerFactory... factories) { @@ -43,9 +41,4 @@ class MetricConsumerProviders { return registry; } - public static StateMonitor newStateMonitor() { - return new StateMonitor(new HealthMonitorConfig(new HealthMonitorConfig.Builder()), - new SystemTimer()); - } - } |