Merge branch 'master' into hmusum/config-proxy-refactoring-2

6 files changed, 15 insertions, 173 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 5ffc7293742..e815fd9a5f9 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
@@ -133,7 +133,6 @@ public class ConfigProxyRpcServer implements Runnable, TargetWatcher, RpcServer
         dispatchRpcRequest(req, () -> {
             JRTServerConfigRequest request = JRTServerConfigRequestV3.createFromRequest(req);
             if (isProtocolVersionSupported(request)) {
-                proxyServer.getStatistics().incRpcRequests();
                 req.target().addWatcher(this);
                 getConfigImpl(request);
                 return;
diff --git a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java b/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java
deleted file mode 100644
index 314a4b0cb11..00000000000
--- a/config-proxy/src/main/java/com/yahoo/vespa/config/proxy/ConfigProxyStatistics.java
+++ /dev/null
@@ -1,104 +0,0 @@
-// 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.log.LogLevel;
-import com.yahoo.log.event.Event;
-
-/**
- * Statistics/metrics for config proxy.
- * //TODO Use metrics framework
- *
- * @author hmusum
- */
-class ConfigProxyStatistics implements Runnable {
-    static final long defaultEventInterval = 5 * 60; // in seconds
-
-    private final long eventInterval; // in seconds
-    private boolean stopped;
-    private long lastRun = System.currentTimeMillis();
-
-    /* Number of RPC getConfig requests */
-    private long rpcRequests = 0;
-    private long processedRequests = 0;
-    private long errors = 0;
-    private long delayedResponses = 0;
-
-    ConfigProxyStatistics() {
-        this(defaultEventInterval);
-    }
-
-    ConfigProxyStatistics(long eventInterval) {
-        this.eventInterval = eventInterval;
-    }
-
-    // Send events every eventInterval seconds
-    public void run() {
-        while (true) {
-            try {
-                Thread.sleep(1000);
-            } catch (InterruptedException e) {
-                ProxyServer.log.log(LogLevel.WARNING, e.getMessage());
-            }
-            if (stopped) {
-                return;
-            }
-            ProxyServer.log.log(LogLevel.SPAM, "Running ConfigProxyStatistics");
-            // Only send events every eventInterval seconds
-            if ((System.currentTimeMillis() - lastRun) > eventInterval * 1000) {
-                lastRun = System.currentTimeMillis();
-                sendEvents();
-            }
-        }
-    }
-
-    private void sendEvents() {
-        Event.count("rpc_requests", rpcRequests());
-        Event.count("processed_messages", processedRequests());
-        Event.count("errors", errors());
-        Event.value("delayed_responses", delayedResponses());
-    }
-
-    void stop() {
-        stopped = true;
-    }
-
-    Long getEventInterval() {
-        return eventInterval;
-    }
-
-    void incRpcRequests() {
-        rpcRequests++;
-    }
-
-    void incProcessedRequests() {
-        processedRequests++;
-    }
-
-    void incErrorCount() {
-        errors++;
-    }
-
-    long processedRequests() {
-        return processedRequests;
-    }
-
-    long rpcRequests() {
-        return rpcRequests;
-    }
-
-    long errors() {
-        return errors;
-    }
-
-    long delayedResponses() {
-        return delayedResponses;
-    }
-
-    void delayedResponses(long count) {
-        delayedResponses = count;
-    }
-
-    void decDelayedResponses() {
-        delayedResponses--;
-    }
-}
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 1f7053e0ad3..f4f2308f261 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
@@ -38,7 +38,7 @@ public class ProxyServer implements Runnable {
     private static final int JRT_TRANSPORT_THREADS = 4;
     static final String DEFAULT_PROXY_CONFIG_SOURCES = "tcp/localhost:19070";
 
-    final static Logger log = Logger.getLogger(ProxyServer.class.getName());
+    private final static Logger log = Logger.getLogger(ProxyServer.class.getName());
     private final AtomicBoolean signalCaught = new AtomicBoolean(false);
 
     // Scheduled executor that periodically checks for requests that have timed out and response should be returned to clients
@@ -52,7 +52,6 @@ public class ProxyServer implements Runnable {
 
     private volatile ConfigSourceClient configClient;
 
-    private final ConfigProxyStatistics statistics;
     private final TimingValues timingValues;
     private final MemoryCache memoryCache;
     private static final double timingValuesRatio = 0.8;
@@ -72,32 +71,28 @@ public class ProxyServer implements Runnable {
         defaultTimingValues = tv;
     }
 
-    private ProxyServer(Spec spec, ConfigSourceSet source,
-                        ConfigProxyStatistics statistics, TimingValues timingValues,
-                        boolean delayedResponseHandling, MemoryCache memoryCache,
-                        ConfigSourceClient configClient) {
+    private ProxyServer(Spec spec, ConfigSourceSet source, TimingValues timingValues,
+                        boolean delayedResponseHandling, MemoryCache memoryCache, ConfigSourceClient configClient) {
         this.delayedResponses = new DelayedResponses();
         this.configSource = source;
         log.log(LogLevel.DEBUG, "Using config source '" + source);
-        this.statistics = statistics;
         this.timingValues = timingValues;
         this.delayedResponseHandling = delayedResponseHandling;
         this.memoryCache = memoryCache;
         this.rpcServer = createRpcServer(spec);
-        this.configClient = createClient(rpcServer, statistics, delayedResponses, source, timingValues, memoryCache, configClient);
+        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(), new ConfigProxyStatistics());
+        return createTestServer(source, null, new MemoryCache());
     }
 
     static ProxyServer createTestServer(ConfigSourceSet source,
                                         ConfigSourceClient configSourceClient,
-                                        MemoryCache memoryCache,
-                                        ConfigProxyStatistics statistics) {
+                                        MemoryCache memoryCache) {
         final boolean delayedResponseHandling = false;
-        return new ProxyServer(null, source, statistics, defaultTimingValues(), delayedResponseHandling,
+        return new ProxyServer(null, source, defaultTimingValues(), delayedResponseHandling,
                                memoryCache, configSourceClient);
     }
 
@@ -119,7 +114,6 @@ public class ProxyServer implements Runnable {
     }
 
     RawConfig resolveConfig(JRTServerConfigRequest req) {
-        statistics.incProcessedRequests();
         // Calling getConfig() will either return with an answer immediately or
         // create a background thread that retrieves config from the server and
         // calls updateSubscribers when new config is returned from the config source.
@@ -154,12 +148,11 @@ public class ProxyServer implements Runnable {
         }
     }
 
-    private ConfigSourceClient createClient(RpcServer rpcServer, ConfigProxyStatistics statistics,
-                                            DelayedResponses delayedResponses,
+    private ConfigSourceClient createClient(RpcServer rpcServer, DelayedResponses delayedResponses,
                                             ConfigSourceSet source, TimingValues timingValues,
                                             MemoryCache memoryCache, ConfigSourceClient client) {
         return (client == null)
-                ? new RpcConfigSourceClient(rpcServer, source, statistics, memoryCache, timingValues, delayedResponses)
+                ? new RpcConfigSourceClient(rpcServer, source, memoryCache, timingValues, delayedResponses)
                 : client;
     }
 
@@ -168,7 +161,7 @@ public class ProxyServer implements Runnable {
     }
 
     private RpcConfigSourceClient createRpcClient() {
-        return new RpcConfigSourceClient(rpcServer, configSource, statistics, memoryCache, timingValues, delayedResponses);
+        return new RpcConfigSourceClient(rpcServer, configSource, memoryCache, timingValues, delayedResponses);
     }
 
     private void setupSignalHandler() {
@@ -201,14 +194,9 @@ public class ProxyServer implements Runnable {
             port = Integer.parseInt(args[0]);
         }
         Event.started("configproxy");
-        ConfigProxyStatistics statistics = new ConfigProxyStatistics(properties.eventInterval);
-        Thread t = new Thread(statistics);
-        t.setName("Metrics generator");
-        t.setDaemon(true);
-        t.start();
 
         ConfigSourceSet configSources = new ConfigSourceSet(properties.configSources);
-        ProxyServer proxyServer = new ProxyServer(new Spec(null, port), configSources, statistics,
+        ProxyServer proxyServer = new ProxyServer(new Spec(null, port), configSources,
                                                   defaultTimingValues(), true, new MemoryCache(), null);
         // catch termination and interrupt signal
         proxyServer.setupSignalHandler();
@@ -219,18 +207,14 @@ public class ProxyServer implements Runnable {
     }
 
     static Properties getSystemProperties() {
-        // Read system properties
-        long eventInterval = Long.getLong("eventinterval", ConfigProxyStatistics.defaultEventInterval);
         final String[] inputConfigSources = System.getProperty("proxyconfigsources", DEFAULT_PROXY_CONFIG_SOURCES).split(",");
-        return new Properties(eventInterval, inputConfigSources);
+        return new Properties(inputConfigSources);
     }
 
     static class Properties {
-        final long eventInterval;
         final String[] configSources;
 
-        Properties(long eventInterval, String[] configSources) {
-            this.eventInterval = eventInterval;
+        Properties(String[] configSources) {
             this.configSources = configSources;
         }
     }
@@ -243,10 +227,6 @@ public class ProxyServer implements Runnable {
         return timingValues;
     }
 
-    ConfigProxyStatistics getStatistics() {
-        return statistics;
-    }
-
     // Cancels all config instances and flushes the cache. When this method returns,
     // the cache will not be updated again before someone calls getConfig().
     private synchronized void flush() {
@@ -259,7 +239,6 @@ public class ProxyServer implements Runnable {
         if (rpcServer != null) rpcServer.shutdown();
         if (delayedResponseScheduler != null) delayedResponseScheduler.cancel(true);
         flush();
-        if (statistics != null) statistics.stop();
         fileDistributionAndUrlDownload.close();
     }
 
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 c9f43ac48e2..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
@@ -38,7 +38,6 @@ class RpcConfigSourceClient implements ConfigSourceClient {
     private final HashMap<ConfigCacheKey, Subscriber> activeSubscribers = new HashMap<>();
     private final Object activeSubscribersLock = new Object();
     private final MemoryCache memoryCache;
-    private final ConfigProxyStatistics statistics;
     private final DelayedResponses delayedResponses;
     private final TimingValues timingValues;
 
@@ -48,13 +47,11 @@ class RpcConfigSourceClient implements ConfigSourceClient {
 
     RpcConfigSourceClient(RpcServer rpcServer,
                           ConfigSourceSet configSourceSet,
-                          ConfigProxyStatistics statistics,
                           MemoryCache memoryCache,
                           TimingValues timingValues,
                           DelayedResponses delayedResponses) {
         this.rpcServer = rpcServer;
         this.configSourceSet = configSourceSet;
-        this.statistics = statistics;
         this.memoryCache = memoryCache;
         this.delayedResponses = delayedResponses;
         this.timingValues = timingValues;
@@ -122,7 +119,6 @@ class RpcConfigSourceClient implements ConfigSourceClient {
         // happens at the same time
         DelayedResponse delayedResponse = new DelayedResponse(request);
         delayedResponses.add(delayedResponse);
-        statistics.delayedResponses(delayedResponses.size());
 
         final ConfigCacheKey configCacheKey = new ConfigCacheKey(input.getKey(), input.getDefMd5());
         RawConfig cachedConfig = memoryCache.get(configCacheKey);
@@ -139,7 +135,6 @@ class RpcConfigSourceClient implements ConfigSourceClient {
                     // unless another thread already did it
                     ret = cachedConfig;
                 }
-                statistics.decDelayedResponses();
             }
             if (!cachedConfig.isError() && cachedConfig.getGeneration() > 0) {
                 needToGetConfig = false;
@@ -220,7 +215,6 @@ class RpcConfigSourceClient implements ConfigSourceClient {
      */
     public void updateSubscribers(RawConfig config) {
         log.log(LogLevel.DEBUG, () -> "Config updated for " + config.getKey() + "," + config.getGeneration());
-        if (config.isError()) { statistics.incErrorCount(); }
         DelayQueue<DelayedResponse> responseDelayQueue = delayedResponses.responses();
         log.log(LogLevel.SPAM, () -> "Delayed response queue: " + responseDelayQueue);
         if (responseDelayQueue.size() == 0) {
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 9d6d0ca2a39..803f5c85b5c 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
@@ -23,7 +23,6 @@ public class ProxyServerTest {
     private final MemoryCache memoryCache = new MemoryCache();
     private final MockConfigSource source = new MockConfigSource();
     private MockConfigSourceClient client = new MockConfigSourceClient(source, memoryCache);
-    private final ConfigProxyStatistics statistics = new ConfigProxyStatistics();
     private ProxyServer proxy;
 
     static final RawConfig fooConfig = ConfigTester.fooConfig;
@@ -42,7 +41,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, statistics);
+        proxy = ProxyServer.createTestServer(source, client, memoryCache);
     }
 
     @After
@@ -64,27 +63,6 @@ public class ProxyServerTest {
         assertThat(res.getPayload().toString(), is(ConfigTester.fooPayload.toString()));
         assertEquals(1, memoryCache.size());
         assertThat(memoryCache.get(new ConfigCacheKey(fooConfig.getKey(), fooConfig.getDefMd5())), is(res));
-
-
-        assertEquals(1, statistics.processedRequests());
-        assertEquals(0, statistics.rpcRequests());
-        assertEquals(0, statistics.errors());
-        assertEquals(0, statistics.delayedResponses());
-
-        statistics.incProcessedRequests();
-        statistics.incRpcRequests();
-        statistics.incErrorCount();
-        statistics.delayedResponses(1);
-
-        assertEquals(2, statistics.processedRequests());
-        assertEquals(1, statistics.rpcRequests());
-        assertEquals(1, statistics.errors());
-        assertEquals(1, statistics.delayedResponses());
-
-        statistics.decDelayedResponses();
-        assertEquals(0, statistics.delayedResponses());
-
-        assertEquals(ConfigProxyStatistics.defaultEventInterval, statistics.getEventInterval().longValue());
     }
 
     /**
@@ -228,7 +206,6 @@ public class ProxyServerTest {
     @Test
     public void testReadingSystemProperties() {
         ProxyServer.Properties properties = ProxyServer.getSystemProperties();
-        assertThat(properties.eventInterval, is(ConfigProxyStatistics.defaultEventInterval));
         assertThat(properties.configSources.length, is(1));
         assertThat(properties.configSources[0], is(ProxyServer.DEFAULT_PROXY_CONFIG_SOURCES));
     }
diff --git a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java
index 7f762955b92..35f1dd8fcd8 100644
--- a/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java
+++ b/config-proxy/src/test/java/com/yahoo/vespa/config/proxy/RpcConfigSourceClientTest.java
@@ -18,7 +18,6 @@ import static org.junit.Assert.assertEquals;
 public class RpcConfigSourceClientTest {
 
     private MockRpcServer rpcServer;
-    private ConfigProxyStatistics statistics;
     private DelayedResponses delayedResponses;
     private RpcConfigSourceClient rpcConfigSourceClient;
 
@@ -29,10 +28,9 @@ public class RpcConfigSourceClientTest {
     @Before
     public void setup() {
         rpcServer = new MockRpcServer();
-        statistics = new ConfigProxyStatistics();
         delayedResponses = new DelayedResponses();
         rpcConfigSourceClient =
-                new RpcConfigSourceClient(rpcServer, new MockConfigSource(), statistics,
+                new RpcConfigSourceClient(rpcServer, new MockConfigSource(),
                                           new MemoryCache(), ProxyServer.defaultTimingValues(), delayedResponses);
     }
 
@@ -57,7 +55,6 @@ public class RpcConfigSourceClientTest {
     public void errorResponse() {
         configUpdatedSendResponse(ProxyServerTest.errorConfig);
         assertSentResponses(0);
-        assertEquals(1, statistics.errors());
     }
 
     @Test