Merge pull request #12129 from vespa-engine/hmusum/cleanup-5

Config cleanup 5

7 files changed, 75 insertions, 80 deletions

diff --git a/config/abi-spec.json b/config/abi-spec.json
index b60f9053642..cf0ea291138 100644
--- a/config/abi-spec.json
+++ b/config/abi-spec.json
@@ -209,12 +209,15 @@
       "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()",
@@ -223,7 +226,8 @@
     ],
     "fields": [
       "protected final java.util.List subscriptionHandles",
-      "protected java.util.Map requesters"
+      "protected java.util.Map requesters",
+      "protected com.yahoo.config.subscription.impl.JRTConfigRequester requester"
     ]
   },
   "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 5c0b932dcce..be039c335d8 100644
--- a/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
+++ b/config/src/main/java/com/yahoo/config/subscription/ConfigSubscriber.java
@@ -10,7 +10,6 @@ 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;
@@ -19,8 +18,8 @@ import java.util.logging.Logger;
 import static java.util.stream.Collectors.toList;
 
 /**
- * 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.
+ * 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.
  *
  * {@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.
@@ -43,9 +42,15 @@ public class ConfigSubscriber implements AutoCloseable {
     private boolean internalRedeploy = false;
 
     /**
-     * Reuse requesters for equal source sets, limit number if many subscriptions.
+     * Reuse requester, limit number if many subscriptions.
+     *
+     * @deprecated use {@link #requester}
      */
-    protected Map<ConfigSourceSet, JRTConfigRequester> requesters = new HashMap<>();
+    @Deprecated
+    // TODO: Remove in Vespa 8
+    protected Map<ConfigSourceSet, JRTConfigRequester> requesters = Map.of();
+
+    protected JRTConfigRequester requester = null;
 
     /**
      * The states of the subscriber. Affects the validity of calling certain methods.
@@ -324,17 +329,28 @@ public class ConfigSubscriber implements AutoCloseable {
         for (ConfigHandle<? extends ConfigInstance> h : subscriptionHandles) {
             h.subscription().close();
         }
-        closeRequesters();
+        closeRequester();
         log.log(LogLevel.DEBUG, "Config subscriber has been closed.");
     }
 
+
     /**
-     * Closes all open requesters
+     * Closes requester
+     * @deprecated use {@link #closeRequester()} instead
+     *
      */
+    @Deprecated
+    // TODO: Remove in Vespa 8
     protected void closeRequesters() {
-        for (JRTConfigRequester requester : requesters.values()) {
+        closeRequester();
+    }
+
+    /**
+     * Closes requester
+     */
+    protected void closeRequester() {
+        if (requester != null)
             requester.close();
-        }
     }
 
     @Override
@@ -394,9 +410,24 @@ 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 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;
     }
 
     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 324546230d9..457c15eda3d 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,42 +2,33 @@
 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 the class. Used by configproxy.
+ * A subscriber that can subscribe without supplying a config class. Used by config proxy.
  *
  * @author Vegard Havdal
  */
 public class GenericConfigSubscriber extends ConfigSubscriber {
 
     /**
-     * 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)}
+     * Constructs a new subscriber using the given requester
      *
-     * @param requesters a map from config source set to config requester
+     * @param requester a config requester
      */
-    public GenericConfigSubscriber(Map<ConfigSourceSet, JRTConfigRequester> requesters) {
-        this.requesters = requesters;
-    }
-
-    public GenericConfigSubscriber() {
-        super();
+    public GenericConfigSubscriber(JRTConfigRequester requester) {
+        this.requester = requester;
     }
 
     /**
-     * Subscribes to config without using the class. For internal use in config proxy.
+     * Subscribes to config without using a config 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
@@ -68,10 +59,4 @@ 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 e9daafb779c..50f4eeae1b8 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,20 +52,10 @@ public class JRTConfigRequester implements RequestWaiter {
 
     /**
      * Returns a new requester
-     * @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 connectionPool the connectionPool this requester should use
      * @param timingValues timeouts and delays used when sending JRT config requests
      */
-    JRTConfigRequester(ConnectionPool connectionPool, TimingValues timingValues) {
+    public 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 39e6c69f539..58a6c5f9f0c 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.requesters().get(sources);
+        JRTConfigRequester requester = subscriber.requester();
         if (requester == null) {
             requester = new JRTConfigRequester(new JRTConnectionPool(sources), timingValues);
-            subscriber.requesters().put(sources, requester);
+            subscriber.requester(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 08d215670db..7a10db49408 100644
--- a/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java
+++ b/config/src/test/java/com/yahoo/config/subscription/GenericConfigSubscriberTest.java
@@ -1,20 +1,21 @@
 // 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.*;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
 
 /**
  *
@@ -26,48 +27,33 @@ public class GenericConfigSubscriberTest {
 
     @Test
     public void testSubscribeGeneric() {
-        Map<ConfigSourceSet, JRTConfigRequester> requesters = new HashMap<>();
         ConfigSourceSet sourceSet = new ConfigSourceSet("blabla");
-        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());
+        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());
         assertTrue(handle.isChanged());
         assertThat(handle.getRawConfig().getPayload().withCompression(CompressionType.UNCOMPRESSED).toString(), is("{}")); // MockConnection returns empty string
-        assertFalse(sub.nextConfig());
+        assertFalse(subscriber.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() {
-        GenericConfigSubscriber sub = new GenericConfigSubscriber();
-        sub.subscribe(null, null);
+        createSubscriber().subscribe(null, null);
     }
 
     @Test(expected=UnsupportedOperationException.class)
     public void testOverriddenSubscribeInvalid2() {
-        GenericConfigSubscriber sub = new GenericConfigSubscriber();
-        sub.subscribe(null, null, 0L);
+        createSubscriber().subscribe(null, null, 0L);
     }
 
     @Test(expected=UnsupportedOperationException.class)
     public void testOverriddenSubscribeInvalid3() {
-        GenericConfigSubscriber sub = new GenericConfigSubscriber();
-        sub.subscribe(null, null, "");
+        createSubscriber().subscribe(null, null, "");
+    }
+
+    private GenericConfigSubscriber createSubscriber() {
+        return new GenericConfigSubscriber(new JRTConfigRequester(new MockConnection(), JRTConfigRequesterTest.getTestTimingValues()));
     }
 }
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 5fcbd76b822..a9fbdde3dfa 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,7 +1,6 @@
 // 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;
@@ -9,6 +8,7 @@ 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,7 +24,6 @@ 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;
@@ -240,7 +239,7 @@ public abstract class JRTConfigRequestBase {
         });
 
         ConfigSourceSet src = new ConfigSourceSet();
-        ConfigSubscriber subscriber = new GenericConfigSubscriber(Collections.singletonMap(src, JRTConfigRequester.get(connection, new TimingValues())));
+        ConfigSubscriber subscriber = new GenericConfigSubscriber(new JRTConfigRequester(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));