Reuse Supervisor instance

This will allow the client to reuse existing SSL sessions

3 files changed, 65 insertions, 51 deletions

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/ConfigSentinelClient.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/ConfigSentinelClient.java
index 9fceafc8849..b5bd6c073cd 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/ConfigSentinelClient.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/service/ConfigSentinelClient.java
@@ -2,6 +2,7 @@
 package ai.vespa.metricsproxy.service;
 
 import com.google.inject.Inject;
+import com.yahoo.component.AbstractComponent;
 import com.yahoo.log.LogLevel;
 
 import com.yahoo.jrt.ErrorCode;
@@ -20,13 +21,21 @@ import java.util.logging.Logger;
 /**
  * Connects to the config sentinel and gets information like pid for the services on the node
  */
-public class ConfigSentinelClient {
+public class ConfigSentinelClient extends AbstractComponent {
     private final static Logger log = Logger.getLogger(ConfigSentinelClient.class.getName());
 
+    private final Supervisor supervisor = new Supervisor(new Transport());
+
     @Inject
     public ConfigSentinelClient() {
     }
 
+    @Override
+    public void deconstruct() {
+        supervisor.transport().shutdown().join();
+        super.deconstruct();
+    }
+
     /**
      * Update all services reading from config sentinel
      *
@@ -145,24 +154,26 @@ public class ConfigSentinelClient {
 
     String sentinelLs() {
         String servicelist = "";
-        Supervisor supervisor = new Supervisor(new Transport());
         int rpcPort = 19097;
         Spec spec = new Spec("localhost", rpcPort);
         Target connection = supervisor.connect(spec);
-        if (connection.isValid()) {
-            Request req = new Request("sentinel.ls");
-            connection.invokeSync(req, 5.0);
-            if (req.errorCode() == ErrorCode.NONE &&
-                req.checkReturnTypes("s"))
-            {
-                servicelist = req.returnValues().get(0).asString();
+        try {
+            if (connection.isValid()) {
+                Request req = new Request("sentinel.ls");
+                connection.invokeSync(req, 5.0);
+                if (req.errorCode() == ErrorCode.NONE &&
+                    req.checkReturnTypes("s"))
+                {
+                    servicelist = req.returnValues().get(0).asString();
+                } else {
+                    log.log(LogLevel.WARNING, "Bad answer to RPC request: " + req.errorMessage());
+                }
             } else {
-                log.log(LogLevel.WARNING, "Bad answer to RPC request: " + req.errorMessage());
+                log.log(LogLevel.WARNING, "Could not connect to sentinel at: "+spec);
             }
-        } else {
-            log.log(LogLevel.WARNING, "Could not connect to sentinel at: "+spec);
+            return servicelist;
+        } finally {
+            connection.close();
         }
-        supervisor.transport().shutdown().join();
-        return servicelist;
     }
 }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ConfigSentinelClientTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ConfigSentinelClientTest.java
index c28e16a8eaa..810596d6d0b 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ConfigSentinelClientTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/ConfigSentinelClientTest.java
@@ -26,38 +26,39 @@ public class ConfigSentinelClientTest {
         services.add(qrserver);
         services.add(docproc);
 
-        MockConfigSentinelClient client = new MockConfigSentinelClient(configsentinel);
-        client.updateServiceStatuses(services);
+        try (MockConfigSentinelClient client = new MockConfigSentinelClient(configsentinel)) {
+            client.updateServiceStatuses(services);
 
-        assertThat(qrserver.getPid(), is(6520));
-        assertThat(qrserver.getState(), is("RUNNING"));
-        assertThat(qrserver.isAlive(), is(true));
-        assertThat(searchnode4.getPid(), is(6534));
-        assertThat(searchnode4.getState(), is("RUNNING"));
-        assertThat(searchnode4.isAlive(), is(true));
+            assertThat(qrserver.getPid(), is(6520));
+            assertThat(qrserver.getState(), is("RUNNING"));
+            assertThat(qrserver.isAlive(), is(true));
+            assertThat(searchnode4.getPid(), is(6534));
+            assertThat(searchnode4.getState(), is("RUNNING"));
+            assertThat(searchnode4.isAlive(), is(true));
 
-        assertThat(docproc.getPid(), is(-1));
-        assertThat(docproc.getState(), is("FINISHED"));
-        assertThat(docproc.isAlive(), is(false));
+            assertThat(docproc.getPid(), is(-1));
+            assertThat(docproc.getState(), is("FINISHED"));
+            assertThat(docproc.isAlive(), is(false));
 
 
-        configsentinel.reConfigure();
+            configsentinel.reConfigure();
 
-        client.ping(docproc);
-        assertThat(docproc.getPid(), is(100));
-        assertThat(docproc.getState(), is("RUNNING"));
-        assertThat(docproc.isAlive(), is(true));
+            client.ping(docproc);
+            assertThat(docproc.getPid(), is(100));
+            assertThat(docproc.getState(), is("RUNNING"));
+            assertThat(docproc.isAlive(), is(true));
 
-        //qrserver has yet not been checked
-        assertThat(qrserver.isAlive(), is(true));
+            //qrserver has yet not been checked
+            assertThat(qrserver.isAlive(), is(true));
 
-        client.updateServiceStatuses(services);
+            client.updateServiceStatuses(services);
 
-        assertThat(docproc.getPid(), is(100));
-        assertThat(docproc.getState(), is("RUNNING"));
-        assertThat(docproc.isAlive(), is(true));
-        //qrserver is no longer running on this node - so should be false
-        assertThat(qrserver.isAlive(), is(false));
+            assertThat(docproc.getPid(), is(100));
+            assertThat(docproc.getState(), is("RUNNING"));
+            assertThat(docproc.isAlive(), is(true));
+            //qrserver is no longer running on this node - so should be false
+            assertThat(qrserver.isAlive(), is(false));
+        }
     }
 
     @Test
@@ -88,15 +89,16 @@ public class ConfigSentinelClientTest {
         services.add(containerClusterController);
         services.add(notPresent);
 
-        MockConfigSentinelClient client = new MockConfigSentinelClient(configsentinel);
-        client.updateServiceStatuses(services);
-        assertThat(container.isAlive(),is(true));
-        assertThat(container.getPid(),is(14338));
-        assertThat(container.getState(),is("RUNNING"));
+        try (MockConfigSentinelClient client = new MockConfigSentinelClient(configsentinel)) {
+            client.updateServiceStatuses(services);
+            assertThat(container.isAlive(),is(true));
+            assertThat(container.getPid(),is(14338));
+            assertThat(container.getState(),is("RUNNING"));
 
-        assertThat(containerClusterController.isAlive(),is(true));
-        assertThat(containerClusterController.getPid(),is(25020));
-        assertThat(containerClusterController.getState(),is("RUNNING"));
+            assertThat(containerClusterController.isAlive(),is(true));
+            assertThat(containerClusterController.getPid(),is(25020));
+            assertThat(containerClusterController.getState(),is("RUNNING"));
+        }
     }
 
 }
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MockConfigSentinelClient.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MockConfigSentinelClient.java
index 4c660711c5d..55702b4dc45 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MockConfigSentinelClient.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/service/MockConfigSentinelClient.java
@@ -1,10 +1,6 @@
 // Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package ai.vespa.metricsproxy.service;
 
-import com.yahoo.log.LogLevel;
-
-import java.util.ArrayList;
-import java.util.List;
 import java.util.logging.Logger;
 
 /**
@@ -12,7 +8,7 @@ import java.util.logging.Logger;
  *
  * @author hmusum
  */
-public class MockConfigSentinelClient extends ConfigSentinelClient {
+public class MockConfigSentinelClient extends ConfigSentinelClient implements AutoCloseable {
     private final ConfigSentinelDummy configSentinel;
     private final static Logger log = Logger.getLogger(MockConfigSentinelClient.class.getPackage().getName());
 
@@ -25,4 +21,9 @@ public class MockConfigSentinelClient extends ConfigSentinelClient {
     String sentinelLs() {
         return configSentinel.getServiceList();
     }
+
+    @Override
+    public void close() {
+        super.deconstruct();
+    }
 }