Separate internal redeploys from application package changes

Separate system internal redeploys from application package
changes such that we can ignore config updates originating
from application package changes if restart on deploy is true,
as the config change will be followed by a restart - but continue
to effect config changes originating from internal system changes
as those will not (and should not) be followed by a restart.

6 files changed, 41 insertions, 35 deletions

diff --git a/container-di/src/main/java/com/yahoo/container/di/config/Subscriber.java b/container-di/src/main/java/com/yahoo/container/di/config/Subscriber.java
index 9fd30f888b9..0feab7779ad 100644
--- a/container-di/src/main/java/com/yahoo/container/di/config/Subscriber.java
+++ b/container-di/src/main/java/com/yahoo/container/di/config/Subscriber.java
@@ -14,6 +14,7 @@ public interface Subscriber {
 
     long waitNextGeneration();
     long generation();
+    boolean internalRedeploy();
 
     boolean configChanged();
     Map<ConfigKey<ConfigInstance>, ConfigInstance> config();
diff --git a/container-di/src/main/scala/com/yahoo/container/di/CloudSubscriberFactory.scala b/container-di/src/main/scala/com/yahoo/container/di/CloudSubscriberFactory.scala
index afef3e96821..e38ff9d4491 100644
--- a/container-di/src/main/scala/com/yahoo/container/di/CloudSubscriberFactory.scala
+++ b/container-di/src/main/scala/com/yahoo/container/di/CloudSubscriberFactory.scala
@@ -50,9 +50,12 @@ object CloudSubscriberFactory {
     private val handles: Map[ConfigKeyT, ConfigHandle[_ <: ConfigInstance]] = keys.map(subscribe).toMap
 
 
-    //if waitNextGeneration has not yet been called, -1 should be returned
+    // if waitNextGeneration has not yet been called, -1 should be returned
     var generation: Long = -1
 
+    // True if this reconfiguration was caused by a system-internal redeploy, not an external application change
+    var internalRedeploy: Boolean = false
+
     private def subscribe(key: ConfigKeyT) = (key, subscriber.subscribe(key.getConfigClass, key.getConfigId))
 
     override def configChanged = handles.values.exists(_.isChanged)
@@ -86,6 +89,7 @@ object CloudSubscriberFactory {
       }
 
       generation = subscriber.getGeneration
+      internalRedeploy = subscriber.isInternalRedeploy
       generation
     }
 
diff --git a/container-di/src/main/scala/com/yahoo/container/di/ConfigRetriever.scala b/container-di/src/main/scala/com/yahoo/container/di/ConfigRetriever.scala
index dc94d789f7b..0d829b0456d 100644
--- a/container-di/src/main/scala/com/yahoo/container/di/ConfigRetriever.scala
+++ b/container-di/src/main/scala/com/yahoo/container/di/ConfigRetriever.scala
@@ -29,25 +29,27 @@ final class ConfigRetriever(bootstrapKeys: Set[ConfigKeyT],
 
 
   @tailrec
-  final def getConfigs(componentConfigKeys: Set[ConfigKeyT], leastGeneration: Long): ConfigSnapshot = {
+  final def getConfigs(componentConfigKeys: Set[ConfigKeyT], leastGeneration: Long, restartOnRedeploy: Boolean = false): ConfigSnapshot = {
     require(componentConfigKeys intersect bootstrapKeys isEmpty)
     log.log(DEBUG, "getConfigs: " + componentConfigKeys)
 
     setupComponentSubscriber(componentConfigKeys ++ bootstrapKeys)
 
-    getConfigsOptional(leastGeneration) match {
+    getConfigsOptional(leastGeneration, restartOnRedeploy) match {
       case Some(snapshot) => resetComponentSubscriberIfBootstrap(snapshot); snapshot
-      case None => getConfigs(componentConfigKeys, leastGeneration)
+      case None => getConfigs(componentConfigKeys, leastGeneration, restartOnRedeploy)
     }
   }
 
-  private def getConfigsOptional(leastGeneration: Long): Option[ConfigSnapshot] = {
+  private def getConfigsOptional(leastGeneration: Long, restartOnRedeploy: Boolean): Option[ConfigSnapshot] = {
     val newestComponentGeneration = componentSubscriber.waitNextGeneration()
     log.log(DEBUG, s"getConfigsOptional: new component generation: $newestComponentGeneration")
 
     // leastGeneration is only used to ensure newer generation when the previous generation was invalidated due to an exception
     if (newestComponentGeneration < leastGeneration) {
       None
+    } else if (restartOnRedeploy && ! componentSubscriber.internalRedeploy()) { // Don't reconfig - wait for restart
+      None
     } else if (bootstrapSubscriber.generation < newestComponentGeneration) {
       val newestBootstrapGeneration = bootstrapSubscriber.waitNextGeneration()
       log.log(DEBUG, s"getConfigsOptional: new bootstrap generation: ${bootstrapSubscriber.generation}")
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 0ef01e5aa3d..2a185d41a6c 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 getNewConfigGraph(oldGraph: ComponentGraph = new ComponentGraph,
-                        fallbackInjector: GuiceInjector = Guice.createInjector(),
-                        restartOnRedeploy: Boolean = false): ComponentGraph = {
+  def getNewComponentGraph(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,8 +56,7 @@ class Container(
     }
 
     try {
-      val newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector)
-      if (restartOnRedeploy) return oldGraph // wait for restart to cause reconfig instead
+      val newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy)
       newGraph.reuseNodes(oldGraph)
       constructComponents(newGraph)
       deconstructObsoleteComponents(oldGraph, newGraph)
@@ -115,9 +114,10 @@ class Container(
   }
 
   final def getConfigAndCreateGraph(graph: ComponentGraph = new ComponentGraph,
-                                    fallbackInjector: Injector): ComponentGraph = {
+                                    fallbackInjector: Injector,
+                                    restartOnRedeploy: Boolean): ComponentGraph = {
 
-    val snapshot = configurer.getConfigs(graph.configKeys, leastGeneration)
+    val snapshot = configurer.getConfigs(graph.configKeys, leastGeneration, restartOnRedeploy)
 
     log.log(DEBUG,
             """createNewGraph:
@@ -141,8 +141,7 @@ class Container(
               .format(getBootstrapGeneration, getComponentsGeneration, previousConfigGeneration).stripMargin)
           installBundles(configs)
           getConfigAndCreateGraph(
-            createComponentsGraph(configs, getBootstrapGeneration,fallbackInjector),
-            fallbackInjector)
+            createComponentsGraph(configs, getBootstrapGeneration,fallbackInjector), fallbackInjector, restartOnRedeploy)
         case ComponentsConfigs(configs) =>
           log.log(DEBUG,
             """Got components configs,
diff --git a/container-di/src/test/java/demo/ContainerTestBase.java b/container-di/src/test/java/demo/ContainerTestBase.java
index d413a61759b..9c2415c3514 100644
--- a/container-di/src/test/java/demo/ContainerTestBase.java
+++ b/container-di/src/test/java/demo/ContainerTestBase.java
@@ -59,7 +59,7 @@ public class ContainerTestBase extends ContainerTest {
                             throw new UnsupportedOperationException("getBundle not supported.");
                         }
                     });
-            componentGraph = container.getNewConfigGraph(componentGraph, Guice.createInjector(), false);
+            componentGraph = container.getNewComponentGraph(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 83645a8b35c..9f07acc7dc9 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.getNewConfigGraph())
+    val component = createComponentTakingConfig(container.getNewComponentGraph())
     assertThat(component.config.stringVal(), is("myString"))
 
     container.shutdownConfigurer()
@@ -57,7 +57,7 @@ class ContainerTest {
 
     val container = newContainer(dirConfigSource)
 
-    val componentGraph = container.getNewConfigGraph()
+    val componentGraph = container.getNewComponentGraph()
     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.getNewConfigGraph(componentGraph)
+    val newComponentGraph = container.getNewComponentGraph(componentGraph)
     val component2 = createComponentTakingConfig(newComponentGraph)
     assertThat(component2.config.stringVal(), is("reconfigured"))
 
@@ -80,7 +80,7 @@ class ContainerTest {
 
     val container = newContainer(dirConfigSource)
 
-    val graph = container.getNewConfigGraph()
+    val graph = container.getNewComponentGraph()
     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.getNewConfigGraph(graph)
+    val newGraph = container.getNewComponentGraph(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.getNewConfigGraph()
+    val oldGraph = container.getNewComponentGraph()
     val componentToDestruct = oldGraph.getInstance(classOf[DestructableComponent])
 
     writeBootstrapConfigs("id2", classOf[DestructableComponent])
     container.reloadConfig(2)
-    container.getNewConfigGraph(oldGraph)
+    container.getNewComponentGraph(oldGraph)
     assertTrue(componentToDestruct.deconstructed)
   }
 
@@ -123,7 +123,7 @@ class ContainerTest {
     val container = newContainer(dirConfigSource)
     var currentGraph: ComponentGraph = null
     try {
-      currentGraph = container.getNewConfigGraph()
+      currentGraph = container.getNewComponentGraph()
       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.getNewConfigGraph()
+    var currentGraph = container.getNewComponentGraph()
 
     val simpleComponent = currentGraph.getInstance(classOf[SimpleComponent])
 
     writeBootstrapConfigs("thrower", classOf[ComponentThrowingExceptionInConstructor])
     container.reloadConfig(2)
     try {
-      currentGraph = container.getNewConfigGraph(currentGraph)
+      currentGraph = container.getNewComponentGraph(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.getNewConfigGraph(currentGraph)
+    currentGraph = container.getNewComponentGraph(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.getNewConfigGraph()
+    var currentGraph = container.getNewComponentGraph()
 
     val simpleComponent = currentGraph.getInstance(classOf[SimpleComponent])
 
@@ -177,7 +177,7 @@ class ContainerTest {
     dirConfigSource.writeConfig("test", """stringVal "myString" """)
     container.reloadConfig(2)
     try {
-      currentGraph = container.getNewConfigGraph(currentGraph)
+      currentGraph = container.getNewComponentGraph(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.getNewConfigGraph()
+    var currentGraph = container.getNewComponentGraph()
 
     writeBootstrapConfigs("thrower", classOf[ComponentThrowingExceptionForMissingConfig])
     container.reloadConfig(2)
 
     try {
-      currentGraph = container.getNewConfigGraph(currentGraph)
+      currentGraph = container.getNewComponentGraph(currentGraph)
       fail("expected exception")
     } catch {
       case e: Exception =>
     }
 
     val newGraph = Future {
-      currentGraph = container.getNewConfigGraph(currentGraph)
+      currentGraph = container.getNewComponentGraph(currentGraph)
       currentGraph
     }
 
@@ -230,7 +230,7 @@ class ContainerTest {
     dirConfigSource.writeConfig("jersey-injection", """inject[0]" """)
 
     val container = newContainer(dirConfigSource)
-    val componentGraph = container.getNewConfigGraph()
+    val componentGraph = container.getNewComponentGraph()
 
     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.getNewConfigGraph()
+    val componentGraph = container.getNewComponentGraph()
 
     val restApiContext = componentGraph.getInstance(restApiClass)
   }
@@ -328,12 +328,12 @@ class ContainerTest {
 
     val container = newContainer(dirConfigSource, deconstructor)
 
-    val oldGraph = container.getNewConfigGraph()
+    val oldGraph = container.getNewComponentGraph()
     val destructableEntity = oldGraph.getInstance(classOf[DestructableEntity])
 
     writeBootstrapConfigs("id2", classOf[DestructableProvider])
     container.reloadConfig(2)
-    container.getNewConfigGraph(oldGraph)
+    container.getNewComponentGraph(oldGraph)
 
     assertTrue(destructableEntity.deconstructed)
   }