Use a thread pool for wathcing for new sessions

Also, make sure to use a daemon thread factory and do not close
executor service when it should live until JVM is shutdown

3 files changed, 11 insertions, 18 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ZKTenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ZKTenantApplications.java
index eb01e8f50f8..35c1e4fee80 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ZKTenantApplications.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ZKTenantApplications.java
@@ -41,7 +41,7 @@ public class ZKTenantApplications implements TenantApplications, PathChildrenCac
     private final Path applicationsPath;
     // One thread pool for all instances of this class
     private static final ExecutorService pathChildrenExecutor =
-            Executors.newCachedThreadPool(ThreadFactoryFactory.getThreadFactory(ZKTenantApplications.class.getName()));
+            Executors.newCachedThreadPool(ThreadFactoryFactory.getDaemonThreadFactory(ZKTenantApplications.class.getName()));
     private final Curator.DirectoryCache directoryCache;
     private final ReloadHandler reloadHandler;
     private final TenantName tenant;
@@ -120,11 +120,10 @@ public class ZKTenantApplications implements TenantApplications, PathChildrenCac
     @Override
     public void close() {
         directoryCache.close();
-        pathChildrenExecutor.shutdown();
     }
 
     @Override
-    public void childEvent(CuratorFramework client, PathChildrenCacheEvent event) throws Exception {
+    public void childEvent(CuratorFramework client, PathChildrenCacheEvent event) {
         switch (event.getType()) {
             case CHILD_ADDED:
                 applicationAdded(ApplicationId.fromSerializedForm(Path.fromString(event.getData().getPath()).getName()));
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java
index b71e85021c3..12fa828f692 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java
@@ -3,12 +3,14 @@ package com.yahoo.vespa.config.server.session;
 
 import java.util.*;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
 import com.google.common.collect.HashMultiset;
 import com.google.common.collect.Multiset;
+import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.config.provision.TenantName;
 import com.yahoo.log.LogLevel;
 import com.yahoo.path.Path;
@@ -38,6 +40,10 @@ import org.apache.curator.framework.recipes.cache.*;
 public class RemoteSessionRepo extends SessionRepo<RemoteSession> implements NodeCacheListener, PathChildrenCacheListener {
 
     private static final Logger log = Logger.getLogger(RemoteSessionRepo.class.getName());
+    // One thread pool for all instances of this class
+    private static final ExecutorService pathChildrenExecutor =
+            Executors.newCachedThreadPool(ThreadFactoryFactory.getDaemonThreadFactory(RemoteSessionRepo.class.getName()));
+
     private final Curator curator;
     private final Path sessionsPath;
     private final RemoteSessionFactory remoteSessionFactory;
@@ -53,15 +59,13 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> implements Nod
      * @param reloadHandler        a {@link com.yahoo.vespa.config.server.ReloadHandler}
      * @param tenant               a {@link TenantName} instance.
      * @param applicationRepo      a {@link TenantApplications} instance.
-     * @param executorService      an {@link ExecutorService} to run callbacks from ZooKeeper.
      */
     public RemoteSessionRepo(Curator curator,
                              RemoteSessionFactory remoteSessionFactory,
                              ReloadHandler reloadHandler,
                              TenantName tenant,
                              TenantApplications applicationRepo,
-                             MetricUpdater metricUpdater,
-                             ExecutorService executorService) {
+                             MetricUpdater metricUpdater) {
         this.curator = curator;
         this.sessionsPath = TenantRepository.getSessionsPath(tenant);
         this.applicationRepo = applicationRepo;
@@ -69,7 +73,7 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> implements Nod
         this.reloadHandler = reloadHandler;
         this.metrics = metricUpdater;
         initializeSessions();
-        this.directoryCache = curator.createDirectoryCache(sessionsPath.getAbsolute(), false, false, executorService);
+        this.directoryCache = curator.createDirectoryCache(sessionsPath.getAbsolute(), false, false, pathChildrenExecutor);
         this.directoryCache.addListener(this);
         this.directoryCache.start();
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantBuilder.java
index cd437704f9a..69721ed01d4 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantBuilder.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantBuilder.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.server.tenant;
 
-import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.path.Path;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.TenantName;
@@ -20,8 +19,6 @@ import com.yahoo.vespa.defaults.Defaults;
 import java.io.File;
 import java.time.Clock;
 import java.util.Collections;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 
 /**
  * Builder for helping out with tenant creation. Each of a tenants dependencies may be overridden for testing.
@@ -159,17 +156,10 @@ public class TenantBuilder {
                     reloadHandler,
                     tenant,
                     applicationRepo,
-                    componentRegistry.getMetrics().getOrCreateMetricUpdater(Metrics.createDimensions(tenant)),
-                    // TODO: Check if we can avoid using one executor service per tenant. Either one for all
-                    // or have a few executors and hash on tenant name to find out which one to use here
-                    createSingleThreadedExecutorService(RemoteSessionRepo.class.getName()));
+                    componentRegistry.getMetrics().getOrCreateMetricUpdater(Metrics.createDimensions(tenant)));
         }
     }
 
-    private ExecutorService createSingleThreadedExecutorService(String executorName) {
-        return Executors.newSingleThreadExecutor(ThreadFactoryFactory.getThreadFactory(executorName + "-" + tenant.value()));
-    }
-
     private void createServerDbDirs() {
         if (tenantFileSystemDirs == null) {
             tenantFileSystemDirs = new TenantFileSystemDirs(new File(