From b097a24f16f641441adab25faaad2581a7c938c7 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Tue, 3 Sep 2019 08:43:23 +0200 Subject: Simplify --- .../vespa/config/proxy/ConfigProxyRpcServer.java | 7 ++-- .../java/com/yahoo/vespa/config/proxy/Mode.java | 14 +------- .../com/yahoo/vespa/config/proxy/ProxyServer.java | 42 +++++++--------------- .../FileDistributionRpcServer.java | 2 +- .../config/proxy/ConfigProxyRpcServerTest.java | 16 +++++---- .../yahoo/vespa/config/proxy/ProxyServerTest.java | 13 +++++-- 6 files changed, 38 insertions(+), 56 deletions(-) diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java index e815fd9a5f9..9d33824e0be 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServer.java @@ -221,15 +221,14 @@ public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer private void setMode(Request req) { dispatchRpcRequest(req, () -> { String suppliedMode = req.parameters().get(0).asString(); - log.log(LogLevel.DEBUG, () -> "Supplied mode=" + suppliedMode); String[] s = new String[2]; - if (Mode.validModeName(suppliedMode.toLowerCase())) { + try { proxyServer.setMode(suppliedMode); s[0] = "0"; s[1] = "success"; - } else { + } catch (Exception e) { s[0] = "1"; - s[1] = "Could not set mode to '" + suppliedMode + "'. Legal modes are '" + Mode.modes() + "'"; + s[1] = e.getMessage(); } req.returnValues().add(new StringArray(s)); diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Mode.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Mode.java index fdc2b886701..f201811922e 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Mode.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/Mode.java @@ -21,10 +21,6 @@ class Mode { DEFAULT, MEMORYCACHE } - Mode() { - this(ModeName.DEFAULT); - } - Mode(ModeName modeName) { mode = modeName; } @@ -38,7 +34,7 @@ class Mode { mode = ModeName.MEMORYCACHE; break; default: - throw new IllegalArgumentException("Unrecognized mode '" + modeString + "' supplied"); + throw new IllegalArgumentException("Unrecognized mode '" + modeString + "' supplied. Legal modes are '" + Mode.modes() + "'"); } } @@ -50,18 +46,10 @@ class Mode { return mode.equals(ModeName.DEFAULT); } - boolean isMemoryCache() { - return mode.equals(ModeName.MEMORYCACHE); - } - boolean requiresConfigSource() { return mode.equals(ModeName.DEFAULT); } - static boolean validModeName(String modeString) { - return (modeString != null) && modes().contains(modeString); - } - static Set modes() { Set modes = new HashSet<>(); for (ModeName mode : ModeName.values()) { 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 f4f2308f261..cd515383950 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 @@ -56,7 +56,6 @@ public class ProxyServer implements Runnable { private final MemoryCache memoryCache; private static final double timingValuesRatio = 0.8; private final static TimingValues defaultTimingValues; - private final boolean delayedResponseHandling; private final FileDistributionAndUrlDownload fileDistributionAndUrlDownload; private volatile Mode mode = new Mode(DEFAULT); @@ -71,46 +70,29 @@ public class ProxyServer implements Runnable { defaultTimingValues = tv; } - private ProxyServer(Spec spec, ConfigSourceSet source, TimingValues timingValues, - boolean delayedResponseHandling, MemoryCache memoryCache, ConfigSourceClient configClient) { + ProxyServer(Spec spec, ConfigSourceSet source, TimingValues timingValues, + MemoryCache memoryCache, ConfigSourceClient configClient) { this.delayedResponses = new DelayedResponses(); this.configSource = source; log.log(LogLevel.DEBUG, "Using config source '" + source); this.timingValues = timingValues; - this.delayedResponseHandling = delayedResponseHandling; this.memoryCache = memoryCache; this.rpcServer = createRpcServer(spec); this.configClient = createClient(rpcServer, delayedResponses, source, timingValues, memoryCache, configClient); this.fileDistributionAndUrlDownload = new FileDistributionAndUrlDownload(supervisor, source); } - static ProxyServer createTestServer(ConfigSourceSet source) { - return createTestServer(source, null, new MemoryCache()); - } - - static ProxyServer createTestServer(ConfigSourceSet source, - ConfigSourceClient configSourceClient, - MemoryCache memoryCache) { - final boolean delayedResponseHandling = false; - return new ProxyServer(null, source, defaultTimingValues(), delayedResponseHandling, - memoryCache, configSourceClient); - } - public void run() { if (rpcServer != null) { Thread t = new Thread(rpcServer); t.setName("RpcServer"); t.start(); } - if (delayedResponseHandling) { - // Wait for 5 seconds initially, then run every second - delayedResponseScheduler = scheduler.scheduleAtFixedRate(new DelayedResponseHandler(delayedResponses, - memoryCache, - rpcServer), - 5, 1, SECONDS); - } else { - log.log(LogLevel.INFO, "Running without delayed response handling"); - } + // Wait for 5 seconds initially, then run every second + delayedResponseScheduler = scheduler.scheduleAtFixedRate(new DelayedResponseHandler(delayedResponses, + memoryCache, + rpcServer), + 5, 1, SECONDS); } RawConfig resolveConfig(JRTServerConfigRequest req) { @@ -132,20 +114,22 @@ public class ProxyServer implements Runnable { void setMode(String modeName) { if (modeName.equals(this.mode.name())) return; - log.log(LogLevel.INFO, "Switching from " + this.mode + " mode to " + modeName.toLowerCase() + " mode"); - this.mode = new Mode(modeName); + String oldMode = this.mode.name(); switch (mode.getMode()) { case MEMORYCACHE: configClient.shutdownSourceConnections(); configClient = new MemoryCacheConfigClient(memoryCache); + this.mode = new Mode(modeName); break; case DEFAULT: flush(); configClient = createRpcClient(); + this.mode = new Mode(modeName); break; default: - throw new IllegalArgumentException("Not able to handle mode '" + modeName + "'"); + throw new IllegalArgumentException("Cannot set invalid mode '" + modeName + "'"); } + log.log(LogLevel.INFO, "Switching from '" + oldMode + "' mode to '" + modeName.toLowerCase() + "' mode"); } private ConfigSourceClient createClient(RpcServer rpcServer, DelayedResponses delayedResponses, @@ -197,7 +181,7 @@ public class ProxyServer implements Runnable { ConfigSourceSet configSources = new ConfigSourceSet(properties.configSources); ProxyServer proxyServer = new ProxyServer(new Spec(null, port), configSources, - defaultTimingValues(), true, new MemoryCache(), null); + defaultTimingValues(), new MemoryCache(), null); // catch termination and interrupt signal proxyServer.setupSignalHandler(); Thread proxyserverThread = new Thread(proxyServer); diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionRpcServer.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionRpcServer.java index 0d72a1f02b6..c6f33a29410 100644 --- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionRpcServer.java +++ b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/filedistribution/FileDistributionRpcServer.java @@ -75,7 +75,7 @@ class FileDistributionRpcServer { //---------------- RPC methods ------------------------------------ - // TODO: Duplicate of code in FileAcquirereImpl. Find out where to put it. What about C++ code using this RPC call? + // TODO: Duplicate of code in FileAcquirerImpl. Find out where to put it. What about C++ code using this RPC call? private static final int baseErrorCode = 0x10000; private static final int baseFileProviderErrorCode = baseErrorCode + 0x1000; diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java index b810c6c7e4c..ffa70b5bfb5 100644 --- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java +++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/ConfigProxyRpcServerTest.java @@ -28,7 +28,7 @@ import static org.junit.Assert.assertThat; public class ConfigProxyRpcServerTest { private static final String hostname = "localhost"; private static final int port = 12345; - private static final String address = "tcp/" + hostname + ":" + port; + private static final String configSourceAddress = "tcp/" + hostname + ":" + port; private TestServer server; private TestClient client; @@ -46,7 +46,7 @@ public class ConfigProxyRpcServerTest { @Test public void basic() { - ProxyServer proxy = ProxyServer.createTestServer(new MockConfigSource()); + ProxyServer proxy = createTestServer(new MockConfigSource()); Spec spec = new Spec("localhost", 12345); ConfigProxyRpcServer server = new ConfigProxyRpcServer(proxy, new Supervisor(new Transport()), spec); assertThat(server.getSpec(), is(spec)); @@ -131,8 +131,8 @@ public class ConfigProxyRpcServerTest { assertThat(req.returnValues().size(), is(1)); final String[] ret = req.returnValues().get(0).asStringArray(); assertThat(ret.length, is(2)); - assertThat(ret[0], is("Current source: " + address)); - assertThat(ret[1], is("All sources:\n" + address + "\n")); + assertThat(ret[0], is("Current source: " + configSourceAddress)); + assertThat(ret[1], is("All sources:\n" + configSourceAddress + "\n")); } /** @@ -204,7 +204,7 @@ public class ConfigProxyRpcServerTest { ret = req.returnValues().get(0).asStringArray(); assertThat(ret.length, is(2)); assertThat(ret[0], is("1")); - assertThat(ret[1], is("Could not set mode to '" + mode + "'. Legal modes are '" + Mode.modes() + "'")); + assertThat(ret[1], is("Unrecognized mode '" + mode + "' supplied. Legal modes are '" + Mode.modes() + "'")); assertThat(server.proxyServer().getMode().name(), is(oldMode)); } @@ -257,11 +257,15 @@ public class ConfigProxyRpcServerTest { assertThat(req.returnValues().get(0).asString(), is("success")); } + private static ProxyServer createTestServer(ConfigSourceSet source) { + return new ProxyServer(null, source, ProxyServer.defaultTimingValues(), new MemoryCache(), null); + } + private static class TestServer implements AutoCloseable { private static final Spec SPEC = new Spec(0); - private final ProxyServer proxyServer = ProxyServer.createTestServer(new ConfigSourceSet(address)); + private final ProxyServer proxyServer = createTestServer(new ConfigSourceSet(configSourceAddress)); private final Supervisor supervisor = new Supervisor(new Transport()); private final ConfigProxyRpcServer rpcServer = new ConfigProxyRpcServer(proxyServer, supervisor, SPEC); private final Acceptor acceptor; 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 1c64631d205..712567774f1 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 @@ -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.proxy; +import com.yahoo.config.subscription.ConfigSourceSet; import com.yahoo.vespa.config.*; import com.yahoo.vespa.config.protocol.JRTServerConfigRequest; import com.yahoo.vespa.config.protocol.Payload; @@ -41,7 +42,7 @@ public class ProxyServerTest { source.clear(); source.put(fooConfig.getKey(), createConfigWithNextConfigGeneration(fooConfig, 0)); source.put(errorConfigKey, createConfigWithNextConfigGeneration(fooConfig, ErrorCode.UNKNOWN_DEFINITION)); - proxy = ProxyServer.createTestServer(source, client, memoryCache); + proxy = createTestServer(source, client, memoryCache); } @After @@ -70,7 +71,7 @@ public class ProxyServerTest { */ @Test public void testModeSwitch() { - ProxyServer proxy = ProxyServer.createTestServer(source); + ProxyServer proxy = createTestServer(source, client, new MemoryCache()); assertTrue(proxy.getMode().isDefault()); for (String mode : Mode.modes()) { @@ -83,7 +84,7 @@ public class ProxyServerTest { proxy.setMode("invalid"); assert (false); } catch (IllegalArgumentException e) { - assertEquals("Unrecognized mode 'invalid' supplied", e.getMessage()); + assertEquals("Unrecognized mode 'invalid' supplied. Legal modes are '" + Mode.modes() + "'", e.getMessage()); } // Also switch to DEFAULT mode, as that is not covered above @@ -218,6 +219,12 @@ public class ProxyServerTest { assertThat(properties.configSources[0], is(ProxyServer.DEFAULT_PROXY_CONFIG_SOURCES)); } + private static ProxyServer createTestServer(ConfigSourceSet source, + ConfigSourceClient configSourceClient, + MemoryCache memoryCache) { + return new ProxyServer(null, source, ProxyServer.defaultTimingValues(), memoryCache, configSourceClient); + } + static RawConfig createConfigWithNextConfigGeneration(RawConfig config, int errorCode) { return createConfigWithNextConfigGeneration(config, errorCode, config.getPayload()); } -- cgit v1.2.3