Use one thread for removing unused applications

Use one thread instead of same number of threads as tenants

5 files changed, 34 insertions, 15 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
index 0435c8e59db..82231fbf5d8 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
@@ -45,6 +45,12 @@ public interface TenantApplications {
     Transaction deleteApplication(ApplicationId applicationId);
 
     /**
+     * Removes unused applications
+     *
+     */
+    void removeUnusedApplications();
+
+    /**
      * Closes the application repo. Once a repo has been closed, it should not be used again.
      */
     void close();
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 c3d13b86591..d6f34650f8f 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
@@ -18,15 +18,11 @@ import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent;
 import org.apache.curator.framework.recipes.cache.PathChildrenCacheListener;
 
-import java.time.Duration;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
 /**
@@ -40,13 +36,11 @@ import java.util.logging.Logger;
 public class ZKTenantApplications implements TenantApplications, PathChildrenCacheListener {
 
     private static final Logger log = Logger.getLogger(ZKTenantApplications.class.getName());
-    private static final Duration checkForRemovedApplicationsInterval = Duration.ofMinutes(1);
 
     private final Curator curator;
     private final Path applicationsPath;
     private final ExecutorService pathChildrenExecutor =
             Executors.newFixedThreadPool(1, ThreadFactoryFactory.getThreadFactory(ZKTenantApplications.class.getName()));
-    private final ScheduledExecutorService checkForRemovedApplicationsService = new ScheduledThreadPoolExecutor(1);
     private final Curator.DirectoryCache directoryCache;
     private final ReloadHandler reloadHandler;
     private final TenantName tenant;
@@ -60,10 +54,6 @@ public class ZKTenantApplications implements TenantApplications, PathChildrenCac
         this.directoryCache = curator.createDirectoryCache(applicationsPath.getAbsolute(), false, false, pathChildrenExecutor);
         this.directoryCache.start();
         this.directoryCache.addListener(this);
-        checkForRemovedApplicationsService.scheduleWithFixedDelay(this::removeApplications,
-                                                                  checkForRemovedApplicationsInterval.getSeconds(),
-                                                                  checkForRemovedApplicationsInterval.getSeconds(),
-                                                                  TimeUnit.SECONDS);
     }
 
     public static TenantApplications create(Curator curator, ReloadHandler reloadHandler, TenantName tenant) {
@@ -130,7 +120,6 @@ public class ZKTenantApplications implements TenantApplications, PathChildrenCac
     public void close() {
         directoryCache.close();
         pathChildrenExecutor.shutdown();
-        checkForRemovedApplicationsService.shutdown();
     }
 
     @Override
@@ -151,7 +140,7 @@ public class ZKTenantApplications implements TenantApplications, PathChildrenCac
         }
         // We might have lost events and might need to remove applications (new applications are
         // not added by listening for events here, they are added when session is added, see RemoteSessionRepo)
-        removeApplications();
+        removeUnusedApplications();
     }
 
     private void applicationRemoved(ApplicationId applicationId) {
@@ -163,7 +152,7 @@ public class ZKTenantApplications implements TenantApplications, PathChildrenCac
         log.log(LogLevel.DEBUG, Tenants.logPre(applicationId) + "Application added: " + applicationId);
     }
 
-    private void removeApplications() {
+    public void removeUnusedApplications() {
         ImmutableSet<ApplicationId> activeApplications = ImmutableSet.copyOf(listApplications());
         log.log(LogLevel.DEBUG, "Removing stale applications for tenant '" + tenant +
                 "', not removing these active applications: " + activeApplications);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java
index 7e26e7fe8f5..11b1d1de057 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenants.java
@@ -19,6 +19,7 @@ import org.apache.curator.framework.state.ConnectionState;
 import org.apache.curator.framework.state.ConnectionStateListener;
 import org.apache.zookeeper.KeeperException;
 
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
@@ -29,6 +30,8 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
@@ -43,7 +46,7 @@ import java.util.logging.Logger;
  * To create or delete a tenant, the handler calls {@link Tenants#addTenant} and {@link Tenants#deleteTenant} methods.
  * This will delete shared state from zookeeper, and return, so it does not mean a tenant is immediately deleted.
  *
- * Once a tenant is deleted from zookeeper, the zookeeper watcher thread will get notified on all configservers, and
+ * Once a tenant is deleted from zookeeper, the zookeeper watcher thread will get notified on all config servers, and
  * shutdown and delete any per-configserver state.
  *
  * @author Vegard Havdal
@@ -57,7 +60,7 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
 
     private static final Path tenantsPath = Path.fromString("/config/v2/tenants/");
     private static final Path vespaPath = Path.fromString("/vespa");
-
+    private static final Duration checkForRemovedApplicationsInterval = Duration.ofMinutes(1);
     private static final Logger log = Logger.getLogger(Tenants.class.getName());
 
     private final Map<TenantName, Tenant> tenants = new LinkedHashMap<>();
@@ -67,6 +70,7 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
 
     private final MetricUpdater metricUpdater;
     private final ExecutorService pathChildrenExecutor = Executors.newFixedThreadPool(1, ThreadFactoryFactory.getThreadFactory(Tenants.class.getName()));
+    private final ScheduledExecutorService checkForRemovedApplicationsService = new ScheduledThreadPoolExecutor(1);
     private final Curator.DirectoryCache directoryCache;
 
 
@@ -94,6 +98,10 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
         createTenants();
         notifyTenantsLoaded();
         log.log(LogLevel.INFO, "All tenants created"); // TODO: Change to debug
+        checkForRemovedApplicationsService.scheduleWithFixedDelay(this::removeUnusedApplications,
+                                                                  checkForRemovedApplicationsInterval.getSeconds(),
+                                                                  checkForRemovedApplicationsInterval.getSeconds(),
+                                                                  TimeUnit.SECONDS);
     }
 
     /**
@@ -199,6 +207,11 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
         return tenants.get(DEFAULT_TENANT);
     }
 
+
+    private void removeUnusedApplications() {
+        getAllTenants().forEach(tenant -> tenant.getApplicationRepo().removeUnusedApplications());
+    }
+
     private void notifyNewTenant(Tenant tenant) {
         for (TenantListener listener : tenantListeners) {
             listener.onTenantCreate(tenant.getName(), tenant);
@@ -328,6 +341,7 @@ public class Tenants implements ConnectionStateListener, PathChildrenCacheListen
     public void close() {
         directoryCache.close();
         pathChildrenExecutor.shutdown();
+        checkForRemovedApplicationsService.shutdown();
     }
 
     public boolean checkThatTenantExists(TenantName tenant) {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/MemoryTenantApplications.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MemoryTenantApplications.java
index 03bcb4d71e9..28b24f15a60 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/MemoryTenantApplications.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/MemoryTenantApplications.java
@@ -51,6 +51,11 @@ public class MemoryTenantApplications implements TenantApplications {
     }
 
     @Override
+    public void removeUnusedApplications() {
+        // do nothing
+    }
+
+    @Override
     public void close() {
         isOpen = false;
     }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionRepoTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionRepoTest.java
index 878339bd703..50c741c494c 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionRepoTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionRepoTest.java
@@ -173,6 +173,11 @@ public class RemoteSessionRepoTest extends TestWithCurator {
         }
 
         @Override
+        public void removeUnusedApplications() {
+            // do nothing
+        }
+
+        @Override
         public void close() {
 
         }