Revert "Bratseth/simplify config take 2"

7 files changed, 51 insertions, 17 deletions

diff --git a/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java b/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java
index 75a660789e2..1133363be8e 100644
--- a/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java
+++ b/container-di/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java
@@ -30,9 +30,8 @@ public class CloudSubscriberFactory implements SubscriberFactory {
     private static final Logger log = Logger.getLogger(CloudSubscriberFactory.class.getName());
 
     private final ConfigSource configSource;
-    private final Map<CloudSubscriber, Integer> activeSubscribers = new WeakHashMap<>();
-
     private Optional<Long> testGeneration = Optional.empty();
+    private Map<CloudSubscriber, Integer> activeSubscribers = new WeakHashMap<>();
 
     public CloudSubscriberFactory(ConfigSource configSource) {
         this.configSource = configSource;
@@ -71,6 +70,9 @@ public class CloudSubscriberFactory implements SubscriberFactory {
         // if waitNextGeneration has not yet been called, -1 should be returned
         private long generation = -1L;
 
+        // True if this reconfiguration was caused by a system-internal redeploy, not an external application change
+        private boolean internalRedeploy = false;
+
         private CloudSubscriber(Set<ConfigKey<ConfigInstance>> keys, ConfigSource configSource) {
             this.subscriber = new ConfigSubscriber(configSource);
             keys.forEach(k -> handles.put(k, subscriber.subscribe(k.getConfigClass(), k.getConfigId())));
@@ -86,6 +88,11 @@ public class CloudSubscriberFactory implements SubscriberFactory {
             return generation;
         }
 
+        @Override
+        public boolean internalRedeploy() {
+            return internalRedeploy;
+        }
+
         //mapValues returns a view,, so we need to force evaluation of it here to prevent deferred evaluation.
         @Override
         public Map<ConfigKey<ConfigInstance>, ConfigInstance> config() {
@@ -121,6 +128,7 @@ public class CloudSubscriberFactory implements SubscriberFactory {
             }
 
             generation = subscriber.getGeneration();
+            internalRedeploy = subscriber.isInternalRedeploy();
             return generation;
         }
 
diff --git a/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java
index 44e38648230..f892fe410a8 100644
--- a/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java
+++ b/container-di/src/main/java/com/yahoo/container/di/ConfigRetriever.java
@@ -46,10 +46,11 @@ public final class ConfigRetriever {
     }
 
     public ConfigSnapshot getConfigs(Set<ConfigKey<? extends ConfigInstance>> componentConfigKeys,
-                                     long leastGeneration) {
+                                     long leastGeneration,
+                                     boolean restartOnRedeploy) {
         // Loop until we get config.
         while (true) {
-            Optional<ConfigSnapshot> maybeSnapshot = getConfigsOnce(componentConfigKeys, leastGeneration);
+            Optional<ConfigSnapshot> maybeSnapshot = getConfigsOnce(componentConfigKeys, leastGeneration, restartOnRedeploy);
             if (maybeSnapshot.isPresent()) {
                 var configSnapshot = maybeSnapshot.get();
                 resetComponentSubscriberIfBootstrap(configSnapshot);
@@ -58,8 +59,13 @@ public final class ConfigRetriever {
         }
     }
 
+    ConfigSnapshot getConfigs(Set<ConfigKey<? extends ConfigInstance>> componentConfigKeys, long leastGeneration) {
+        return getConfigs(componentConfigKeys, leastGeneration, false);
+    }
+
     Optional<ConfigSnapshot> getConfigsOnce(Set<ConfigKey<? extends ConfigInstance>> componentConfigKeys,
-                                            long leastGeneration) {
+                                            long leastGeneration,
+                                            boolean restartOnRedeploy) {
         if (!Sets.intersection(componentConfigKeys, bootstrapKeys).isEmpty()) {
             throw new IllegalArgumentException(
                     "Component config keys [" + componentConfigKeys + "] overlaps with bootstrap config keys [" + bootstrapKeys + "]");
@@ -70,16 +76,18 @@ public final class ConfigRetriever {
         allKeys.addAll(bootstrapKeys);
         setupComponentSubscriber(allKeys);
 
-        return getConfigsOptional(leastGeneration);
+        return getConfigsOptional(leastGeneration, restartOnRedeploy);
     }
 
-    private Optional<ConfigSnapshot> getConfigsOptional(long leastGeneration) {
+    private Optional<ConfigSnapshot> getConfigsOptional(long leastGeneration, boolean restartOnRedeploy) {
         long newestComponentGeneration = componentSubscriber.waitNextGeneration();
         log.log(FINE, "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) {
             return Optional.empty();
+        } else if (restartOnRedeploy && !componentSubscriber.internalRedeploy()) { // Don't reconfig - wait for restart
+            return Optional.empty();
         } else if (bootstrapSubscriber.generation() < newestComponentGeneration) {
             long newestBootstrapGeneration = bootstrapSubscriber.waitNextGeneration();
             log.log(FINE, "getConfigsOptional: new bootstrap generation: " + bootstrapSubscriber.generation());
diff --git a/container-di/src/main/java/com/yahoo/container/di/Container.java b/container-di/src/main/java/com/yahoo/container/di/Container.java
index 7855cd6b2f1..7fc2f2e55bc 100644
--- a/container-di/src/main/java/com/yahoo/container/di/Container.java
+++ b/container-di/src/main/java/com/yahoo/container/di/Container.java
@@ -72,10 +72,10 @@ public class Container {
         });
     }
 
-    public ComponentGraph getNewComponentGraph(ComponentGraph oldGraph, Injector fallbackInjector) {
+    public ComponentGraph getNewComponentGraph(ComponentGraph oldGraph, Injector fallbackInjector, boolean restartOnRedeploy) {
         try {
             Collection<Bundle> obsoleteBundles = new HashSet<>();
-            ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, obsoleteBundles);
+            ComponentGraph newGraph = getConfigAndCreateGraph(oldGraph, fallbackInjector, restartOnRedeploy, obsoleteBundles);
             newGraph.reuseNodes(oldGraph);
             constructComponents(newGraph);
             deconstructObsoleteComponents(oldGraph, newGraph, obsoleteBundles);
@@ -88,11 +88,12 @@ public class Container {
 
     private ComponentGraph getConfigAndCreateGraph(ComponentGraph graph,
                                                    Injector fallbackInjector,
+                                                   boolean restartOnRedeploy,
                                                    Collection<Bundle> obsoleteBundles) // NOTE: Return value
     {
         ConfigSnapshot snapshot;
         while (true) {
-            snapshot = configurer.getConfigs(graph.configKeys(), leastGeneration);
+            snapshot = configurer.getConfigs(graph.configKeys(), leastGeneration, restartOnRedeploy);
 
             log.log(FINE, String.format("createNewGraph:\n" + "graph.configKeys = %s\n" + "graph.generation = %s\n" + "snapshot = %s\n",
                                         graph.configKeys(), graph.generation(), snapshot));
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/test/java/com/yahoo/container/di/ConfigRetrieverTest.java b/container-di/src/test/java/com/yahoo/container/di/ConfigRetrieverTest.java
index 4751b9b74b7..e6b0309981a 100644
--- a/container-di/src/test/java/com/yahoo/container/di/ConfigRetrieverTest.java
+++ b/container-di/src/test/java/com/yahoo/container/di/ConfigRetrieverTest.java
@@ -51,7 +51,7 @@ public class ConfigRetrieverTest {
     public void require_that_bootstrap_configs_come_first() {
         writeConfigs();
         ConfigRetriever retriever = createConfigRetriever();
-        ConfigSnapshot bootstrapConfigs = retriever.getConfigs(Collections.emptySet(), 0);
+        ConfigSnapshot bootstrapConfigs = retriever.getConfigs(Collections.emptySet(), 0, false);
 
         assertThat(bootstrapConfigs, Matchers.instanceOf(BootstrapConfigs.class));
     }
@@ -61,7 +61,7 @@ public class ConfigRetrieverTest {
     public void require_that_components_comes_after_bootstrap() {
         writeConfigs();
         ConfigRetriever retriever = createConfigRetriever();
-        ConfigSnapshot bootstrapConfigs = retriever.getConfigs(Collections.emptySet(), 0);
+        ConfigSnapshot bootstrapConfigs = retriever.getConfigs(Collections.emptySet(), 0, false);
 
         ConfigKey<? extends ConfigInstance> testConfigKey = new ConfigKey<>(TestConfig.class, dirConfigSource.configId());
         ConfigSnapshot componentsConfigs = retriever.getConfigs(Collections.singleton(testConfigKey), 0);
@@ -73,6 +73,22 @@ public class ConfigRetrieverTest {
         }
     }
 
+    @Test
+    @SuppressWarnings("unused")
+    public void require_no_reconfig_when_restart_on_redeploy() {
+        // TODO
+        writeConfigs();
+        ConfigRetriever retriever = createConfigRetriever();
+        ConfigSnapshot bootstrapConfigs = retriever.getConfigs(Collections.emptySet(), 0, false);
+
+        ConfigKey<? extends ConfigInstance> testConfigKey = new ConfigKey<>(TestConfig.class, dirConfigSource.configId());
+        Optional<ConfigSnapshot> componentsConfigs = retriever.getConfigsOnce(Collections.singleton(testConfigKey), 0, true);
+
+        if (componentsConfigs.isPresent()) {
+            fail("Expected no configs");
+        }
+    }
+
     @Rule
     public ExpectedException expectedException = ExpectedException.none();
 
@@ -84,8 +100,8 @@ public class ConfigRetrieverTest {
         writeConfigs();
         ConfigRetriever retriever = createConfigRetriever();
         ConfigKey<? extends ConfigInstance> testConfigKey = new ConfigKey<>(TestConfig.class, dirConfigSource.configId());
-        ConfigSnapshot bootstrapConfigs = retriever.getConfigs(Collections.emptySet(), 0);
-        ConfigSnapshot componentsConfigs = retriever.getConfigs(Collections.singleton(testConfigKey), 0);
+        ConfigSnapshot bootstrapConfigs = retriever.getConfigs(Collections.emptySet(), 0, false);
+        ConfigSnapshot componentsConfigs = retriever.getConfigs(Collections.singleton(testConfigKey), 0, false);
         Set<ConfigKey<? extends ConfigInstance>> keys = new HashSet<>();
         keys.add(testConfigKey);
         keys.add(new ConfigKey<>(TestConfig.class, ""));
diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java
index 37d84df2612..d39e8a36aed 100644
--- a/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java
+++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTest.java
@@ -402,11 +402,11 @@ public class ContainerTest extends ContainerTestBase {
     }
 
     ComponentGraph getNewComponentGraph(Container container, ComponentGraph oldGraph) {
-        return container.getNewComponentGraph(oldGraph, Guice.createInjector());
+        return container.getNewComponentGraph(oldGraph, Guice.createInjector(), false);
     }
 
     ComponentGraph getNewComponentGraph(Container container) {
-        return container.getNewComponentGraph(new ComponentGraph(), Guice.createInjector());
+        return container.getNewComponentGraph(new ComponentGraph(), Guice.createInjector(), false);
     }
 
     private ComponentTakingConfig createComponentTakingConfig(ComponentGraph componentGraph) {
diff --git a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java
index 815865536f0..f1f3c4a2ae4 100644
--- a/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java
+++ b/container-di/src/test/java/com/yahoo/container/di/ContainerTestBase.java
@@ -70,7 +70,7 @@ public class ContainerTestBase {
                             throw new UnsupportedOperationException("getBundle not supported.");
                         }
                     });
-            componentGraph = container.getNewComponentGraph(componentGraph, Guice.createInjector());
+            componentGraph = container.getNewComponentGraph(componentGraph, Guice.createInjector(), false);
         } catch (Exception e) {
             throw new RuntimeException(e);
         }