Revert "Config cleanup 5"

7 files changed, 80 insertions, 75 deletions

diff --git a/config/abi-spec.json b/config/abi-spec.json
index cf0ea291138..b60f9053642 100644
--- a/config/abi-spec.json
+++ b/config/abi-spec.json
@@ -209,15 +209,12 @@
       "protected void throwIfExceptionSet(com.yahoo.config.subscription.impl.ConfigSubscription)",
       "public void close()",
       "protected void closeRequesters()",
-      "protected void closeRequester()",
       "public java.lang.String toString()",
       "public java.lang.Thread startConfigThread(java.lang.Runnable)",
       "protected com.yahoo.config.subscription.ConfigSubscriber$State state()",
       "public void reload(long)",
       "public com.yahoo.config.subscription.ConfigSource getSource()",
       "public java.util.Map requesters()",
-      "public com.yahoo.config.subscription.impl.JRTConfigRequester requester()",
-      "public void requester(com.yahoo.config.subscription.impl.JRTConfigRequester)",
       "public boolean isClosed()",
       "public com.yahoo.config.subscription.ConfigHandle subscribe(com.yahoo.config.subscription.ConfigSubscriber$SingleSubscriber, java.lang.Class, java.lang.String)",
       "public long getGeneration()",
@@ -226,8 +223,7 @@
     ],
     "fields": [
       "protected final java.util.List subscriptionHandles",
-      "protected java.util.Map requesters",
-      "protected com.yahoo.config.subscription.impl.JRTConfigRequester requester"
+      "protected java.util.Map requesters"
     ]
   },
   "com.yahoo.config.subscription.ConfigURI": {
diff --git a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
index be039c335d8..5c0b932dcce 100644
--- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
+++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
@@ -10,6 +10,7 @@ import com.yahoo.vespa.config.ConfigKey;
 import com.yahoo.vespa.config.TimingValues;
 import com.yahoo.yolean.Exceptions;
 
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -18,8 +19,8 @@ import java.util.logging.Logger;
 import static java.util.stream.Collectors.toList;
 
 /**
- * Used for subscribing to one or more configs using the default config source. Can optionally be given a
- * {@link ConfigSource} for the configs that will be used when {@link #subscribe(Class, String)} is called.
+ * Used for subscribing to one or more configs. Can optionally be given a {@link ConfigSource} for the configs
+ * that will be used when {@link #subscribe(Class, String)} is called.
  *
  * {@link #subscribe(Class, String)} on the configs needed, call {@link #nextConfig(long)} and get the config from the
  * {@link ConfigHandle} which {@link #subscribe(Class, String)} returned.
@@ -42,15 +43,9 @@ public class ConfigSubscriber implements AutoCloseable {
     private boolean internalRedeploy = false;
 
     /**
-     * Reuse requester, limit number if many subscriptions.
-     *
-     * @deprecated use {@link #requester}
+     * Reuse requesters for equal source sets, limit number if many subscriptions.
      */
-    @Deprecated
-    // TODO: Remove in Vespa 8
-    protected Map<ConfigSourceSet, JRTConfigRequester> requesters = Map.of();
-
-    protected JRTConfigRequester requester = null;
+    protected Map<ConfigSourceSet, JRTConfigRequester> requesters = new HashMap<>();
 
     /**
      * The states of the subscriber. Affects the validity of calling certain methods.
@@ -329,28 +324,17 @@ public class ConfigSubscriber implements AutoCloseable {
         for (ConfigHandle<? extends ConfigInstance> h : subscriptionHandles) {
             h.subscription().close();
         }
-        closeRequester();
+        closeRequesters();
         log.log(LogLevel.DEBUG, "Config subscriber has been closed.");
     }
 
-
     /**
-     * Closes requester
-     * @deprecated use {@link #closeRequester()} instead
-     *
+     * Closes all open requesters
      */
-    @Deprecated
-    // TODO: Remove in Vespa 8
     protected void closeRequesters() {
-        closeRequester();
-    }
-
-    /**
-     * Closes requester
-     */
-    protected void closeRequester() {
-        if (requester != null)
+        for (JRTConfigRequester requester : requesters.values()) {
             requester.close();
+        }
     }
 
     @Override
@@ -410,24 +394,9 @@ public class ConfigSubscriber implements AutoCloseable {
     /**
      * Implementation detail, do not use.
      * @return requesters
-     * @deprecated use {@link #requester()}
      */
-    @Deprecated
-    // TODO: Remove in Vespa 8
     public Map<ConfigSourceSet, JRTConfigRequester> requesters() {
-        return Map.of((ConfigSourceSet)source, requester);
-    }
-
-    /**
-     * Implementation detail, do not use.
-     * @return requesters
-     */
-    public JRTConfigRequester requester() {
-        return requester;
-    }
-
-    public void requester(JRTConfigRequester requester) {
-        this.requester = requester;
+        return requesters;
     }
 
     public boolean isClosed() {
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java b/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java
index 457c15eda3d..324546230d9 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/GenericConfigSubscriber.java
@@ -2,33 +2,42 @@
 package com.yahoo.config.subscription.impl;
 
 import java.util.List;
+import java.util.Map;
 
 import com.yahoo.config.ConfigInstance;
 import com.yahoo.config.subscription.ConfigHandle;
 import com.yahoo.config.subscription.ConfigSource;
+import com.yahoo.config.subscription.ConfigSourceSet;
 import com.yahoo.config.subscription.ConfigSubscriber;
 import com.yahoo.vespa.config.ConfigKey;
 import com.yahoo.vespa.config.RawConfig;
 import com.yahoo.vespa.config.TimingValues;
 
 /**
- * A subscriber that can subscribe without supplying a config class. Used by config proxy.
+ * A subscriber that can subscribe without the class. Used by configproxy.
  *
  * @author Vegard Havdal
  */
 public class GenericConfigSubscriber extends ConfigSubscriber {
 
     /**
-     * Constructs a new subscriber using the given requester
+     * Constructs a new subscriber using the given pool of requesters (JRTConfigRequester holds 1 connection which in
+     * turn is subject to failover across the elems in the source set.)
+     * The behaviour is undefined if the map key is different from the source set the requester was built with.
+     * See also {@link JRTConfigRequester#get(com.yahoo.vespa.config.ConnectionPool, com.yahoo.vespa.config.TimingValues)}
      *
-     * @param requester a config requester
+     * @param requesters a map from config source set to config requester
      */
-    public GenericConfigSubscriber(JRTConfigRequester requester) {
-        this.requester = requester;
+    public GenericConfigSubscriber(Map<ConfigSourceSet, JRTConfigRequester> requesters) {
+        this.requesters = requesters;
+    }
+
+    public GenericConfigSubscriber() {
+        super();
     }
 
     /**
-     * Subscribes to config without using a config class. For internal use in config proxy.
+     * Subscribes to config without using the class. For internal use in config proxy.
      *
      * @param key the {@link ConfigKey to subscribe to}
      * @param defContent the config definition content for the config to subscribe to
@@ -59,4 +68,10 @@ public class GenericConfigSubscriber extends ConfigSubscriber {
         throw new UnsupportedOperationException();
     }
 
+    /**
+     * Do nothing, since we share requesters
+     */
+    public void closeRequesters() {
+    }
+
 }
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java
index 50f4eeae1b8..e9daafb779c 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigRequester.java
@@ -52,10 +52,20 @@ public class JRTConfigRequester implements RequestWaiter {
 
     /**
      * Returns a new requester
-     * @param connectionPool the connectionPool this requester should use
+     * @param connectionPool The connectionPool to use
+     * @param timingValues The timing values
+     * @return new requester object
+     */
+    public static JRTConfigRequester get(ConnectionPool connectionPool, TimingValues timingValues) {
+        return new JRTConfigRequester(connectionPool, timingValues);
+    }
+
+    /**
+     * New requester
+     *  @param connectionPool the connectionPool this requester should use
      * @param timingValues timeouts and delays used when sending JRT config requests
      */
-    public JRTConfigRequester(ConnectionPool connectionPool, TimingValues timingValues) {
+    JRTConfigRequester(ConnectionPool connectionPool, TimingValues timingValues) {
         this.connectionPool = connectionPool;
         this.timingValues = timingValues;
     }
diff --git a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java
index 58a6c5f9f0c..39e6c69f539 100644
--- a/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java
+++ b/config/src/main/java/com/yahoo/config/subscription/impl/JRTConfigSubscription.java
@@ -154,10 +154,10 @@ public class JRTConfigSubscription<T extends ConfigInstance> extends ConfigSubsc
     }
 
     private JRTConfigRequester getRequester() {
-        JRTConfigRequester requester = subscriber.requester();
+        JRTConfigRequester requester = subscriber.requesters().get(sources);
         if (requester == null) {
             requester = new JRTConfigRequester(new JRTConnectionPool(sources), timingValues);
-            subscriber.requester(requester);
+            subscriber.requesters().put(sources, requester);
         }
         return requester;
     }
diff --git a/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java b/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java
index 7a10db49408..08d215670db 100644
--- a/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java
+++ b/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java
@@ -1,21 +1,20 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.config.subscription;
 
+import java.util.*;
+
 import com.yahoo.config.subscription.impl.GenericConfigHandle;
 import com.yahoo.config.subscription.impl.GenericConfigSubscriber;
 import com.yahoo.config.subscription.impl.JRTConfigRequester;
 import com.yahoo.config.subscription.impl.JRTConfigRequesterTest;
 import com.yahoo.config.subscription.impl.MockConnection;
 import com.yahoo.vespa.config.ConfigKey;
+import com.yahoo.vespa.config.JRTConnectionPool;
 import com.yahoo.vespa.config.protocol.CompressionType;
 import org.junit.Test;
 
-import java.util.List;
-
 import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 /**
  *
@@ -27,33 +26,48 @@ public class GenericConfigSubscriberTest {
 
     @Test
     public void testSubscribeGeneric() {
+        Map<ConfigSourceSet, JRTConfigRequester> requesters = new HashMap<>();
         ConfigSourceSet sourceSet = new ConfigSourceSet("blabla");
-        GenericConfigSubscriber subscriber = createSubscriber();
-        final List<String> defContent = List.of("myVal int");
-        GenericConfigHandle handle = subscriber.subscribe(new ConfigKey<>("simpletypes", "id", "config"), defContent, sourceSet, JRTConfigRequesterTest.getTestTimingValues());
-        assertTrue(subscriber.nextConfig());
+        requesters.put(sourceSet, JRTConfigRequester.get(new MockConnection(), JRTConfigRequesterTest.getTestTimingValues()));
+        GenericConfigSubscriber sub = new GenericConfigSubscriber(requesters);
+        final List<String> defContent = Arrays.asList("myVal int");
+        GenericConfigHandle handle = sub.subscribe(new ConfigKey<>("simpletypes", "id", "config"), defContent, sourceSet, JRTConfigRequesterTest.getTestTimingValues());
+        assertTrue(sub.nextConfig());
         assertTrue(handle.isChanged());
         assertThat(handle.getRawConfig().getPayload().withCompression(CompressionType.UNCOMPRESSED).toString(), is("{}")); // MockConnection returns empty string
-        assertFalse(subscriber.nextConfig());
+        assertFalse(sub.nextConfig());
         assertFalse(handle.isChanged());
     }
 
+    @Test
+    public void testGenericRequesterPooling() {
+        ConfigSourceSet source1 = new ConfigSourceSet("tcp/foo:78");
+        ConfigSourceSet source2 = new ConfigSourceSet("tcp/bar:79");
+        JRTConfigRequester req1 = JRTConfigRequester.get(new JRTConnectionPool(source1), JRTConfigRequesterTest.getTestTimingValues());
+        JRTConfigRequester req2 = JRTConfigRequester.get(new JRTConnectionPool(source2), JRTConfigRequesterTest.getTestTimingValues());
+        Map<ConfigSourceSet, JRTConfigRequester> requesters = new LinkedHashMap<>();
+        requesters.put(source1, req1);
+        requesters.put(source2, req2);
+        GenericConfigSubscriber sub = new GenericConfigSubscriber(requesters);
+        assertEquals(sub.requesters().get(source1).getConnectionPool().getCurrent().getAddress(), "tcp/foo:78");
+        assertEquals(sub.requesters().get(source2).getConnectionPool().getCurrent().getAddress(), "tcp/bar:79");
+    }
+
     @Test(expected=UnsupportedOperationException.class)
     public void testOverriddenSubscribeInvalid1() {
-        createSubscriber().subscribe(null, null);
+        GenericConfigSubscriber sub = new GenericConfigSubscriber();
+        sub.subscribe(null, null);
     }
 
     @Test(expected=UnsupportedOperationException.class)
     public void testOverriddenSubscribeInvalid2() {
-        createSubscriber().subscribe(null, null, 0L);
+        GenericConfigSubscriber sub = new GenericConfigSubscriber();
+        sub.subscribe(null, null, 0L);
     }
 
     @Test(expected=UnsupportedOperationException.class)
     public void testOverriddenSubscribeInvalid3() {
-        createSubscriber().subscribe(null, null, "");
-    }
-
-    private GenericConfigSubscriber createSubscriber() {
-        return new GenericConfigSubscriber(new JRTConfigRequester(new MockConnection(), JRTConfigRequesterTest.getTestTimingValues()));
+        GenericConfigSubscriber sub = new GenericConfigSubscriber();
+        sub.subscribe(null, null, "");
     }
 }
diff --git a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java
index a9fbdde3dfa..5fcbd76b822 100644
--- a/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.java
+++ b/config/src/test/java/com/yahoo/vespa/config/protocol/JRTConfigRequestBase.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.protocol;
 
+import com.yahoo.foo.SimpletypesConfig;
 import com.yahoo.config.subscription.ConfigSet;
 import com.yahoo.config.subscription.ConfigSourceSet;
 import com.yahoo.config.subscription.ConfigSubscriber;
@@ -8,7 +9,6 @@ import com.yahoo.config.subscription.impl.GenericConfigSubscriber;
 import com.yahoo.config.subscription.impl.JRTConfigRequester;
 import com.yahoo.config.subscription.impl.JRTConfigSubscription;
 import com.yahoo.config.subscription.impl.MockConnection;
-import com.yahoo.foo.SimpletypesConfig;
 import com.yahoo.jrt.Request;
 import com.yahoo.slime.Inspector;
 import com.yahoo.slime.JsonDecoder;
@@ -24,6 +24,7 @@ import com.yahoo.vespa.config.util.ConfigUtils;
 import org.junit.Before;
 import org.junit.Test;
 
+import java.util.Collections;
 import java.util.Optional;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -239,7 +240,7 @@ public abstract class JRTConfigRequestBase {
         });
 
         ConfigSourceSet src = new ConfigSourceSet();
-        ConfigSubscriber subscriber = new GenericConfigSubscriber(new JRTConfigRequester(connection, new TimingValues()));
+        ConfigSubscriber subscriber = new GenericConfigSubscriber(Collections.singletonMap(src, JRTConfigRequester.get(connection, new TimingValues())));
         JRTConfigSubscription<SimpletypesConfig> sub = new JRTConfigSubscription<>(new ConfigKey<>(SimpletypesConfig.class, configId), subscriber, src, new TimingValues());
         sub.subscribe(120_0000);
         assertTrue(sub.nextConfig(120_0000));