From 8bb44b23d954cb96819b475a8813843d37c6c418 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Fri, 3 Mar 2017 11:51:09 +0100 Subject: Refactorings * Make ConfigSourceClient and interface * Clean up consttructor and creation of test server --- .../vespa/config/proxy/ConfigSourceClient.java | 39 ++++--------- .../vespa/config/proxy/MapBackedConfigSource.java | 12 ++-- .../config/proxy/MemoryCacheConfigClient.java | 12 ++-- .../com/yahoo/vespa/config/proxy/ProxyServer.java | 66 +++++++++++++--------- .../vespa/config/proxy/RpcConfigSourceClient.java | 12 ++-- .../yahoo/vespa/config/proxy/ProxyServerTest.java | 7 --- 6 files changed, 67 insertions(+), 81 deletions(-) (limited to 'config-proxy') diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigSourceClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigSourceClient.java index f8b2cafa50f..b805640de87 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigSourceClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigSourceClient.java @@ -1,10 +1,7 @@ // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.proxy; -import com.yahoo.config.subscription.ConfigSource; -import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.vespa.config.RawConfig; -import com.yahoo.vespa.config.TimingValues; import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; import java.util.List; @@ -16,29 +13,15 @@ import java.util.List; * @author hmusum * @since 5.1.9 */ -public abstract class ConfigSourceClient { - static ConfigSourceClient createClient(ConfigSource source, - ClientUpdater clientUpdater, - MemoryCache memoryCache, - TimingValues timingValues, - DelayedResponses delayedResponses) { - if (source instanceof ConfigSourceSet) { - return new RpcConfigSourceClient((ConfigSourceSet) source, clientUpdater, memoryCache, timingValues, delayedResponses); - } else if (source instanceof MapBackedConfigSource) { - return (ConfigSourceClient) source; - } else { - throw new IllegalArgumentException("config source of type " + source.getClass().getName() + " is not allowed"); - } - } - - abstract RawConfig getConfig(RawConfig input, JRTServerConfigRequest request); - - abstract void cancel(); - - // TODO Should only be in rpc config source client - abstract void shutdownSourceConnections(); - - abstract String getActiveSourceConnection(); - - abstract List getSourceConnections(); +interface ConfigSourceClient { + + RawConfig getConfig(RawConfig input, JRTServerConfigRequest request); + + void cancel(); + + void shutdownSourceConnections(); + + String getActiveSourceConnection(); + + List getSourceConnections(); } \ No newline at end of file diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MapBackedConfigSource.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MapBackedConfigSource.java index 164b486334c..e8a76257b14 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MapBackedConfigSource.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MapBackedConfigSource.java @@ -17,7 +17,7 @@ import java.util.List; * @author hmusum * @since 5.1.10 */ -public class MapBackedConfigSource extends ConfigSourceClient implements ConfigSource { +public class MapBackedConfigSource implements ConfigSource, ConfigSourceClient { private final HashMap, RawConfig> backing = new HashMap<>(); private final ClientUpdater clientUpdater; @@ -32,7 +32,7 @@ public class MapBackedConfigSource extends ConfigSourceClient implements ConfigS } @Override - RawConfig getConfig(RawConfig input, JRTServerConfigRequest request) { + public RawConfig getConfig(RawConfig input, JRTServerConfigRequest request) { return getConfig(input.getKey()); } @@ -41,12 +41,12 @@ public class MapBackedConfigSource extends ConfigSourceClient implements ConfigS } @Override - void cancel() { + public void cancel() { clear(); } @Override - void shutdownSourceConnections() { + public void shutdownSourceConnections() { } public void clear() { @@ -54,12 +54,12 @@ public class MapBackedConfigSource extends ConfigSourceClient implements ConfigS } @Override - String getActiveSourceConnection() { + public String getActiveSourceConnection() { return "N/A"; } @Override - List getSourceConnections() { + public List getSourceConnections() { return Collections.singletonList("N/A"); } } diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClient.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClient.java index f1980b05817..6caed8a77e6 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClient.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/MemoryCacheConfigClient.java @@ -13,7 +13,7 @@ import java.util.logging.Logger; * @author hmusum * @since 5.1.10 */ -class MemoryCacheConfigClient extends ConfigSourceClient { +class MemoryCacheConfigClient implements ConfigSourceClient { private final static Logger log = Logger.getLogger(MemoryCacheConfigClient.class.getName()); private final MemoryCache cache; @@ -29,7 +29,7 @@ class MemoryCacheConfigClient extends ConfigSourceClient { * @return A Config with a payload. */ @Override - RawConfig getConfig(RawConfig input, JRTServerConfigRequest request) { + public RawConfig getConfig(RawConfig input, JRTServerConfigRequest request) { log.log(LogLevel.DEBUG, "Getting config from cache"); ConfigKey key = input.getKey(); RawConfig cached = cache.get(new ConfigCacheKey(key, input.getDefMd5())); @@ -42,20 +42,20 @@ class MemoryCacheConfigClient extends ConfigSourceClient { } @Override - void cancel() { + public void cancel() { } @Override - void shutdownSourceConnections() { + public void shutdownSourceConnections() { } @Override - String getActiveSourceConnection() { + public String getActiveSourceConnection() { return "N/A"; } @Override - List getSourceConnections() { + public List getSourceConnections() { return Collections.singletonList("N/A"); } diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java index a0ce5853768..c5df12f1e96 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ProxyServer.java @@ -24,7 +24,7 @@ import static com.yahoo.vespa.config.proxy.Mode.ModeName.DEFAULT; import static java.util.concurrent.TimeUnit.SECONDS; /** - * A proxy server that handles RPC config requests. The proxy can run in three modes: + * A proxy server that handles RPC config requests. The proxy can run in two modes: * 'default' and 'memorycache', where the last one will not get config from an upstream * config source, but will serve config only from memory cache. * @@ -47,7 +47,7 @@ public class ProxyServer implements Runnable { final DelayedResponses delayedResponses; private ConfigSource configSource; - private volatile ConfigSourceClient configSourceClient; + private volatile ConfigSourceClient configClient; private final ConfigProxyStatistics statistics; private final TimingValues timingValues; @@ -56,7 +56,7 @@ public class ProxyServer implements Runnable { private final static TimingValues defaultTimingValues; private final boolean delayedResponseHandling; - private volatile Mode mode; + private volatile Mode mode = new Mode(DEFAULT); static { // Proxy should time out before clients upon subscription. @@ -69,38 +69,35 @@ public class ProxyServer implements Runnable { } private ProxyServer(Spec spec, DelayedResponses delayedResponses, ConfigSource source, - ConfigProxyStatistics statistics, TimingValues timingValues, Mode mode, boolean delayedResponseHandling, - MemoryCache memoryCache) { + ConfigProxyStatistics statistics, TimingValues timingValues, + boolean delayedResponseHandling, MemoryCache memoryCache, + ConfigSourceClient configClient) { this.delayedResponses = delayedResponses; this.configSource = source; log.log(LogLevel.DEBUG, "Using config source '" + source); this.statistics = statistics; this.timingValues = timingValues; - this.mode = mode; this.delayedResponseHandling = delayedResponseHandling; this.memoryCache = memoryCache; - if (spec == null) { - rpcServer = null; - } else { - rpcServer = new ConfigProxyRpcServer(this, spec); - } + this.rpcServer = createRpcServer(spec); clientUpdater = new ClientUpdater(memoryCache, rpcServer, statistics, delayedResponses, mode); - this.configSourceClient = ConfigSourceClient.createClient(source, clientUpdater, memoryCache, timingValues, delayedResponses); + this.configClient = createClient(clientUpdater, delayedResponses, source, timingValues, memoryCache, configClient); } - private static ProxyServer create(int port, DelayedResponses delayedResponses, ConfigSource source, - ConfigProxyStatistics statistics, Mode mode) { - return new ProxyServer(new Spec(null, port), delayedResponses, source, statistics, defaultTimingValues(), mode, true, new MemoryCache()); + static ProxyServer createTestServer(ConfigSourceSet source) { + return createTestServer(source, null); } + static ProxyServer createTestServer(MapBackedConfigSource source) { + return createTestServer(source, source); + } - static ProxyServer createTestServer(ConfigSource source) { - final Mode mode = new Mode(DEFAULT); + private static ProxyServer createTestServer(ConfigSource source, ConfigSourceClient configSourceClient) { final ConfigProxyStatistics statistics = new ConfigProxyStatistics(); final boolean delayedResponseHandling = false; return new ProxyServer(null, new DelayedResponses(statistics), - source, statistics, defaultTimingValues(), mode, delayedResponseHandling, - new MemoryCache()); + source, statistics, defaultTimingValues(), delayedResponseHandling, + new MemoryCache(), configSourceClient); } public void run() { @@ -126,7 +123,7 @@ public class ProxyServer implements Runnable { // create a background thread that retrieves config from the server and // calls updateSubscribers when new config is returned from the config source. // In the last case the method below will return null. - RawConfig config = configSourceClient.getConfig(RawConfig.createFromServerRequest(req), req); + RawConfig config = configClient.getConfig(RawConfig.createFromServerRequest(req), req); if (configOrGenerationHasChanged(config, req)) { memoryCache.put(config); } @@ -148,18 +145,30 @@ public class ProxyServer implements Runnable { this.mode = new Mode(modeName); switch (mode.getMode()) { case MEMORYCACHE: - configSourceClient.shutdownSourceConnections(); - configSourceClient = new MemoryCacheConfigClient(memoryCache); + configClient.shutdownSourceConnections(); + configClient = new MemoryCacheConfigClient(memoryCache); break; case DEFAULT: flush(); - configSourceClient = createRpcClient(); + configClient = createRpcClient(); break; default: throw new IllegalArgumentException("Not able to handle mode '" + modeName + "'"); } } + private ConfigSourceClient createClient(ClientUpdater clientUpdater, DelayedResponses delayedResponses, + Object source, TimingValues timingValues, + MemoryCache memoryCache, ConfigSourceClient client) { + return (client == null) + ? new RpcConfigSourceClient((ConfigSourceSet) source, clientUpdater, memoryCache, timingValues, delayedResponses) + : client; + } + + private ConfigProxyRpcServer createRpcServer(Spec spec) { + return (spec == null) ? null : new ConfigProxyRpcServer(this, spec); // TODO: Try to avoid first argument being 'this' + } + private RpcConfigSourceClient createRpcClient() { return new RpcConfigSourceClient((ConfigSourceSet) configSource, clientUpdater, memoryCache, timingValues, delayedResponses); } @@ -202,7 +211,8 @@ public class ProxyServer implements Runnable { ConfigSourceSet configSources = new ConfigSourceSet(properties.configSources); DelayedResponses delayedResponses = new DelayedResponses(statistics); - ProxyServer proxyServer = ProxyServer.create(port, delayedResponses, configSources, statistics, new Mode(DEFAULT)); + ProxyServer proxyServer = new ProxyServer(new Spec(null, port), delayedResponses, configSources, statistics, + defaultTimingValues(), true, new MemoryCache(), null); // catch termination signal proxyServer.setupSigTermHandler(); Thread proxyserverThread = new Thread(proxyServer); @@ -244,7 +254,7 @@ public class ProxyServer implements Runnable { // the cache will not be updated again before someone calls getConfig(). synchronized void flush() { memoryCache.clear(); - configSourceClient.cancel(); + configClient.cancel(); } public void stop() { @@ -262,16 +272,16 @@ public class ProxyServer implements Runnable { } String getActiveSourceConnection() { - return configSourceClient.getActiveSourceConnection(); + return configClient.getActiveSourceConnection(); } List getSourceConnections() { - return configSourceClient.getSourceConnections(); + return configClient.getSourceConnections(); } public void updateSourceConnections(List sources) { configSource = new ConfigSourceSet(sources); flush(); - configSourceClient = createRpcClient(); + configClient = createRpcClient(); } } 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 942d06367b8..b0ad57e8a6c 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 @@ -25,7 +25,7 @@ import java.util.logging.Logger; * @author hmusum * @since 5.1.9 */ -class RpcConfigSourceClient extends ConfigSourceClient { +class RpcConfigSourceClient implements ConfigSourceClient { private final static Logger log = Logger.getLogger(RpcConfigSourceClient.class.getName()); private final Supervisor supervisor = new Supervisor(new Transport()); @@ -111,7 +111,7 @@ class RpcConfigSourceClient extends ConfigSourceClient { * @return A Config with a payload. */ @Override - RawConfig getConfig(RawConfig input, JRTServerConfigRequest request) { + public RawConfig getConfig(RawConfig input, JRTServerConfigRequest request) { long start = System.currentTimeMillis(); RawConfig ret = null; final ConfigCacheKey configCacheKey = new ConfigCacheKey(input.getKey(), input.getDefMd5()); @@ -166,7 +166,7 @@ class RpcConfigSourceClient extends ConfigSourceClient { } @Override - void cancel() { + public void cancel() { shutdownSourceConnections(); } @@ -174,7 +174,7 @@ class RpcConfigSourceClient extends ConfigSourceClient { * Takes down connection(s) to config sources and running tasks */ @Override - void shutdownSourceConnections() { + public void shutdownSourceConnections() { synchronized (activeSubscribersLock) { for (Subscriber subscriber : activeSubscribers.values()) { subscriber.cancel(); @@ -188,7 +188,7 @@ class RpcConfigSourceClient extends ConfigSourceClient { } @Override - String getActiveSourceConnection() { + public String getActiveSourceConnection() { if (requesterPool.get(configSourceSet) != null) { return requesterPool.get(configSourceSet).getConnectionPool().getCurrent().getAddress(); } else { @@ -197,7 +197,7 @@ class RpcConfigSourceClient extends ConfigSourceClient { } @Override - List getSourceConnections() { + public List getSourceConnections() { ArrayList ret = new ArrayList<>(); final JRTConfigRequester jrtConfigRequester = requesterPool.get(configSourceSet); if (jrtConfigRequester != null) { diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java index bdeedba7b78..ed6d2056733 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ProxyServerTest.java @@ -2,7 +2,6 @@ package com.yahoo.vespa.config.proxy; import com.yahoo.config.subscription.ConfigSourceSet; -import com.yahoo.config.subscription.RawSource; import com.yahoo.vespa.config.*; import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; import org.junit.After; @@ -168,12 +167,6 @@ public class ProxyServerTest { assertThat(properties.configSources[0], is(ProxyServer.DEFAULT_PROXY_CONFIG_SOURCES)); } - @Test(expected = IllegalArgumentException.class) - public void testIllegalConfigSource() { - RawSource source = new RawSource("bar 1"); - proxy = ProxyServer.createTestServer(source); - } - static RawConfig createConfigWithNextConfigGeneration(RawConfig config, int errorCode) { return new RawConfig(config.getKey(), config.getDefMd5(), config.getPayload(), config.getConfigMd5(), -- cgit v1.2.3