Remove getSupervisor() from ConnectionPool interface

Cleanup use of supervisor in connection pool and some file download
classes

4 files changed, 42 insertions, 72 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionUtil.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionUtil.java
index 2d39c5549cc..a1ddad7bfd4 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionUtil.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionUtil.java
@@ -3,13 +3,7 @@ package com.yahoo.vespa.config.server.filedistribution;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.FileReference;
-import com.yahoo.config.subscription.ConfigSourceSet;
-import com.yahoo.jrt.Supervisor;
-import com.yahoo.jrt.Transport;
 import com.yahoo.net.HostName;
-import com.yahoo.vespa.config.Connection;
-import com.yahoo.vespa.config.ConnectionPool;
-import com.yahoo.vespa.config.JRTConnectionPool;
 import com.yahoo.vespa.config.server.ConfigServerSpec;
 
 import java.io.File;
@@ -38,62 +32,16 @@ public class FileDistributionUtil {
         return fileReferencesOnDisk;
     }
 
-    /**
-     * Returns a connection pool with all config servers except this one, or an empty pool if there
-     * is only one config server (no point in trying to download from yourself).
-     */
-    public static ConnectionPool createConnectionPool(ConfigserverConfig configserverConfig) {
-        List<String> configServers = ConfigServerSpec.fromConfig(configserverConfig)
-                .stream()
-                .filter(spec -> !spec.getHostName().equals(HostName.getLocalhost()))
-                .map(spec -> "tcp/" + spec.getHostName() + ":" + spec.getConfigServerPort())
-                .collect(Collectors.toList());
-
-        return configServers.size() > 0
-                ? new JRTConnectionPool(new ConfigSourceSet(configServers),
-                                        new Supervisor(new Transport("filedistribution-pool"))
-                                                .setDropEmptyBuffers(true))
-                : emptyConnectionPool();
+    public static List<String> getOtherConfigServersInCluster(ConfigserverConfig configserverConfig) {
+        return ConfigServerSpec.fromConfig(configserverConfig)
+                               .stream()
+                               .filter(spec -> !spec.getHostName().equals(HostName.getLocalhost()))
+                               .map(spec -> "tcp/" + spec.getHostName() + ":" + spec.getConfigServerPort())
+                               .collect(Collectors.toList());
     }
 
     public static boolean fileReferenceExistsOnDisk(File downloadDirectory, FileReference applicationPackageReference) {
         return getFileReferencesOnDisk(downloadDirectory).contains(applicationPackageReference.value());
     }
 
-    static ConnectionPool emptyConnectionPool() {
-        return new EmptyConnectionPool();
-    }
-
-    private static class EmptyConnectionPool implements ConnectionPool {
-        private Supervisor supervisor;
-
-        @Override
-        public void close() {
-            synchronized (this) {
-                if (supervisor != null) {
-                    supervisor.transport().shutdown().join();
-                }
-            }
-        }
-
-        @Override
-        public Connection getCurrent() { return null; }
-
-        @Override
-        public Connection switchConnection(Connection connection) { return null; }
-
-        @Override
-        public int getSize() { return 0; }
-
-        @Override
-        public Supervisor getSupervisor() {
-            synchronized (this) {
-                if (supervisor == null) {
-                    supervisor = new Supervisor(new Transport("empty-connection-pool")).setDropEmptyBuffers(true);
-                }
-            }
-            return supervisor;
-        }
-    }
-
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java
index 4c7d0b3122a..5d9a9c7b836 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileServer.java
@@ -8,6 +8,8 @@ import com.yahoo.config.FileReference;
 import com.yahoo.jrt.Int32Value;
 import com.yahoo.jrt.Request;
 import com.yahoo.jrt.StringValue;
+import com.yahoo.jrt.Supervisor;
+import com.yahoo.jrt.Transport;
 import com.yahoo.vespa.defaults.Defaults;
 import com.yahoo.vespa.filedistribution.CompressedFileReference;
 import com.yahoo.vespa.filedistribution.EmptyFileReferenceData;
@@ -27,8 +29,7 @@ import java.util.concurrent.Executors;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.createConnectionPool;
-import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.emptyConnectionPool;
+import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.getOtherConfigServersInCluster;
 
 public class FileServer {
     private static final Logger log = Logger.getLogger(FileServer.class.getName());
@@ -70,12 +71,16 @@ public class FileServer {
     @Inject
     public FileServer(ConfigserverConfig configserverConfig) {
         this(new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir())),
-             new FileDownloader(createConnectionPool(configserverConfig)));
+             new FileDownloader(getOtherConfigServersInCluster(configserverConfig),
+                                new Supervisor(new Transport("filedistribution-pool"))
+                                        .setDropEmptyBuffers(true)));
     }
 
     // For testing only
     public FileServer(File rootDir) {
-        this(rootDir, new FileDownloader(emptyConnectionPool()));
+        this(rootDir, new FileDownloader(FileDownloader.emptyConnectionPool(),
+                                         new Supervisor(new Transport("fileserver-for-testing"))
+                                                 .setDropEmptyBuffers(true)));
     }
 
     public FileServer(File rootDir, FileDownloader fileDownloader) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java
index 64765871676..78f61feed85 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java
@@ -4,7 +4,10 @@ package com.yahoo.vespa.config.server.maintenance;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.FileReference;
 import com.yahoo.config.provision.ApplicationId;
-import com.yahoo.vespa.config.ConnectionPool;
+import com.yahoo.config.subscription.ConfigSourceSet;
+import com.yahoo.jrt.Supervisor;
+import com.yahoo.jrt.Transport;
+import com.yahoo.vespa.config.JRTConnectionPool;
 import com.yahoo.vespa.config.server.ApplicationRepository;
 import com.yahoo.vespa.config.server.session.Session;
 import com.yahoo.vespa.config.server.session.SessionRepository;
@@ -19,8 +22,8 @@ import java.io.File;
 import java.time.Duration;
 import java.util.logging.Logger;
 
-import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.createConnectionPool;
 import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.fileReferenceExistsOnDisk;
+import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.getOtherConfigServersInCluster;
 
 /**
  * Verifies that all active sessions has an application package on local disk.
@@ -33,8 +36,10 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
     private static final Logger log = Logger.getLogger(ApplicationPackageMaintainer.class.getName());
 
     private final ApplicationRepository applicationRepository;
-    private final ConnectionPool connectionPool;
     private final File downloadDirectory;
+    private final ConfigserverConfig configserverConfig;
+    private final Supervisor supervisor;
+
 
     ApplicationPackageMaintainer(ApplicationRepository applicationRepository,
                                  Curator curator,
@@ -42,9 +47,8 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
                                  FlagSource flagSource) {
         super(applicationRepository, curator, flagSource, applicationRepository.clock().instant(), interval);
         this.applicationRepository = applicationRepository;
-        ConfigserverConfig configserverConfig = applicationRepository.configserverConfig();
-        connectionPool = createConnectionPool(configserverConfig);
-
+        this.configserverConfig = applicationRepository.configserverConfig();
+        this.supervisor = new Supervisor(new Transport("filedistribution-pool")).setDropEmptyBuffers(true);
         downloadDirectory = new File(Defaults.getDefaults().underVespaHome(configserverConfig.fileReferencesDir()));
     }
 
@@ -53,7 +57,7 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
         int attempts = 0;
         int failures = 0;
 
-        try (var fileDownloader = new FileDownloader(connectionPool, downloadDirectory, new Downloads())) {
+        try (var fileDownloader = createFileDownloader()) {
             for (var applicationId : applicationRepository.listApplications()) {
                 log.fine(() -> "Verifying application package for " + applicationId);
                 Session session = applicationRepository.getActiveSession(applicationId);
@@ -81,9 +85,16 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer {
         return asSuccessFactor(attempts, failures);
     }
 
+    private FileDownloader createFileDownloader() {
+        return new FileDownloader(new JRTConnectionPool(new ConfigSourceSet(getOtherConfigServersInCluster(configserverConfig)), supervisor),
+                                  supervisor,
+                                  downloadDirectory,
+                                  new Downloads());
+    }
+
     @Override
     public void awaitShutdown() {
-        connectionPool.close();
+        supervisor.transport().shutdown().join();
         super.awaitShutdown();
     }
 
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java
index 1f2efdfe17b..ec94b845546 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/filedistribution/FileServerTest.java
@@ -4,6 +4,8 @@ package com.yahoo.vespa.config.server.filedistribution;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.FileReference;
 import com.yahoo.io.IOUtils;
+import com.yahoo.jrt.Supervisor;
+import com.yahoo.jrt.Transport;
 import com.yahoo.net.HostName;
 import com.yahoo.vespa.filedistribution.Downloads;
 import com.yahoo.vespa.filedistribution.FileDownloader;
@@ -22,7 +24,6 @@ import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ExecutionException;
 
-import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.emptyConnectionPool;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -138,7 +139,12 @@ public class FileServerTest {
     private static class MockFileDownloader extends FileDownloader {
 
         public MockFileDownloader(File downloadDirectory) {
-            super(emptyConnectionPool(), downloadDirectory, new Downloads(), Duration.ofMillis(100), Duration.ofMillis(100));
+            super(FileDownloader.emptyConnectionPool(),
+                  new Supervisor(new Transport("mock")).setDropEmptyBuffers(true),
+                  downloadDirectory,
+                  new Downloads(),
+                  Duration.ofMillis(100),
+                  Duration.ofMillis(100));
         }
 
     }