Use separate executors for ZooKeeper watchers for applications and sessions

Session events might involve doing expensive operations (e.g. creating
config models for an application). Having just one executor has been
observed to lead to events for applications take so long that some
operations (e.g. deleting an application) times out and prevents holding
state in sync across config servers.

3 files changed, 15 insertions, 8 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java
index 13d8908d197..28211cf1d3e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java
@@ -103,7 +103,8 @@ public class TenantRepository {
     private final Metrics metrics;
     private final MetricUpdater metricUpdater;
     private final ExecutorService zkCacheExecutor;
-    private final StripedExecutor<TenantName> zkWatcherExecutor;
+    private final StripedExecutor<TenantName> zkSessionWatcherExecutor;
+    private final StripedExecutor<TenantName> zkApplicationWatcherExecutor;
     private final FileDistributionFactory fileDistributionFactory;
     private final FlagSource flagSource;
     private final SecretStore secretStore;
@@ -142,6 +143,7 @@ public class TenantRepository {
              configCurator,
              metrics,
              new StripedExecutor<>(),
+             new StripedExecutor<>(),
              new FileDistributionFactory(configserverConfig),
              flagSource,
              Executors.newFixedThreadPool(1, ThreadFactoryFactory.getThreadFactory(TenantRepository.class.getName())),
@@ -160,7 +162,8 @@ public class TenantRepository {
     public TenantRepository(HostRegistry hostRegistry,
                             ConfigCurator configCurator,
                             Metrics metrics,
-                            StripedExecutor<TenantName> zkWatcherExecutor,
+                            StripedExecutor<TenantName> zkApplicationWatcherExecutor ,
+                            StripedExecutor<TenantName> zkSessionWatcherExecutor,
                             FileDistributionFactory fileDistributionFactory,
                             FlagSource flagSource,
                             ExecutorService zkCacheExecutor,
@@ -182,7 +185,8 @@ public class TenantRepository {
         this.metrics = metrics;
         metricUpdater = metrics.getOrCreateMetricUpdater(Collections.emptyMap());
         this.zkCacheExecutor = zkCacheExecutor;
-        this.zkWatcherExecutor = zkWatcherExecutor;
+        this.zkApplicationWatcherExecutor = zkApplicationWatcherExecutor;
+        this.zkSessionWatcherExecutor = zkSessionWatcherExecutor;
         this.fileDistributionFactory = fileDistributionFactory;
         this.flagSource = flagSource;
         this.secretStore = secretStore;
@@ -315,7 +319,7 @@ public class TenantRepository {
         TenantApplications applicationRepo =
                 new TenantApplications(tenantName,
                                        curator,
-                                       zkWatcherExecutor,
+                                       zkApplicationWatcherExecutor,
                                        zkCacheExecutor,
                                        metrics,
                                        reloadListener,
@@ -339,7 +343,7 @@ public class TenantRepository {
                                                                     sessionPreparer,
                                                                     configCurator,
                                                                     metrics,
-                                                                    zkWatcherExecutor,
+                                                                    zkSessionWatcherExecutor,
                                                                     permanentApplicationPackage,
                                                                     flagSource,
                                                                     zkCacheExecutor,
@@ -508,12 +512,12 @@ public class TenantRepository {
             case CHILD_ADDED:
                 TenantName t1 = getTenantNameFromEvent(event);
                 if ( ! tenants.containsKey(t1))
-                    zkWatcherExecutor.execute(t1, () -> bootstrapTenant(t1));
+                    zkApplicationWatcherExecutor.execute(t1, () -> bootstrapTenant(t1));
                 break;
             case CHILD_REMOVED:
                 TenantName t2 = getTenantNameFromEvent(event);
                 if (tenants.containsKey(t2))
-                    zkWatcherExecutor.execute(t2, () -> deleteTenant(t2));
+                    zkApplicationWatcherExecutor.execute(t2, () -> deleteTenant(t2));
                 break;
             default:
                 break; // Nothing to do
@@ -534,7 +538,8 @@ public class TenantRepository {
         try {
             zkCacheExecutor.shutdown();
             checkForRemovedApplicationsService.shutdown();
-            zkWatcherExecutor.shutdownAndWait();
+            zkApplicationWatcherExecutor.shutdownAndWait();
+            zkSessionWatcherExecutor.shutdownAndWait();
             zkCacheExecutor.awaitTermination(50, TimeUnit.SECONDS);
             checkForRemovedApplicationsService.awaitTermination(50, TimeUnit.SECONDS);
         }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java
index b3d1495eb6b..6f7e0541cc7 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java
@@ -212,6 +212,7 @@ public class TenantRepositoryTest {
                   ConfigCurator.create(new MockCurator()),
                   Metrics.createTestMetrics(),
                   new StripedExecutor<>(new InThreadExecutorService()),
+                  new StripedExecutor<>(new InThreadExecutorService()),
                   new FileDistributionFactory(new ConfigserverConfig.Builder().build()),
                   new InMemoryFlagSource(),
                   new InThreadExecutorService(),
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TestTenantRepository.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TestTenantRepository.java
index 8279bf4df5e..687d58fd23b 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TestTenantRepository.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TestTenantRepository.java
@@ -50,6 +50,7 @@ public class TestTenantRepository extends TenantRepository {
               ConfigCurator.create(curator),
               metrics,
               new StripedExecutor<>(new InThreadExecutorService()),
+              new StripedExecutor<>(new InThreadExecutorService()),
               fileDistributionFactory,
               flagSource,
               new InThreadExecutorService(),