Exclude current sources when all sources are unhealthy

When choosing a new source when all are unhealthy exclude the current
source.
Minor changes to toString for JRTConnection

3 files changed, 61 insertions, 19 deletions

diff --git a/config/src/main/java/com/yahoo/vespa/config/JRTConnection.java b/config/src/main/java/com/yahoo/vespa/config/JRTConnection.java
index 52031ebde72..0d5f483ad2c 100644
--- a/config/src/main/java/com/yahoo/vespa/config/JRTConnection.java
+++ b/config/src/main/java/com/yahoo/vespa/config/JRTConnection.java
@@ -79,17 +79,14 @@ public class JRTConnection implements Connection {
 
     public String toString() {
         StringBuilder sb = new StringBuilder();
-        sb.append("Address: ");
         sb.append(address);
-        sb.append("\n").append("Healthy: ").append(isHealthy());
+        sb.append(", ").append(isHealthy() ? "healthy" : "unhealthy");
         if (lastSuccess.isAfter(Instant.EPOCH)) {
-            sb.append("\n");
-            sb.append("Last success: ");
+            sb.append(", last success: ");
             sb.append(lastSuccess);
         }
         if (lastFailure.isAfter(Instant.EPOCH)) {
-            sb.append("\n");
-            sb.append("Last failure: ");
+            sb.append(", last failure: ");
             sb.append(lastFailure);
         }
         return sb.toString();
diff --git a/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java b/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java
index 801cdc868e1..019103c7015 100644
--- a/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java
+++ b/config/src/main/java/com/yahoo/vespa/config/JRTConnectionPool.java
@@ -19,7 +19,7 @@ import java.util.stream.Collectors;
  * The current connection is chosen randomly when calling {#link {@link #switchConnection()}}
  * (it will continue to use the same connection if there is only one source).
  * The current connection is available with {@link #getCurrent()}.
- * When calling {@link #setError(Connection, int)}, {#link {@link #switchConnection()}} will always be called.
+ * When calling {@link #setError(Connection, int)}, {@link #switchConnection()} will always be called.
  *
  * @author Gunnar Gauslaa Bergem
  * @author hmusum
@@ -53,7 +53,7 @@ public class JRTConnectionPool implements ConnectionPool {
                 connections.put(address, new JRTConnection(address, supervisor));
             }
         }
-        initialize();
+        currentConnection = initialize();
     }
 
     /**
@@ -70,23 +70,21 @@ public class JRTConnectionPool implements ConnectionPool {
         List<JRTConnection> sources = getSources();
         if (sources.size() <= 1) return currentConnection;
 
-        List<JRTConnection> healthySources = sources.stream()
+        List<JRTConnection> sourceCandidates = sources.stream()
                                                     .filter(JRTConnection::isHealthy)
                                                     .collect(Collectors.toList());
-        if (healthySources.size() == 0) {
-            log.log(Level.INFO, "No healthy sources, keep using " + currentConnection);
-            return currentConnection;
+        JRTConnection newConnection;
+        if (sourceCandidates.size() == 0) {
+            sourceCandidates = getSources();
+            sourceCandidates.remove(currentConnection);
         }
-        JRTConnection newConnection = pickNewConnectionRandomly(healthySources);
+        newConnection = pickNewConnectionRandomly(sourceCandidates);
         log.log(Level.INFO, () -> "Switching from " + currentConnection + " to " + newConnection);
         return currentConnection = newConnection;
     }
 
     public synchronized JRTConnection initialize() {
-        List<JRTConnection> sources = getSources();
-        currentConnection = pickNewConnectionRandomly(sources);
-        log.log(Level.INFO, () -> "Choosing new connection: " + currentConnection);
-        return currentConnection;
+        return pickNewConnectionRandomly(getSources());
     }
 
     private JRTConnection pickNewConnectionRandomly(List<JRTConnection> sources) {
diff --git a/config/src/test/java/com/yahoo/vespa/config/JRTConnectionPoolTest.java b/config/src/test/java/com/yahoo/vespa/config/JRTConnectionPoolTest.java
index 56063fd94a5..08f9d0ad31d 100644
--- a/config/src/test/java/com/yahoo/vespa/config/JRTConnectionPoolTest.java
+++ b/config/src/test/java/com/yahoo/vespa/config/JRTConnectionPoolTest.java
@@ -4,12 +4,21 @@ package com.yahoo.vespa.config;
 import com.yahoo.config.subscription.ConfigSourceSet;
 import org.junit.Test;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.stream.Collectors;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  * Tests for the JRTConnectionPool class.
@@ -68,6 +77,7 @@ public class JRTConnectionPoolTest {
             }
         }
         assertConnectionDistributionIsFair(timesUsed);
+        sourcePool.close();
     }
 
     // Tests that the number of times each connection is used is close to equal
@@ -122,5 +132,42 @@ public class JRTConnectionPoolTest {
         assertThat(newSourceSet2.getSources().size(), is(1));
         assertThat(newSourceSet2, is(not(newSourceSet)));
         assertTrue(newSourceSet2.getSources().contains("host4"));
+
+        sourcePool.close();
     }
+
+    @Test
+    public void testFailingSources() {
+        List<String> sources = new ArrayList<>();
+
+        sources.add("host0");
+        sources.add("host1");
+        sources.add("host2");
+        JRTConnectionPool sourcePool = new JRTConnectionPool(sources);
+
+        Connection firstConnection = sourcePool.getCurrent();
+
+        // Should change connection away from first connection
+        sourcePool.setError(firstConnection, 123);
+        JRTConnection secondConnection = sourcePool.getCurrent();
+        assertNotEquals(secondConnection, firstConnection);
+
+        // Should change connection away from first AND second connection
+        sourcePool.setError(secondConnection, 123);
+        JRTConnection thirdConnection = sourcePool.getCurrent();
+        assertNotEquals(sourcePool.getCurrent(), firstConnection);
+        assertNotEquals(sourcePool.getCurrent(), secondConnection);
+
+        // Should change connection away from third connection
+        sourcePool.setError(thirdConnection, 123);
+        JRTConnection currentConnection = sourcePool.getCurrent();
+        assertNotEquals(sourcePool.getCurrent(), thirdConnection);
+
+        // Should change connection from current connection
+        sourcePool.setError(thirdConnection, 123);
+        assertNotEquals(sourcePool.getCurrent(), currentConnection);
+
+        sourcePool.close();
+    }
+
 }