Abandon in-flight metrics requests on deconstruct

3 files changed, 8 insertions, 9 deletions

diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetriever.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetriever.java
index 05b146bbacd..df5b0a16bee 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetriever.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/ApplicationMetricsRetriever.java
@@ -102,6 +102,11 @@ public class ApplicationMetricsRetriever extends AbstractComponent implements Ru
 
     @Override
     public void deconstruct() {
+        try {
+            httpClient.close();
+        } catch (IOException e) {
+            log.warning("Failed closing httpclient: " + e);
+        }
         synchronized (pollThread) {
             stopped = true;
             pollThread.notifyAll();
@@ -109,11 +114,6 @@ public class ApplicationMetricsRetriever extends AbstractComponent implements Ru
         try {
             pollThread.join();
         } catch (InterruptedException e) {}
-        try {
-            httpClient.close();
-        } catch (IOException e) {
-            log.warning("Failed closing httpclient: " + e);
-        }
         super.deconstruct();
     }
 
@@ -161,13 +161,12 @@ public class ApplicationMetricsRetriever extends AbstractComponent implements Ru
         int numOk = 0;
         int numTried = futures.size();
         for (Map.Entry<Node, Future<Boolean>> entry : futures.entrySet()) {
-            if (stopped) break;
             try {
                 Boolean result = entry.getValue().get(taskTimeout.get().toMillis(), TimeUnit.MILLISECONDS);
                 if (result == Boolean.TRUE) numOk++;
             } catch (InterruptedException | ExecutionException | TimeoutException e) {
                 Throwable cause = e.getCause();
-                if (e instanceof ExecutionException && ((cause instanceof SocketException) || cause instanceof ConnectTimeoutException)) {
+                if (stopped || e instanceof ExecutionException && ((cause instanceof SocketException) || cause instanceof ConnectTimeoutException)) {
                     log.log(Level.FINE, "Failed retrieving metrics for '" + entry.getKey() + "' : " + cause.getMessage());
                 } else {
                     log.log(Level.WARNING, "Failed retrieving metrics for '" + entry.getKey() + "' : ", e);
diff --git a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java
index 15edbe9230a..927d7b67d46 100644
--- a/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java
+++ b/metrics-proxy/src/main/java/ai/vespa/metricsproxy/http/application/NodeMetricsClient.java
@@ -32,7 +32,7 @@ import static java.util.stream.Collectors.toList;
  * Retrieves metrics from a single Vespa node over http. To avoid unnecessary load on metrics
  * proxies, a cached snapshot per consumer is retained and served for a fixed TTL period.
  * Upon failure to retrieve metrics, an empty snapshot is cached.
- *
+ * <p>
  * This class assumes that the consumer id is a valid and existing one, which is already
  * ensured by the {@link ApplicationMetricsHandler}.
  *
diff --git a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/NodeMetricsClientTest.java b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/NodeMetricsClientTest.java
index f9f5c96e598..0d3782a6e51 100644
--- a/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/NodeMetricsClientTest.java
+++ b/metrics-proxy/src/test/java/ai/vespa/metricsproxy/http/application/NodeMetricsClientTest.java
@@ -32,7 +32,7 @@ import static org.junit.Assert.assertTrue;
 
 /**
  * Two optimizations worth noting:
- *
+ * <p>
  * 1. Using a ClassRule for the wire mocking means it is reused between test methods.
  * 2. Configuring stubs on the rule is faster than using the static WireMock.stubFor method.
  *