diff options
author | Harald Musum <musum@verizonmedia.com> | 2020-02-10 19:49:18 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-02-10 19:49:18 +0100 |
commit | 10856151d8c1ecb1aa23591b67a2c6a3afd966f3 (patch) | |
tree | 3b4af4abbc9c9931e841325cc233494abbdf7ddd | |
parent | 97ace087461254d5cc5a710d6b891176297ecfca (diff) |
Revert "Config cleanup 5"
10 files changed, 122 insertions, 92 deletions
diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java index 9c0f104f7de..91bb669f396 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCache.java @@ -124,8 +124,8 @@ public class MemoryCache { // First three lines are meta-data about config as comment lines, fourth line is empty writer.write("# defMd5:" + config.getDefMd5() + "\n"); writer.write("# configMd5:" + config.getConfigMd5() + "\n"); - writer.write("# generation:" + config.getGeneration() + "\n"); - writer.write("# protocolVersion:" + protocolVersion + "\n"); + writer.write("# generation:" + Long.toString(config.getGeneration()) + "\n"); + writer.write("# protocolVersion:" + Long.toString(protocolVersion) + "\n"); writer.write("\n"); writer.write(payload.withCompression(CompressionType.UNCOMPRESSED).toString()); writer.write("\n"); diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java index 865bb2472c2..d809a3c97ed 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClient.java @@ -11,15 +11,13 @@ import com.yahoo.jrt.Supervisor; import com.yahoo.jrt.Target; import com.yahoo.jrt.Transport; import com.yahoo.log.LogLevel; -import com.yahoo.vespa.config.ConfigCacheKey; -import com.yahoo.vespa.config.JRTConnectionPool; -import com.yahoo.vespa.config.RawConfig; -import com.yahoo.vespa.config.TimingValues; +import com.yahoo.vespa.config.*; import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; import java.util.ArrayList; import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.DelayQueue; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -44,7 +42,8 @@ class RpcConfigSourceClient implements ConfigSourceClient { private final TimingValues timingValues; private final ExecutorService exec; - private final JRTConfigRequester requester; + private final Map<ConfigSourceSet, JRTConfigRequester> requesterPool; + RpcConfigSourceClient(RpcServer rpcServer, ConfigSourceSet configSourceSet, @@ -58,7 +57,20 @@ class RpcConfigSourceClient implements ConfigSourceClient { this.timingValues = timingValues; checkConfigSources(); exec = Executors.newCachedThreadPool(new DaemonThreadFactory("subscriber-")); - requester = new JRTConfigRequester(new JRTConnectionPool(configSourceSet), timingValues); + requesterPool = createRequesterPool(configSourceSet, timingValues); + } + + /** + * Creates a requester (connection) pool of one entry, to be used each time this {@link RpcConfigSourceClient} is used + * @param ccs a {@link ConfigSourceSet} + * @param timingValues a {@link TimingValues} + * @return requester map + */ + private Map<ConfigSourceSet, JRTConfigRequester> createRequesterPool(ConfigSourceSet ccs, TimingValues timingValues) { + Map<ConfigSourceSet, JRTConfigRequester> ret = new HashMap<>(); + if (ccs.getSources().isEmpty()) return ret; // unit test, just skip creating any requester + ret.put(ccs, JRTConfigRequester.get(new JRTConnectionPool(ccs), timingValues)); + return ret; } /** @@ -141,7 +153,7 @@ class RpcConfigSourceClient implements ConfigSourceClient { } else { log.log(LogLevel.DEBUG, () -> "Could not find good config in cache, creating subscriber for: " + configCacheKey); UpstreamConfigSubscriber subscriber = new UpstreamConfigSubscriber(input, this, configSourceSet, - timingValues, requester, memoryCache); + timingValues, requesterPool, memoryCache); try { subscriber.subscribe(); activeSubscribers.put(configCacheKey, subscriber); @@ -171,17 +183,28 @@ class RpcConfigSourceClient implements ConfigSourceClient { activeSubscribers.clear(); } exec.shutdown(); - requester.close(); + for (JRTConfigRequester requester : requesterPool.values()) { + requester.close(); + } } @Override public String getActiveSourceConnection() { - return requester.getConnectionPool().getCurrent().getAddress(); + if (requesterPool.get(configSourceSet) != null) { + return requesterPool.get(configSourceSet).getConnectionPool().getCurrent().getAddress(); + } else { + return ""; + } } @Override public List<String> getSourceConnections() { - return new ArrayList<>(configSourceSet.getSources()); + ArrayList<String> ret = new ArrayList<>(); + final JRTConfigRequester jrtConfigRequester = requesterPool.get(configSourceSet); + if (jrtConfigRequester != null) { + ret.addAll(configSourceSet.getSources()); + } + return ret; } /** diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java index ba440da685a..f8df16cb3d2 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/UpstreamConfigSubscriber.java @@ -2,15 +2,17 @@ package com.yahoo.vespa.config.proxy; import com.yahoo.config.subscription.ConfigSource; +import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.config.subscription.impl.GenericConfigHandle; import com.yahoo.config.subscription.impl.GenericConfigSubscriber; import com.yahoo.config.subscription.impl.JRTConfigRequester; import com.yahoo.log.LogLevel; import com.yahoo.vespa.config.ConfigKey; +import com.yahoo.yolean.Exceptions; import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; -import com.yahoo.yolean.Exceptions; +import java.util.Map; import java.util.logging.Logger; /** @@ -24,24 +26,24 @@ public class UpstreamConfigSubscriber implements Subscriber { private final ConfigSourceClient configSourceClient; private final ConfigSource configSourceSet; private final TimingValues timingValues; - private final JRTConfigRequester requester; + private final Map<ConfigSourceSet, JRTConfigRequester> requesterPool; private final MemoryCache memoryCache; private GenericConfigSubscriber subscriber; private GenericConfigHandle handle; UpstreamConfigSubscriber(RawConfig config, ConfigSourceClient configSourceClient, ConfigSource configSourceSet, - TimingValues timingValues, JRTConfigRequester requester, + TimingValues timingValues, Map<ConfigSourceSet, JRTConfigRequester> requesterPool, MemoryCache memoryCache) { this.config = config; this.configSourceClient = configSourceClient; this.configSourceSet = configSourceSet; this.timingValues = timingValues; - this.requester = requester; + this.requesterPool = requesterPool; this.memoryCache = memoryCache; } void subscribe() { - subscriber = new GenericConfigSubscriber(requester); + subscriber = new GenericConfigSubscriber(requesterPool); ConfigKey<?> key = config.getKey(); handle = subscriber.subscribe(new ConfigKey<>(key.getName(), key.getConfigId(), key.getNamespace()), config.getDefContent(), configSourceSet, timingValues); diff --git a/config/abi-spec.json b/config/abi-spec.json index cf0ea291138..b60f9053642 100644 --- a/config/abi-spec.json +++ b/config/abi-spec.json @@ -209,15 +209,12 @@ "protected void throwIfExceptionSet(com.yahoo.config.subscription.impl.ConfigSubscription)", "public void close()", "protected void closeRequesters()", - "protected void closeRequester()", "public java.lang.String toString()", "public java.lang.Thread startConfigThread(java.lang.Runnable)", "protected com.yahoo.config.subscription.ConfigSubscriber$State state()", "public void reload(long)", "public com.yahoo.config.subscription.ConfigSource getSource()", "public java.util.Map requesters()", - "public com.yahoo.config.subscription.impl.JRTConfigRequester requester()", - "public void requester(com.yahoo.config.subscription.impl.JRTConfigRequester)", "public boolean isClosed()", "public com.yahoo.config.subscription.ConfigHandle subscribe(com.yahoo.config.subscription.ConfigSubscriber$SingleSubscriber, java.lang.Class, java.lang.String)", "public long getGeneration()", @@ -226,8 +223,7 @@ ], "fields": [ "protected final java.util.List subscriptionHandles", - "protected java.util.Map requesters", - "protected com.yahoo.config.subscription.impl.JRTConfigRequester requester" + "protected java.util.Map requesters" ] }, "com.yahoo.config.subscription.ConfigURI": { 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 be039c335d8..5c0b932dcce 100644 --- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java +++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java @@ -10,6 +10,7 @@ import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.TimingValues; import com.yahoo.yolean.Exceptions; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.CopyOnWriteArrayList; @@ -18,8 +19,8 @@ import java.util.logging.Logger; import static java.util.stream.Collectors.toList; /** - * Used for subscribing to one or more configs using the default config source. Can optionally be given a - * {@link ConfigSource} for the configs that will be used when {@link #subscribe(Class, String)} is called. + * Used for subscribing to one or more configs. Can optionally be given a {@link ConfigSource} for the configs + * that will be used when {@link #subscribe(Class, String)} is called. * * {@link #subscribe(Class, String)} on the configs needed, call {@link #nextConfig(long)} and get the config from the * {@link ConfigHandle} which {@link #subscribe(Class, String)} returned. @@ -42,15 +43,9 @@ public class ConfigSubscriber implements AutoCloseable { private boolean internalRedeploy = false; /** - * Reuse requester, limit number if many subscriptions. - * - * @deprecated use {@link #requester} + * Reuse requesters for equal source sets, limit number if many subscriptions. */ - @Deprecated - // TODO: Remove in Vespa 8 - protected Map<ConfigSourceSet, JRTConfigRequester> requesters = Map.of(); - - protected JRTConfigRequester requester = null; + protected Map<ConfigSourceSet, JRTConfigRequester> requesters = new HashMap<>(); /** * The states of the subscriber. Affects the validity of calling certain methods. @@ -329,28 +324,17 @@ public class ConfigSubscriber implements AutoCloseable { for (ConfigHandle<? extends ConfigInstance> h : subscriptionHandles) { h.subscription().close(); } - closeRequester(); + closeRequesters(); log.log(LogLevel.DEBUG, "Config subscriber has been closed."); } - /** - * Closes requester - * @deprecated use {@link #closeRequester()} instead - * + * Closes all open requesters */ - @Deprecated - // TODO: Remove in Vespa 8 protected void closeRequesters() { - closeRequester(); - } - - /** - * Closes requester - */ - protected void closeRequester() { - if (requester != null) + for (JRTConfigRequester requester : requesters.values()) { requester.close(); + } } @Override @@ -410,24 +394,9 @@ public class ConfigSubscriber implements AutoCloseable { /** * Implementation detail, do not use. * @return requesters - * @deprecated use {@link #requester()} */ - @Deprecated - // TODO: Remove in Vespa 8 public Map<ConfigSourceSet, JRTConfigRequester> requesters() { - return Map.of((ConfigSourceSet)source, requester); - } - - /** - * Implementation detail, do not use. - * @return requesters - */ - public JRTConfigRequester requester() { - return requester; - } - - public void requester(JRTConfigRequester requester) { - this.requester = requester; + return requesters; } public boolean isClosed() { diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java index 457c15eda3d..324546230d9 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java @@ -2,33 +2,42 @@ package com.yahoo.config.subscription.impl; import java.util.List; +import java.util.Map; 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.vespa.config.ConfigKey; import com.yahoo.vespa.config.RawConfig; import com.yahoo.vespa.config.TimingValues; /** - * A subscriber that can subscribe without supplying a config class. Used by config proxy. + * A subscriber that can subscribe without the class. Used by configproxy. * * @author Vegard Havdal */ public class GenericConfigSubscriber extends ConfigSubscriber { /** - * Constructs a new subscriber using the given requester + * Constructs a new subscriber using the given pool of requesters (JRTConfigRequester holds 1 connection which in + * turn is subject to failover across the elems in the source set.) + * The behaviour is undefined if the map key is different from the source set the requester was built with. + * See also {@link JRTConfigRequester#get(com.yahoo.vespa.config.ConnectionPool, com.yahoo.vespa.config.TimingValues)} * - * @param requester a config requester + * @param requesters a map from config source set to config requester */ - public GenericConfigSubscriber(JRTConfigRequester requester) { - this.requester = requester; + public GenericConfigSubscriber(Map<ConfigSourceSet, JRTConfigRequester> requesters) { + this.requesters = requesters; + } + + public GenericConfigSubscriber() { + super(); } /** - * Subscribes to config without using a config class. For internal use in config proxy. + * Subscribes to config without using the class. For internal use in config proxy. * * @param key the {@link ConfigKey to subscribe to} * @param defContent the config definition content for the config to subscribe to @@ -59,4 +68,10 @@ public class GenericConfigSubscriber extends ConfigSubscriber { throw new UnsupportedOperationException(); } + /** + * Do nothing, since we share requesters + */ + public void closeRequesters() { + } + } diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java index 50f4eeae1b8..e9daafb779c 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java @@ -52,10 +52,20 @@ public class JRTConfigRequester implements RequestWaiter { /** * Returns a new requester - * @param connectionPool the connectionPool this requester should use + * @param connectionPool The connectionPool to use + * @param timingValues The timing values + * @return new requester object + */ + public static JRTConfigRequester get(ConnectionPool connectionPool, TimingValues timingValues) { + return new JRTConfigRequester(connectionPool, timingValues); + } + + /** + * New requester + * @param connectionPool the connectionPool this requester should use * @param timingValues timeouts and delays used when sending JRT config requests */ - public JRTConfigRequester(ConnectionPool connectionPool, TimingValues timingValues) { + JRTConfigRequester(ConnectionPool connectionPool, TimingValues timingValues) { this.connectionPool = connectionPool; this.timingValues = timingValues; } diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java index 58a6c5f9f0c..39e6c69f539 100644 --- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java +++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java @@ -154,10 +154,10 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc } private JRTConfigRequester getRequester() { - JRTConfigRequester requester = subscriber.requester(); + JRTConfigRequester requester = subscriber.requesters().get(sources); if (requester == null) { requester = new JRTConfigRequester(new JRTConnectionPool(sources), timingValues); - subscriber.requester(requester); + subscriber.requesters().put(sources, requester); } return requester; } diff --git a/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java b/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java index 7a10db49408..08d215670db 100644 --- a/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java +++ b/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java @@ -1,21 +1,20 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.config.subscription; +import java.util.*; + import com.yahoo.config.subscription.impl.GenericConfigHandle; import com.yahoo.config.subscription.impl.GenericConfigSubscriber; import com.yahoo.config.subscription.impl.JRTConfigRequester; import com.yahoo.config.subscription.impl.JRTConfigRequesterTest; import com.yahoo.config.subscription.impl.MockConnection; import com.yahoo.vespa.config.ConfigKey; +import com.yahoo.vespa.config.JRTConnectionPool; import com.yahoo.vespa.config.protocol.CompressionType; import org.junit.Test; -import java.util.List; - import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; /** * @@ -27,33 +26,48 @@ public class GenericConfigSubscriberTest { @Test public void testSubscribeGeneric() { + Map<ConfigSourceSet, JRTConfigRequester> requesters = new HashMap<>(); ConfigSourceSet sourceSet = new ConfigSourceSet("blabla"); - GenericConfigSubscriber subscriber = createSubscriber(); - final List<String> defContent = List.of("myVal int"); - GenericConfigHandle handle = subscriber.subscribe(new ConfigKey<>("simpletypes", "id", "config"), defContent, sourceSet, JRTConfigRequesterTest.getTestTimingValues()); - assertTrue(subscriber.nextConfig()); + requesters.put(sourceSet, JRTConfigRequester.get(new MockConnection(), JRTConfigRequesterTest.getTestTimingValues())); + GenericConfigSubscriber sub = new GenericConfigSubscriber(requesters); + final List<String> defContent = Arrays.asList("myVal int"); + GenericConfigHandle handle = sub.subscribe(new ConfigKey<>("simpletypes", "id", "config"), defContent, sourceSet, JRTConfigRequesterTest.getTestTimingValues()); + assertTrue(sub.nextConfig()); assertTrue(handle.isChanged()); assertThat(handle.getRawConfig().getPayload().withCompression(CompressionType.UNCOMPRESSED).toString(), is("{}")); // MockConnection returns empty string - assertFalse(subscriber.nextConfig()); + assertFalse(sub.nextConfig()); assertFalse(handle.isChanged()); } + @Test + public void testGenericRequesterPooling() { + ConfigSourceSet source1 = new ConfigSourceSet("tcp/foo:78"); + ConfigSourceSet source2 = new ConfigSourceSet("tcp/bar:79"); + JRTConfigRequester req1 = JRTConfigRequester.get(new JRTConnectionPool(source1), JRTConfigRequesterTest.getTestTimingValues()); + JRTConfigRequester req2 = JRTConfigRequester.get(new JRTConnectionPool(source2), JRTConfigRequesterTest.getTestTimingValues()); + Map<ConfigSourceSet, JRTConfigRequester> requesters = new LinkedHashMap<>(); + requesters.put(source1, req1); + requesters.put(source2, req2); + GenericConfigSubscriber sub = new GenericConfigSubscriber(requesters); + assertEquals(sub.requesters().get(source1).getConnectionPool().getCurrent().getAddress(), "tcp/foo:78"); + assertEquals(sub.requesters().get(source2).getConnectionPool().getCurrent().getAddress(), "tcp/bar:79"); + } + @Test(expected=UnsupportedOperationException.class) public void testOverriddenSubscribeInvalid1() { - createSubscriber().subscribe(null, null); + GenericConfigSubscriber sub = new GenericConfigSubscriber(); + sub.subscribe(null, null); } @Test(expected=UnsupportedOperationException.class) public void testOverriddenSubscribeInvalid2() { - createSubscriber().subscribe(null, null, 0L); + GenericConfigSubscriber sub = new GenericConfigSubscriber(); + sub.subscribe(null, null, 0L); } @Test(expected=UnsupportedOperationException.class) public void testOverriddenSubscribeInvalid3() { - createSubscriber().subscribe(null, null, ""); - } - - private GenericConfigSubscriber createSubscriber() { - return new GenericConfigSubscriber(new JRTConfigRequester(new MockConnection(), JRTConfigRequesterTest.getTestTimingValues())); + GenericConfigSubscriber sub = new GenericConfigSubscriber(); + sub.subscribe(null, null, ""); } } diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java index a9fbdde3dfa..5fcbd76b822 100644 --- a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java +++ b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java @@ -1,6 +1,7 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.protocol; +import com.yahoo.foo.SimpletypesConfig; import com.yahoo.config.subscription.ConfigSet; import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.config.subscription.ConfigSubscriber; @@ -8,7 +9,6 @@ import com.yahoo.config.subscription.impl.GenericConfigSubscriber; import com.yahoo.config.subscription.impl.JRTConfigRequester; import com.yahoo.config.subscription.impl.JRTConfigSubscription; import com.yahoo.config.subscription.impl.MockConnection; -import com.yahoo.foo.SimpletypesConfig; import com.yahoo.jrt.Request; import com.yahoo.slime.Inspector; import com.yahoo.slime.JsonDecoder; @@ -24,6 +24,7 @@ import com.yahoo.vespa.config.util.ConfigUtils; import org.junit.Before; import org.junit.Test; +import java.util.Collections; import java.util.Optional; import static org.hamcrest.CoreMatchers.is; @@ -239,7 +240,7 @@ public abstract class JRTConfigRequestBase { }); ConfigSourceSet src = new ConfigSourceSet(); - ConfigSubscriber subscriber = new GenericConfigSubscriber(new JRTConfigRequester(connection, new TimingValues())); + ConfigSubscriber subscriber = new GenericConfigSubscriber(Collections.singletonMap(src, JRTConfigRequester.get(connection, new TimingValues()))); JRTConfigSubscription<SimpletypesConfig> sub = new JRTConfigSubscription<>(new ConfigKey<>(SimpletypesConfig.class, configId), subscriber, src, new TimingValues()); sub.subscribe(120_0000); assertTrue(sub.nextConfig(120_0000)); |