diff options
author | Jon Bratseth <bratseth@oath.com> | 2018-05-30 11:39:26 +0200 |
---|---|---|
committer | Jon Bratseth <bratseth@oath.com> | 2018-05-30 11:39:26 +0200 |
commit | 4919fb84be7edc03f7c7feb0080ff0c5f705f654 (patch) | |
tree | 4eb3d1a1032c6a329b05acc05af4acc4eb0594e8 | |
parent | 9bfecc00be5716933ec305aa9161f68c4dca4200 (diff) |
Run reconfigurer thread even when restartOnRedeploy is true
9 files changed, 61 insertions, 53 deletions
diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java index b7c45de9f5b..b0af69d2dbf 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java @@ -24,7 +24,6 @@ import com.yahoo.vespa.config.TimingValues; * {@link ConfigHandle} which {@link #subscribe(Class, String)} returned. * * @author vegardh - * @since 5.1 */ public class ConfigSubscriber { diff --git a/container-core/src/main/java/com/yahoo/container/Server.java b/container-core/src/main/java/com/yahoo/container/Server.java index 207050a8d88..2eec2e36c47 100644 --- a/container-core/src/main/java/com/yahoo/container/Server.java +++ b/container-core/src/main/java/com/yahoo/container/Server.java @@ -19,7 +19,7 @@ public class Server { private ConfigSubscriber subscriber = new ConfigSubscriber(); /** The OSGi container instance of this server */ - private Container container=Container.get(); + private Container container = Container.get(); /** A short string which is different for all the qrserver instances on a given node. */ private String localServerDiscriminator = "qrserver.0"; @@ -48,7 +48,6 @@ public class Server { return instance; } - private void initRpcServer(Rpc rpcConfig) { if (rpcConfig.enabled()) { ContainerRpcAdaptor rpcAdaptor = container.getRpcAdaptor(); @@ -57,8 +56,7 @@ public class Server { } } - /** Ugly hack, see Container.resetInstance - **/ + /** Ugly hack, see Container.resetInstance */ static void resetInstance() { instance = new Server(); } diff --git a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java index 49cacb3a09b..41e3e43f156 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/HandlersConfigurerDi.java @@ -86,7 +86,7 @@ public class HandlersConfigurerDi { container = new Container(subscriberFactory, configId, deconstructor, osgiWrapper); try { - runOnceAndEnsureRegistryHackRun(discInjector); + getNewConfigGraph(discInjector, false); } catch (InterruptedException e) { throw new RuntimeException("Interrupted while setting up handlers for the first time."); } @@ -138,11 +138,18 @@ public class HandlersConfigurerDi { } } - public void runOnceAndEnsureRegistryHackRun(Injector discInjector) throws InterruptedException { - currentGraph = container.runOnce(currentGraph, createFallbackInjector(vespaContainer, discInjector)); - - RegistriesHack registriesHack = currentGraph.getInstance(RegistriesHack.class); - assert (registriesHack != null); + /** + * Wait for new config to arrive and produce the new graph + * + * @return true if this resulted in a new graph that should be applied to the currently running container + */ + public boolean getNewConfigGraph(Injector discInjector, boolean restartOnRedeploy) throws InterruptedException { + ComponentGraph newGraph = container.getNewConfigGraph(currentGraph, createFallbackInjector(vespaContainer, discInjector), restartOnRedeploy); + if (newGraph == currentGraph) return false; + currentGraph = newGraph; + + assert (currentGraph.getInstance(RegistriesHack.class) != null); // TODO: Remove, seems quite pointless? + return true; } @SuppressWarnings("deprecation") diff --git a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java index 89c73e19fe3..008ca5c1d5a 100644 --- a/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java +++ b/container-core/src/main/java/com/yahoo/container/core/config/testutil/HandlersConfigurerTestWrapper.java @@ -112,7 +112,7 @@ public class HandlersConfigurerTestWrapper { public void reloadConfig() { configurer.reloadConfig(++lastGeneration); try { - configurer.runOnceAndEnsureRegistryHackRun(Guice.createInjector()); + configurer.getNewConfigGraph(Guice.createInjector(), false); } catch (InterruptedException e) { throw new RuntimeException(e); } diff --git a/container-di/src/main/scala/com/yahoo/container/di/Container.scala b/container-di/src/main/scala/com/yahoo/container/di/Container.scala index 5ea6422ddad..0ef01e5aa3d 100644 --- a/container-di/src/main/scala/com/yahoo/container/di/Container.scala +++ b/container-di/src/main/scala/com/yahoo/container/di/Container.scala @@ -44,9 +44,9 @@ class Container( var leastGeneration = -1L @throws(classOf[InterruptedException]) - def runOnce( - oldGraph: ComponentGraph = new ComponentGraph, - fallbackInjector: GuiceInjector = Guice.createInjector()): ComponentGraph = { + def getNewConfigGraph(oldGraph: ComponentGraph = new ComponentGraph, + fallbackInjector: GuiceInjector = Guice.createInjector(), + restartOnRedeploy: Boolean = false): ComponentGraph = { def deconstructObsoleteComponents(oldGraph: ComponentGraph, newGraph: ComponentGraph) { val oldComponents = new IdentityHashMap[AnyRef, AnyRef]() @@ -56,7 +56,8 @@ class Container( } try { - val newGraph = createNewGraph(oldGraph, fallbackInjector) + val newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector) + if (restartOnRedeploy) return oldGraph // wait for restart to cause reconfig instead newGraph.reuseNodes(oldGraph) constructComponents(newGraph) deconstructObsoleteComponents(oldGraph, newGraph) @@ -113,10 +114,11 @@ class Container( } } - final def createNewGraph(graph: ComponentGraph = new ComponentGraph, - fallbackInjector: Injector): ComponentGraph = { + final def getConfigAndCreateGraph(graph: ComponentGraph = new ComponentGraph, + fallbackInjector: Injector): ComponentGraph = { val snapshot = configurer.getConfigs(graph.configKeys, leastGeneration) + log.log(DEBUG, """createNewGraph: |graph.configKeys = %s @@ -138,7 +140,7 @@ class Container( |previous generation: %d""" .format(getBootstrapGeneration, getComponentsGeneration, previousConfigGeneration).stripMargin) installBundles(configs) - createNewGraph( + getConfigAndCreateGraph( createComponentsGraph(configs, getBootstrapGeneration,fallbackInjector), fallbackInjector) case ComponentsConfigs(configs) => diff --git a/container-di/src/test/java/demo/ContainerTestBase.java b/container-di/src/test/java/demo/ContainerTestBase.java index 426033ea101..d413a61759b 100644 --- a/container-di/src/test/java/demo/ContainerTestBase.java +++ b/container-di/src/test/java/demo/ContainerTestBase.java @@ -12,12 +12,9 @@ import com.yahoo.container.di.Osgi; import com.yahoo.container.di.componentgraph.core.ComponentGraph; import org.junit.Before; import org.osgi.framework.Bundle; -import scala.collection.*; -import scala.collection.immutable.*; import scala.collection.immutable.Set; import java.util.Collection; -import java.util.List; /** * @author tonytv @@ -62,7 +59,7 @@ public class ContainerTestBase extends ContainerTest { throw new UnsupportedOperationException("getBundle not supported."); } }); - componentGraph = container.runOnce(componentGraph, Guice.createInjector()); + componentGraph = container.getNewConfigGraph(componentGraph, Guice.createInjector(), false); } catch (Exception e) { throw new RuntimeException(e); } diff --git a/container-di/src/test/scala/com/yahoo/container/di/ContainerTest.scala b/container-di/src/test/scala/com/yahoo/container/di/ContainerTest.scala index 8e7fbde3f5e..83645a8b35c 100644 --- a/container-di/src/test/scala/com/yahoo/container/di/ContainerTest.scala +++ b/container-di/src/test/scala/com/yahoo/container/di/ContainerTest.scala @@ -44,7 +44,7 @@ class ContainerTest { val container = newContainer(dirConfigSource) - val component = createComponentTakingConfig(container.runOnce()) + val component = createComponentTakingConfig(container.getNewConfigGraph()) assertThat(component.config.stringVal(), is("myString")) container.shutdownConfigurer() @@ -57,7 +57,7 @@ class ContainerTest { val container = newContainer(dirConfigSource) - val componentGraph = container.runOnce() + val componentGraph = container.getNewConfigGraph() val component = createComponentTakingConfig(componentGraph) assertThat(component.config.stringVal(), is("original")) @@ -66,7 +66,7 @@ class ContainerTest { dirConfigSource.writeConfig("test", """stringVal "reconfigured" """) container.reloadConfig(2) - val newComponentGraph = container.runOnce(componentGraph) + val newComponentGraph = container.getNewConfigGraph(componentGraph) val component2 = createComponentTakingConfig(newComponentGraph) assertThat(component2.config.stringVal(), is("reconfigured")) @@ -80,7 +80,7 @@ class ContainerTest { val container = newContainer(dirConfigSource) - val graph = container.runOnce() + val graph = container.getNewConfigGraph() val component = createComponentTakingConfig(graph) assertThat(component.getId.toString, is("id1")) @@ -89,7 +89,7 @@ class ContainerTest { ("id2", classOf[ComponentTakingConfig]))) container.reloadConfig(2) - val newGraph = container.runOnce(graph) + val newGraph = container.getNewConfigGraph(graph) assertThat(ComponentGraph.getNode(newGraph, "id1"), notNullValue(classOf[Node])) assertThat(ComponentGraph.getNode(newGraph, "id2"), notNullValue(classOf[Node])) @@ -107,12 +107,12 @@ class ContainerTest { val container = newContainer(dirConfigSource) - val oldGraph = container.runOnce() + val oldGraph = container.getNewConfigGraph() val componentToDestruct = oldGraph.getInstance(classOf[DestructableComponent]) writeBootstrapConfigs("id2", classOf[DestructableComponent]) container.reloadConfig(2) - container.runOnce(oldGraph) + container.getNewConfigGraph(oldGraph) assertTrue(componentToDestruct.deconstructed) } @@ -123,7 +123,7 @@ class ContainerTest { val container = newContainer(dirConfigSource) var currentGraph: ComponentGraph = null try { - currentGraph = container.runOnce() + currentGraph = container.getNewConfigGraph() fail("Expected to log and die.") } catch { case _: Throwable => fail("Expected to log and die") @@ -136,14 +136,14 @@ class ContainerTest { writeBootstrapConfigs(Array(simpleComponentEntry)) val container = newContainer(dirConfigSource) - var currentGraph = container.runOnce() + var currentGraph = container.getNewConfigGraph() val simpleComponent = currentGraph.getInstance(classOf[SimpleComponent]) writeBootstrapConfigs("thrower", classOf[ComponentThrowingExceptionInConstructor]) container.reloadConfig(2) try { - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewConfigGraph(currentGraph) fail("Expected exception") } catch { case _: ComponentConstructorException => // Expected, do nothing @@ -156,7 +156,7 @@ class ContainerTest { dirConfigSource.writeConfig("test", """stringVal "myString" """) writeBootstrapConfigs(Array(simpleComponentEntry, componentTakingConfigEntry)) container.reloadConfig(3) - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewConfigGraph(currentGraph) assertEquals(3, currentGraph.generation) assertSame(simpleComponent, currentGraph.getInstance(classOf[SimpleComponent])) @@ -169,7 +169,7 @@ class ContainerTest { writeBootstrapConfigs(Array(simpleComponentEntry)) val container = newContainer(dirConfigSource) - var currentGraph = container.runOnce() + var currentGraph = container.getNewConfigGraph() val simpleComponent = currentGraph.getInstance(classOf[SimpleComponent]) @@ -177,7 +177,7 @@ class ContainerTest { dirConfigSource.writeConfig("test", """stringVal "myString" """) container.reloadConfig(2) try { - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewConfigGraph(currentGraph) fail("Expected exception") } catch { case _: IllegalArgumentException => // Expected, do nothing @@ -192,20 +192,20 @@ class ContainerTest { writeBootstrapConfigs("myId", classOf[ComponentTakingConfig]) val container = newContainer(dirConfigSource) - var currentGraph = container.runOnce() + var currentGraph = container.getNewConfigGraph() writeBootstrapConfigs("thrower", classOf[ComponentThrowingExceptionForMissingConfig]) container.reloadConfig(2) try { - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewConfigGraph(currentGraph) fail("expected exception") } catch { case e: Exception => } val newGraph = Future { - currentGraph = container.runOnce(currentGraph) + currentGraph = container.getNewConfigGraph(currentGraph) currentGraph } @@ -230,7 +230,7 @@ class ContainerTest { dirConfigSource.writeConfig("jersey-injection", """inject[0]" """) val container = newContainer(dirConfigSource) - val componentGraph = container.runOnce() + val componentGraph = container.getNewConfigGraph() val restApiContext = componentGraph.getInstance(clazz) assertNotNull(restApiContext) @@ -278,7 +278,7 @@ class ContainerTest { dirConfigSource.writeConfig("jersey-injection", injectionConfig) val container = newContainer(dirConfigSource) - val componentGraph = container.runOnce() + val componentGraph = container.getNewConfigGraph() val restApiContext = componentGraph.getInstance(restApiClass) } @@ -328,12 +328,12 @@ class ContainerTest { val container = newContainer(dirConfigSource, deconstructor) - val oldGraph = container.runOnce() + val oldGraph = container.getNewConfigGraph() val destructableEntity = oldGraph.getInstance(classOf[DestructableEntity]) writeBootstrapConfigs("id2", classOf[DestructableProvider]) container.reloadConfig(2) - container.runOnce(oldGraph) + container.getNewConfigGraph(oldGraph) assertTrue(destructableEntity.deconstructed) } diff --git a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java index 1977b934253..d42afadfd9b 100644 --- a/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java +++ b/container-disc/src/main/java/com/yahoo/container/jdisc/ConfiguredApplication.java @@ -128,7 +128,7 @@ public final class ConfiguredApplication implements Application { configureComponents(builder.guiceModules().activate()); intitializeAndActivateContainer(builder); - if (! qrConfig.restartOnDeploy()) startReconfigurerThread(); + startReconfigurerThread(); portWatcher = new Thread(this::watchPortChange); portWatcher.setDaemon(true); portWatcher.start(); @@ -199,8 +199,12 @@ public final class ConfiguredApplication implements Application { while ( ! Thread.interrupted()) { try { ContainerBuilder builder = createBuilderWithGuiceBindings(); - configurer.runOnceAndEnsureRegistryHackRun(builder.guiceModules().activate()); - intitializeAndActivateContainer(builder); + + // Block until new config: + boolean gotNewConfigToApply = configurer.getNewConfigGraph(builder.guiceModules().activate(), + qrConfig.restartOnDeploy()); + if (gotNewConfigToApply) + intitializeAndActivateContainer(builder); } catch (ConfigInterruptedException | InterruptedException e) { break; } catch (Exception | LinkageError e) { // LinkageError: OSGi problems diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerActivator.java b/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerActivator.java index 9d1b613e23c..59db453e2c4 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerActivator.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/application/ContainerActivator.java @@ -13,27 +13,28 @@ import com.yahoo.jdisc.Container; * #newContainerBuilder()}, 2) configure the returned {@link ContainerBuilder}, and 3) pass the builder to the {@link * #activateContainer(ContainerBuilder)} method.</p> * - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> + * @author Simon Thoresen */ public interface ContainerActivator { /** - * <p>This method creates and returns a new {@link ContainerBuilder} object that has the necessary references to the - * application and its internal components.</p> + * This method creates and returns a new {@link ContainerBuilder} object that has the necessary references to the + * application and its internal components. * * @return The created builder. */ - public ContainerBuilder newContainerBuilder(); + ContainerBuilder newContainerBuilder(); /** - * <p>Creates and activates a {@link Container} based on the provided {@link ContainerBuilder}. By providing a + * Creates and activates a {@link Container} based on the provided {@link ContainerBuilder}. By providing a * <em>null</em> argument, this method can be used to deactivate the current Container. The returned object can be - * used to schedule a cleanup task that is executed once the the deactivated Container has terminated.</p> + * used to schedule a cleanup task that is executed once the the deactivated Container has terminated. * * @param builder The builder to activate. * @return The previous container, if any. * @throws ApplicationNotReadyException If this method is called before {@link Application#start()} or after {@link * Application#stop()}. */ - public DeactivatedContainer activateContainer(ContainerBuilder builder); + DeactivatedContainer activateContainer(ContainerBuilder builder); + } |