From 2942139738c7756503ae97d39394e1443abf8d3f Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 4 Oct 2021 16:00:17 +0200 Subject: Improve comment --- .../src/main/java/com/yahoo/container/di/ConfigRetriever.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java index 1e7f3e06049..73b7126a11f 100644 --- a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java +++ b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java @@ -77,7 +77,8 @@ public final class ConfigRetriever { long newestComponentGeneration = componentSubscriber.waitNextGeneration(isInitializing); 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 + // leastGeneration is only used to ensure newer generation (than the latest bootstrap or component gen) + // when the previous generation was invalidated due to an exception upon creating the component graph. if (newestComponentGeneration < leastGeneration) { return Optional.empty(); } else if (bootstrapSubscriber.generation() < newestComponentGeneration) { -- cgit v1.2.3 From 1a85e624f46ebc00b370eeeb17f9dce60ddf8c9f Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 4 Oct 2021 16:31:08 +0200 Subject: Use correct method name in log message. --- container-core/src/main/java/com/yahoo/container/di/Container.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/Container.java b/container-core/src/main/java/com/yahoo/container/di/Container.java index bec685c71ef..626f97dd25c 100644 --- a/container-core/src/main/java/com/yahoo/container/di/Container.java +++ b/container-core/src/main/java/com/yahoo/container/di/Container.java @@ -94,7 +94,7 @@ public class Container { snapshot = configurer.getConfigs(graph.configKeys(), leastGeneration, isInitializing); if (log.isLoggable(FINE)) - log.log(FINE, String.format("createNewGraph:\n" + "graph.configKeys = %s\n" + "graph.generation = %s\n" + "snapshot = %s\n", + log.log(FINE, String.format("getConfigAndCreateGraph:\n" + "graph.configKeys = %s\n" + "graph.generation = %s\n" + "snapshot = %s\n", graph.configKeys(), graph.generation(), snapshot)); if (snapshot instanceof BootstrapConfigs) { -- cgit v1.2.3 From f01c95e5cbf03c8a713948acb4d2e80f6d935a92 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Mon, 4 Oct 2021 23:18:27 +0200 Subject: Add more debug log for config generations. --- .../main/java/com/yahoo/container/di/CloudSubscriberFactory.java | 8 ++++++-- .../src/main/java/com/yahoo/container/di/ConfigRetriever.java | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java index d50d56f8987..665da965148 100644 --- a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java +++ b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java @@ -21,6 +21,8 @@ import java.util.WeakHashMap; import java.util.logging.Level; import java.util.logging.Logger; +import static java.util.logging.Level.FINE; + /** * @author Tony Vaagenes * @author ollivir @@ -108,12 +110,14 @@ public class CloudSubscriberFactory implements SubscriberFactory { int numExceptions = 0; while ( ! gotNextGen) { try { - if (subscriber.nextGeneration(isInitializing)) + if (subscriber.nextGeneration(isInitializing)) { gotNextGen = true; + log.log(FINE, () -> this + " got next config generation " + subscriber.getGeneration() + "\n" + subscriber.toString()); + } } catch (IllegalArgumentException e) { numExceptions++; - log.log(Level.WARNING, "Got exception from the config system (ignore if you just removed a " + + log.log(Level.WARNING, this + " got exception from the config system (ignore if you just removed a " + "component from your application that used the mentioned config) Subscriber info: " + subscriber.toString(), e); if (numExceptions >= 5) diff --git a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java index 73b7126a11f..200b963bcad 100644 --- a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java +++ b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java @@ -89,7 +89,7 @@ public final class ConfigRetriever { return bootstrapConfig; } else { if (newestBootstrapGeneration == newestComponentGeneration) { - log.log(FINE, () -> "Got new components configs with unchanged bootstrap configs."); + log.log(FINE, () -> this + " got new components configs with unchanged bootstrap configs."); return componentsConfigIfChanged(); } else { // This should not be a normal case, and hence a warning to allow investigation. -- cgit v1.2.3 From b3733a0d0b08377601edee806459cb9eec1024d0 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 6 Oct 2021 12:15:04 +0200 Subject: Move CloudSubscriber to separate class file. --- .../com/yahoo/container/di/CloudSubscriber.java | 99 ++++++++++++++++++++++ .../yahoo/container/di/CloudSubscriberFactory.java | 77 +---------------- 2 files changed, 101 insertions(+), 75 deletions(-) create mode 100644 container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java new file mode 100644 index 00000000000..4efb36d4b29 --- /dev/null +++ b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java @@ -0,0 +1,99 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.container.di; + +import com.yahoo.config.ConfigInstance; +import com.yahoo.config.subscription.ConfigHandle; +import com.yahoo.config.subscription.ConfigSource; +import com.yahoo.config.subscription.ConfigSubscriber; +import com.yahoo.container.di.config.Subscriber; +import com.yahoo.vespa.config.ConfigKey; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; + +import static java.util.logging.Level.FINE; + +/** + * @author gjoranv + * @author Tony Vaagenes + * @author ollivir + */ +public class CloudSubscriber implements Subscriber { + private static final Logger log = Logger.getLogger(CloudSubscriber.class.getName()); + + private final ConfigSubscriber subscriber; + private final Map, ConfigHandle> handles = new HashMap<>(); + + // if waitNextGeneration has not yet been called, -1 should be returned + private long generation = -1L; + + CloudSubscriber(Set> keys, ConfigSource configSource) { + this.subscriber = new ConfigSubscriber(configSource); + keys.forEach(k -> handles.put(k, subscriber.subscribe(k.getConfigClass(), k.getConfigId()))); + } + + @Override + public boolean configChanged() { + return handles.values().stream().anyMatch(ConfigHandle::isChanged); + } + + @Override + public long generation() { + return generation; + } + + //mapValues returns a view,, so we need to force evaluation of it here to prevent deferred evaluation. + @Override + public Map, ConfigInstance> config() { + Map, ConfigInstance> ret = new HashMap<>(); + handles.forEach((k, v) -> ret.put(k, v.getConfig())); + return ret; + } + + @Override + public long waitNextGeneration(boolean isInitializing) { + if (handles.isEmpty()) + throw new IllegalStateException("No config keys registered"); + + // Catch and just log config exceptions due to missing config values for parameters that do + // not have a default value. These exceptions occur when the user has removed a component + // from services.xml, and the component takes a config that has parameters without a + // default value in the def-file. There is a new 'components' config underway, where the + // component is removed, so this old config generation will soon be replaced by a new one. + boolean gotNextGen = false; + int numExceptions = 0; + while ( ! gotNextGen) { + try { + if (subscriber.nextGeneration(isInitializing)) { + gotNextGen = true; + log.log(FINE, () -> this + " got next config generation " + subscriber.getGeneration() + "\n" + subscriber.toString()); + } + } + catch (IllegalArgumentException e) { + numExceptions++; + log.log(Level.WARNING, this + " got exception from the config system (ignore if you just removed a " + + "component from your application that used the mentioned config) Subscriber info: " + + subscriber.toString(), e); + if (numExceptions >= 5) + throw new IllegalArgumentException("Failed retrieving the next config generation", e); + } + } + + generation = subscriber.getGeneration(); + return generation; + } + + @Override + public void close() { + subscriber.close(); + } + + // TODO: Remove, only used by test specific code in CloudSubscriberFactory + ConfigSubscriber getSubscriber() { + return subscriber; + } + +} diff --git a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java index 665da965148..38f7ded28f8 100644 --- a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java +++ b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java @@ -29,8 +29,6 @@ import static java.util.logging.Level.FINE; */ public class CloudSubscriberFactory implements SubscriberFactory { - private static final Logger log = Logger.getLogger(CloudSubscriberFactory.class.getName()); - private final ConfigSource configSource; private final Map activeSubscribers = new WeakHashMap<>(); @@ -50,7 +48,7 @@ public class CloudSubscriberFactory implements SubscriberFactory { } CloudSubscriber subscriber = new CloudSubscriber(subscriptionKeys, configSource); - testGeneration.ifPresent(subscriber.subscriber::reload); // TODO: test specific code, remove + testGeneration.ifPresent(subscriber.getSubscriber()::reload); // TODO: test specific code, remove activeSubscribers.put(subscriber, 0); return subscriber; @@ -62,78 +60,7 @@ public class CloudSubscriberFactory implements SubscriberFactory { testGeneration = Optional.of(generation); List subscribers = new ArrayList<>(activeSubscribers.keySet()); - subscribers.forEach(s -> s.subscriber.reload(generation)); - } - - private static class CloudSubscriber implements Subscriber { - - private final ConfigSubscriber subscriber; - private final Map, ConfigHandle> handles = new HashMap<>(); - - // if waitNextGeneration has not yet been called, -1 should be returned - private long generation = -1L; - - private CloudSubscriber(Set> keys, ConfigSource configSource) { - this.subscriber = new ConfigSubscriber(configSource); - keys.forEach(k -> handles.put(k, subscriber.subscribe(k.getConfigClass(), k.getConfigId()))); - } - - @Override - public boolean configChanged() { - return handles.values().stream().anyMatch(ConfigHandle::isChanged); - } - - @Override - public long generation() { - return generation; - } - - //mapValues returns a view,, so we need to force evaluation of it here to prevent deferred evaluation. - @Override - public Map, ConfigInstance> config() { - Map, ConfigInstance> ret = new HashMap<>(); - handles.forEach((k, v) -> ret.put(k, v.getConfig())); - return ret; - } - - @Override - public long waitNextGeneration(boolean isInitializing) { - if (handles.isEmpty()) - throw new IllegalStateException("No config keys registered"); - - // Catch and just log config exceptions due to missing config values for parameters that do - // not have a default value. These exceptions occur when the user has removed a component - // from services.xml, and the component takes a config that has parameters without a - // default value in the def-file. There is a new 'components' config underway, where the - // component is removed, so this old config generation will soon be replaced by a new one. - boolean gotNextGen = false; - int numExceptions = 0; - while ( ! gotNextGen) { - try { - if (subscriber.nextGeneration(isInitializing)) { - gotNextGen = true; - log.log(FINE, () -> this + " got next config generation " + subscriber.getGeneration() + "\n" + subscriber.toString()); - } - } - catch (IllegalArgumentException e) { - numExceptions++; - log.log(Level.WARNING, this + " got exception from the config system (ignore if you just removed a " + - "component from your application that used the mentioned config) Subscriber info: " + - subscriber.toString(), e); - if (numExceptions >= 5) - throw new IllegalArgumentException("Failed retrieving the next config generation", e); - } - } - - generation = subscriber.getGeneration(); - return generation; - } - - @Override - public void close() { - subscriber.close(); - } - + subscribers.forEach(s -> s.getSubscriber().reload(generation)); } public static class Provider implements com.google.inject.Provider { -- cgit v1.2.3 From be5fc1c7a88efc0aaad2f945acaa79a1f4206b54 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 8 Oct 2021 19:50:33 +0200 Subject: Simplify by taking a SubscriberFactory instead of a Function. --- .../main/java/com/yahoo/container/di/ConfigRetriever.java | 14 +++++++------- .../src/main/java/com/yahoo/container/di/Container.java | 2 +- .../java/com/yahoo/container/di/ConfigRetrieverTest.java | 5 ++--- 3 files changed, 10 insertions(+), 11 deletions(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java index 200b963bcad..88167c73198 100644 --- a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java +++ b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java @@ -5,6 +5,7 @@ import com.google.common.collect.Sets; import com.yahoo.config.ConfigInstance; import com.yahoo.container.di.componentgraph.core.Keys; import com.yahoo.container.di.config.Subscriber; +import com.yahoo.container.di.config.SubscriberFactory; import com.yahoo.vespa.config.ConfigKey; import java.util.Collections; @@ -31,18 +32,17 @@ public final class ConfigRetriever { private Set> componentSubscriberKeys; private final Subscriber bootstrapSubscriber; private Subscriber componentSubscriber; - private final Function>, Subscriber> subscribe; + private final SubscriberFactory subscriberFactory; - public ConfigRetriever(Set> bootstrapKeys, - Function>, Subscriber> subscribe) { + public ConfigRetriever(Set> bootstrapKeys, SubscriberFactory subscriberFactory) { this.bootstrapKeys = bootstrapKeys; this.componentSubscriberKeys = new HashSet<>(); - this.subscribe = subscribe; + this.subscriberFactory = subscriberFactory; if (bootstrapKeys.isEmpty()) { throw new IllegalArgumentException("Bootstrap key set is empty"); } - this.bootstrapSubscriber = subscribe.apply(bootstrapKeys); - this.componentSubscriber = subscribe.apply(componentSubscriberKeys); + this.bootstrapSubscriber = this.subscriberFactory.getSubscriber(bootstrapKeys); + this.componentSubscriber = this.subscriberFactory.getSubscriber(componentSubscriberKeys); } public ConfigSnapshot getConfigs(Set> componentConfigKeys, @@ -133,7 +133,7 @@ public final class ConfigRetriever { componentSubscriberKeys = keys; try { log.log(FINE, () -> "Setting up new component subscriber for keys: " + keys); - componentSubscriber = subscribe.apply(keys); + componentSubscriber = subscriberFactory.getSubscriber(keys); } catch (Throwable e) { log.log(Level.WARNING, "Failed setting up subscriptions for component configs: " + e.getMessage()); log.log(Level.WARNING, "Config keys: " + keys); diff --git a/container-core/src/main/java/com/yahoo/container/di/Container.java b/container-core/src/main/java/com/yahoo/container/di/Container.java index 626f97dd25c..aaab1a11712 100644 --- a/container-core/src/main/java/com/yahoo/container/di/Container.java +++ b/container-core/src/main/java/com/yahoo/container/di/Container.java @@ -62,7 +62,7 @@ public class Container { platformBundlesConfigKey = new ConfigKey<>(PlatformBundlesConfig.class, configId); componentsConfigKey = new ConfigKey<>(ComponentsConfig.class, configId); var bootstrapKeys = Set.of(applicationBundlesConfigKey, platformBundlesConfigKey, componentsConfigKey); - this.configurer = new ConfigRetriever(bootstrapKeys, subscriberFactory::getSubscriber); + this.configurer = new ConfigRetriever(bootstrapKeys, subscriberFactory); } public Container(SubscriberFactory subscriberFactory, String configId, ComponentDeconstructor componentDeconstructor) { diff --git a/container-core/src/test/java/com/yahoo/container/di/ConfigRetrieverTest.java b/container-core/src/test/java/com/yahoo/container/di/ConfigRetrieverTest.java index bba5675e458..99ea6a4ee58 100644 --- a/container-core/src/test/java/com/yahoo/container/di/ConfigRetrieverTest.java +++ b/container-core/src/test/java/com/yahoo/container/di/ConfigRetrieverTest.java @@ -19,7 +19,6 @@ import org.junit.rules.ExpectedException; import java.util.Collections; import java.util.HashSet; -import java.util.Optional; import java.util.Set; import static org.hamcrest.CoreMatchers.instanceOf; @@ -108,11 +107,11 @@ public class ConfigRetrieverTest { private ConfigRetriever createConfigRetriever() { String configId = dirConfigSource.configId(); - CloudSubscriberFactory subscriber = new CloudSubscriberFactory(dirConfigSource.configSource()); + CloudSubscriberFactory subscriberFactory = new CloudSubscriberFactory(dirConfigSource.configSource()); Set> keys = new HashSet<>(); keys.add(new ConfigKey<>(Bootstrap1Config.class, configId)); keys.add(new ConfigKey<>(Bootstrap2Config.class, configId)); - return new ConfigRetriever(keys, keySet -> subscriber.getSubscriber(keySet)); + return new ConfigRetriever(keys, subscriberFactory); } private void writeConfig(String name, String contents) { -- cgit v1.2.3 From 9f38bbb802e95bea4c646388b6ce0dc202314358 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 8 Oct 2021 20:17:42 +0200 Subject: Improve debugging of CloudSubscriber by adding a name. --- .../src/main/java/com/yahoo/container/di/CloudSubscriber.java | 9 ++++++++- .../java/com/yahoo/container/di/CloudSubscriberFactory.java | 11 ++--------- .../src/main/java/com/yahoo/container/di/ConfigRetriever.java | 7 ++++--- .../java/com/yahoo/container/di/config/SubscriberFactory.java | 2 +- .../java/com/yahoo/container/jdisc/ConfiguredApplication.java | 6 ++++-- .../container/standalone/StandaloneSubscriberFactory.java | 2 +- .../yahoo/container/standalone/StandaloneSubscriberTest.java | 2 +- 7 files changed, 21 insertions(+), 18 deletions(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java index 4efb36d4b29..9201ad2a528 100644 --- a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java +++ b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java @@ -24,13 +24,15 @@ import static java.util.logging.Level.FINE; public class CloudSubscriber implements Subscriber { private static final Logger log = Logger.getLogger(CloudSubscriber.class.getName()); + private final String name; private final ConfigSubscriber subscriber; private final Map, ConfigHandle> handles = new HashMap<>(); // if waitNextGeneration has not yet been called, -1 should be returned private long generation = -1L; - CloudSubscriber(Set> keys, ConfigSource configSource) { + CloudSubscriber(String name, ConfigSource configSource, Set> keys) { + this.name = name; this.subscriber = new ConfigSubscriber(configSource); keys.forEach(k -> handles.put(k, subscriber.subscribe(k.getConfigClass(), k.getConfigId()))); } @@ -91,6 +93,11 @@ public class CloudSubscriber implements Subscriber { subscriber.close(); } + @Override + public String toString() { + return "CloudSubscriber{" + name + ", gen." + generation + "}"; + } + // TODO: Remove, only used by test specific code in CloudSubscriberFactory ConfigSubscriber getSubscriber() { return subscriber; diff --git a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java index 38f7ded28f8..a6327b01e21 100644 --- a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java +++ b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriberFactory.java @@ -2,26 +2,19 @@ package com.yahoo.container.di; import com.yahoo.config.ConfigInstance; -import com.yahoo.config.subscription.ConfigHandle; import com.yahoo.config.subscription.ConfigSource; import com.yahoo.config.subscription.ConfigSourceSet; -import com.yahoo.config.subscription.ConfigSubscriber; import com.yahoo.container.di.config.Subscriber; import com.yahoo.container.di.config.SubscriberFactory; import com.yahoo.vespa.config.ConfigKey; import java.util.ArrayList; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.WeakHashMap; -import java.util.logging.Level; -import java.util.logging.Logger; - -import static java.util.logging.Level.FINE; /** * @author Tony Vaagenes @@ -39,14 +32,14 @@ public class CloudSubscriberFactory implements SubscriberFactory { } @Override - public Subscriber getSubscriber(Set> configKeys) { + public Subscriber getSubscriber(Set> configKeys, String name) { Set> subscriptionKeys = new HashSet<>(); for(ConfigKey key: configKeys) { @SuppressWarnings("unchecked") // ConfigKey is defined as ConfigKey invariant = (ConfigKey) key; subscriptionKeys.add(invariant); } - CloudSubscriber subscriber = new CloudSubscriber(subscriptionKeys, configSource); + CloudSubscriber subscriber = new CloudSubscriber(name, configSource, subscriptionKeys); testGeneration.ifPresent(subscriber.getSubscriber()::reload); // TODO: test specific code, remove activeSubscribers.put(subscriber, 0); diff --git a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java index 88167c73198..5554a5a804f 100644 --- a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java +++ b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java @@ -33,6 +33,7 @@ public final class ConfigRetriever { private final Subscriber bootstrapSubscriber; private Subscriber componentSubscriber; private final SubscriberFactory subscriberFactory; + private int componentSubscriberIndex; public ConfigRetriever(Set> bootstrapKeys, SubscriberFactory subscriberFactory) { this.bootstrapKeys = bootstrapKeys; @@ -41,8 +42,8 @@ public final class ConfigRetriever { if (bootstrapKeys.isEmpty()) { throw new IllegalArgumentException("Bootstrap key set is empty"); } - this.bootstrapSubscriber = this.subscriberFactory.getSubscriber(bootstrapKeys); - this.componentSubscriber = this.subscriberFactory.getSubscriber(componentSubscriberKeys); + this.bootstrapSubscriber = this.subscriberFactory.getSubscriber(bootstrapKeys, "bootstrap"); + this.componentSubscriber = this.subscriberFactory.getSubscriber(componentSubscriberKeys, "component_" + ++componentSubscriberIndex); } public ConfigSnapshot getConfigs(Set> componentConfigKeys, @@ -133,7 +134,7 @@ public final class ConfigRetriever { componentSubscriberKeys = keys; try { log.log(FINE, () -> "Setting up new component subscriber for keys: " + keys); - componentSubscriber = subscriberFactory.getSubscriber(keys); + componentSubscriber = subscriberFactory.getSubscriber(keys, "component_" + ++componentSubscriberIndex); } catch (Throwable e) { log.log(Level.WARNING, "Failed setting up subscriptions for component configs: " + e.getMessage()); log.log(Level.WARNING, "Config keys: " + keys); diff --git a/container-core/src/main/java/com/yahoo/container/di/config/SubscriberFactory.java b/container-core/src/main/java/com/yahoo/container/di/config/SubscriberFactory.java index 1bfd4344b90..f11f44413c7 100644 --- a/container-core/src/main/java/com/yahoo/container/di/config/SubscriberFactory.java +++ b/container-core/src/main/java/com/yahoo/container/di/config/SubscriberFactory.java @@ -14,7 +14,7 @@ import java.util.Set; @ProvidedBy(CloudSubscriberFactory.Provider.class) public interface SubscriberFactory { - Subscriber getSubscriber(Set> configKeys); + Subscriber getSubscriber(Set> configKeys, String name); void reloadActiveSubscribers(long generation); } 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 219616500c8..099df79cd9b 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 @@ -212,7 +212,8 @@ public final class ConfiguredApplication implements Application { } private T getConfig(Class configClass, boolean isInitializing) { - Subscriber subscriber = subscriberFactory.getSubscriber(Collections.singleton(new ConfigKey<>(configClass, configId))); + Subscriber subscriber = subscriberFactory.getSubscriber(Collections.singleton(new ConfigKey<>(configClass, configId)), + configClass.getName()); try { subscriber.waitNextGeneration(isInitializing); return configClass.cast(first(subscriber.config().values())); @@ -222,7 +223,8 @@ public final class ConfiguredApplication implements Application { } private void watchPortChange() { - Subscriber subscriber = subscriberFactory.getSubscriber(Collections.singleton(new ConfigKey<>(QrConfig.class, configId))); + Subscriber subscriber = subscriberFactory.getSubscriber(Collections.singleton(new ConfigKey<>(QrConfig.class, configId)), + "portWatcher"); try { while (true) { subscriber.waitNextGeneration(false); diff --git a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java index 8e8367cbe4c..d9db169ec54 100644 --- a/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java +++ b/standalone-container/src/main/java/com/yahoo/container/standalone/StandaloneSubscriberFactory.java @@ -83,7 +83,7 @@ public class StandaloneSubscriberFactory implements SubscriberFactory { @Override @SuppressWarnings("unchecked") - public Subscriber getSubscriber(Set> configKeys) { + public Subscriber getSubscriber(Set> configKeys, String name) { return new StandaloneSubscriber((Set>) configKeys); } diff --git a/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneSubscriberTest.java b/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneSubscriberTest.java index 76e3f9e6ec0..ecfc2caf722 100644 --- a/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneSubscriberTest.java +++ b/standalone-container/src/test/java/com/yahoo/container/standalone/StandaloneSubscriberTest.java @@ -39,7 +39,7 @@ public class StandaloneSubscriberTest { keys.add(platformBundlesKey); keys.add(applicationBundlesKey); keys.add(componentsKey); - Subscriber subscriber = new StandaloneSubscriberFactory(root).getSubscriber(keys); + Subscriber subscriber = new StandaloneSubscriberFactory(root).getSubscriber(keys, "standalone"); Map, ConfigInstance> config = subscriber.config(); assertEquals(2, config.size()); -- cgit v1.2.3 From 45a7091134149d0f85061a383dce8731372d731a Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 8 Oct 2021 20:40:52 +0200 Subject: minor: rearrange fields. --- .../src/main/java/com/yahoo/container/di/ConfigRetriever.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java index 5554a5a804f..e6ab0d87a63 100644 --- a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java +++ b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java @@ -30,9 +30,10 @@ public final class ConfigRetriever { private final Set> bootstrapKeys; private Set> componentSubscriberKeys; + + private final SubscriberFactory subscriberFactory; private final Subscriber bootstrapSubscriber; private Subscriber componentSubscriber; - private final SubscriberFactory subscriberFactory; private int componentSubscriberIndex; public ConfigRetriever(Set> bootstrapKeys, SubscriberFactory subscriberFactory) { -- cgit v1.2.3 From 5447f5db77ea2e01ccc3a4d5109bed9a123d8890 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Fri, 8 Oct 2021 21:06:36 +0200 Subject: Improve debug logging. --- .../src/main/java/com/yahoo/container/di/ConfigRetriever.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java index e6ab0d87a63..d7a258941cf 100644 --- a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java +++ b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java @@ -66,13 +66,13 @@ public final class ConfigRetriever { throw new IllegalArgumentException( "Component config keys [" + componentConfigKeys + "] overlaps with bootstrap config keys [" + bootstrapKeys + "]"); } - log.log(FINE, () -> "getConfigsOnce: " + componentConfigKeys); - Set> allKeys = new HashSet<>(componentConfigKeys); allKeys.addAll(bootstrapKeys); setupComponentSubscriber(allKeys); - return getConfigsOptional(leastGeneration, isInitializing); + var maybeSnapshot = getConfigsOptional(leastGeneration, isInitializing); + log.log(FINE, () -> "getConfigsOnce returning " + maybeSnapshot); + return maybeSnapshot; } private Optional getConfigsOptional(long leastGeneration, boolean isInitializing) { @@ -132,10 +132,11 @@ public final class ConfigRetriever { private void setupComponentSubscriber(Set> keys) { if (! componentSubscriberKeys.equals(keys)) { componentSubscriber.close(); + log.log(FINE, () -> "Closed " + componentSubscriber); componentSubscriberKeys = keys; try { - log.log(FINE, () -> "Setting up new component subscriber for keys: " + keys); componentSubscriber = subscriberFactory.getSubscriber(keys, "component_" + ++componentSubscriberIndex); + log.log(FINE, () -> "Set up new subscriber " + componentSubscriber + " for keys: " + keys); } catch (Throwable e) { log.log(Level.WARNING, "Failed setting up subscriptions for component configs: " + e.getMessage()); log.log(Level.WARNING, "Config keys: " + keys); -- cgit v1.2.3 From c50851c62aa2310d00594e302b6f7dbdf40ae92c Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 13 Oct 2021 13:12:34 +0200 Subject: Simplify and improve config retrieval. - Retrive bootstrap snapshot first when the system is in the stable state. When bootstrap is newer than components, retrieve the new components generation. This avoids getting exceptions from the config system when a component that takes a config with missing default value has been removed. - Do not close set up empty component subscriber after bootstrap, should be unnecessary as it's always done when component config keys are changed. - Declare getConfigsOnce private. - Improve debug logging --- .../com/yahoo/container/di/ConfigRetriever.java | 61 ++++++++++------------ 1 file changed, 27 insertions(+), 34 deletions(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java index d7a258941cf..ff238bd2497 100644 --- a/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java +++ b/container-core/src/main/java/com/yahoo/container/di/ConfigRetriever.java @@ -8,7 +8,6 @@ import com.yahoo.container.di.config.Subscriber; import com.yahoo.container.di.config.SubscriberFactory; import com.yahoo.vespa.config.ConfigKey; -import java.util.Collections; import java.util.HashSet; import java.util.Map; import java.util.Optional; @@ -53,15 +52,13 @@ public final class ConfigRetriever { while (true) { Optional maybeSnapshot = getConfigsOnce(componentConfigKeys, leastGeneration, isInitializing); if (maybeSnapshot.isPresent()) { - var configSnapshot = maybeSnapshot.get(); - resetComponentSubscriberIfBootstrap(configSnapshot); - return configSnapshot; + return maybeSnapshot.get(); } } } - Optional getConfigsOnce(Set> componentConfigKeys, - long leastGeneration, boolean isInitializing) { + private Optional getConfigsOnce(Set> componentConfigKeys, + long leastGeneration, boolean isInitializing) { if (!Sets.intersection(componentConfigKeys, bootstrapKeys).isEmpty()) { throw new IllegalArgumentException( "Component config keys [" + componentConfigKeys + "] overlaps with bootstrap config keys [" + bootstrapKeys + "]"); @@ -76,33 +73,35 @@ public final class ConfigRetriever { } private Optional getConfigsOptional(long leastGeneration, boolean isInitializing) { - long newestComponentGeneration = componentSubscriber.waitNextGeneration(isInitializing); - log.log(FINE, () -> "getConfigsOptional: new component generation: " + newestComponentGeneration); + if (componentSubscriber.generation() < bootstrapSubscriber.generation()) { + return getComponentsSnapshot(leastGeneration, isInitializing); + } + long newestBootstrapGeneration = bootstrapSubscriber.waitNextGeneration(isInitializing); + log.log(FINE, () -> "getConfigsOptional: new bootstrap generation: " + newestBootstrapGeneration); - // leastGeneration is only used to ensure newer generation (than the latest bootstrap or component gen) + // leastGeneration is used to ensure newer generation (than the latest bootstrap or component gen) // when the previous generation was invalidated due to an exception upon creating the component graph. + if (newestBootstrapGeneration < leastGeneration) { + return Optional.empty(); + } + return bootstrapConfigIfChanged(); + } + + private Optional getComponentsSnapshot(long leastGeneration, boolean isInitializing) { + long newestBootstrapGeneration = bootstrapSubscriber.generation(); + long newestComponentGeneration = componentSubscriber.waitNextGeneration(isInitializing); if (newestComponentGeneration < leastGeneration) { + log.log(FINE, () -> "Component generation too old: " + componentSubscriber.generation() + " < " + leastGeneration); return Optional.empty(); - } else if (bootstrapSubscriber.generation() < newestComponentGeneration) { - long newestBootstrapGeneration = bootstrapSubscriber.waitNextGeneration(isInitializing); - log.log(FINE, () -> "getConfigsOptional: new bootstrap generation: " + bootstrapSubscriber.generation()); - Optional bootstrapConfig = bootstrapConfigIfChanged(); - if (bootstrapConfig.isPresent()) { - return bootstrapConfig; - } else { - if (newestBootstrapGeneration == newestComponentGeneration) { - log.log(FINE, () -> this + " got new components configs with unchanged bootstrap configs."); - return componentsConfigIfChanged(); - } else { - // This should not be a normal case, and hence a warning to allow investigation. - log.warning("Did not get same generation for bootstrap (" + newestBootstrapGeneration + - ") and components configs (" + newestComponentGeneration + ")."); - return Optional.empty(); - } - } - } else { - // bootstrapGen==componentGen (happens only when a new component subscriber returns first config after bootstrap) + } + if (newestComponentGeneration == newestBootstrapGeneration) { + log.log(FINE, () -> "getConfigsOptional: new component generation: " + componentSubscriber.generation()); return componentsConfigIfChanged(); + } else { + // Should not be a normal case, and hence a warning to allow investigation. + log.warning("Did not get same generation for bootstrap (" + newestBootstrapGeneration + + ") and components configs (" + newestComponentGeneration + ")."); + return Optional.empty(); } } @@ -123,12 +122,6 @@ public final class ConfigRetriever { } } - private void resetComponentSubscriberIfBootstrap(ConfigSnapshot snapshot) { - if (snapshot instanceof BootstrapConfigs) { - setupComponentSubscriber(Collections.emptySet()); - } - } - private void setupComponentSubscriber(Set> keys) { if (! componentSubscriberKeys.equals(keys)) { componentSubscriber.close(); -- cgit v1.2.3 From 667757395ab4a48a8d9543eac1bcac49fcb87e54 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Wed, 13 Oct 2021 17:44:51 +0200 Subject: Allow exceptions from the config system to propagate up. --- .../src/main/java/com/yahoo/container/di/CloudSubscriber.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java index 9201ad2a528..f252287b403 100644 --- a/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java +++ b/container-core/src/main/java/com/yahoo/container/di/CloudSubscriber.java @@ -66,21 +66,14 @@ public class CloudSubscriber implements Subscriber { // default value in the def-file. There is a new 'components' config underway, where the // component is removed, so this old config generation will soon be replaced by a new one. boolean gotNextGen = false; - int numExceptions = 0; while ( ! gotNextGen) { try { if (subscriber.nextGeneration(isInitializing)) { gotNextGen = true; log.log(FINE, () -> this + " got next config generation " + subscriber.getGeneration() + "\n" + subscriber.toString()); } - } - catch (IllegalArgumentException e) { - numExceptions++; - log.log(Level.WARNING, this + " got exception from the config system (ignore if you just removed a " + - "component from your application that used the mentioned config) Subscriber info: " + - subscriber.toString(), e); - if (numExceptions >= 5) - throw new IllegalArgumentException("Failed retrieving the next config generation", e); + } catch (IllegalArgumentException e) { + throw new IllegalArgumentException("Failed retrieving the next config generation", e); } } -- cgit v1.2.3 From 2f43b8e5486f44e0e229b99b8d66217a55bd76d4 Mon Sep 17 00:00:00 2001 From: gjoranv Date: Thu, 14 Oct 2021 14:59:55 +0200 Subject: Rename config retriever field. --- .../src/main/java/com/yahoo/container/di/Container.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'container-core') diff --git a/container-core/src/main/java/com/yahoo/container/di/Container.java b/container-core/src/main/java/com/yahoo/container/di/Container.java index aaab1a11712..e437f440c41 100644 --- a/container-core/src/main/java/com/yahoo/container/di/Container.java +++ b/container-core/src/main/java/com/yahoo/container/di/Container.java @@ -48,7 +48,7 @@ public class Container { private final ComponentDeconstructor componentDeconstructor; private final Osgi osgi; - private final ConfigRetriever configurer; + private final ConfigRetriever retriever; private List platformBundles; // Used to verify that platform bundles don't change. private long previousConfigGeneration = -1L; private long leastGeneration = -1L; @@ -62,7 +62,7 @@ public class Container { platformBundlesConfigKey = new ConfigKey<>(PlatformBundlesConfig.class, configId); componentsConfigKey = new ConfigKey<>(ComponentsConfig.class, configId); var bootstrapKeys = Set.of(applicationBundlesConfigKey, platformBundlesConfigKey, componentsConfigKey); - this.configurer = new ConfigRetriever(bootstrapKeys, subscriberFactory); + this.retriever = new ConfigRetriever(bootstrapKeys, subscriberFactory); } public Container(SubscriberFactory subscriberFactory, String configId, ComponentDeconstructor componentDeconstructor) { @@ -91,7 +91,7 @@ public class Container { { ConfigSnapshot snapshot; while (true) { - snapshot = configurer.getConfigs(graph.configKeys(), leastGeneration, isInitializing); + snapshot = retriever.getConfigs(graph.configKeys(), leastGeneration, isInitializing); if (log.isLoggable(FINE)) log.log(FINE, String.format("getConfigAndCreateGraph:\n" + "graph.configKeys = %s\n" + "graph.generation = %s\n" + "snapshot = %s\n", @@ -127,11 +127,11 @@ public class Container { } private long getBootstrapGeneration() { - return configurer.getBootstrapGeneration(); + return retriever.getBootstrapGeneration(); } private long getComponentsGeneration() { - return configurer.getComponentsGeneration(); + return retriever.getComponentsGeneration(); } private String configGenerationsString() { @@ -215,7 +215,7 @@ public class Container { } private void invalidateGeneration(long generation, Throwable cause) { - leastGeneration = Math.max(configurer.getComponentsGeneration(), configurer.getBootstrapGeneration()) + 1; + leastGeneration = Math.max(retriever.getComponentsGeneration(), retriever.getBootstrapGeneration()) + 1; if (!(cause instanceof InterruptedException) && !(cause instanceof ConfigInterruptedException)) { log.log(Level.WARNING, newGraphErrorMessage(generation, cause), cause); } @@ -250,7 +250,7 @@ public class Container { } void shutdownConfigurer() { - configurer.shutdown(); + retriever.shutdown(); } // Reload config manually, when subscribing to non-configserver sources -- cgit v1.2.3 From f7674d2676732c6547c7354cc3431fe323722d4f Mon Sep 17 00:00:00 2001 From: gjoranv Date: Thu, 14 Oct 2021 15:29:36 +0200 Subject: Init the config generation to 1 instead of 0. - An initial value of 0, generated config generation sequence 1,1,2,3,... causing an exception in Container.getConfigAndCreateGraph when it got bootstrap configs with generation=1 twice. --- .../container/core/config/testutil/HandlersConfigurerTestWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'container-core') 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 973d5fb3736..99637e08b77 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 @@ -65,7 +65,7 @@ public class HandlersConfigurerTestWrapper { "query-profiles.cfg" }; private final Set createdFiles = new LinkedHashSet<>(); - private int lastGeneration = 0; + private int lastGeneration = 1; private final Container container; private void createFiles(String configId) { -- cgit v1.2.3