Move deletion of inactive sessions to a maintainer

10 files changed, 106 insertions, 59 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index e8cfcf75de3..8d20d610c47 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -553,6 +553,9 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return session.getSessionId();
     }
 
+    public void deleteOldSessions() {
+        listApplications().forEach(app -> tenantRepository.getTenant(app.tenant()).getLocalSessionRepo().purgeOldSessions());
+    }
 
     // ---------------- Tenant operations ----------------------------------------------------------------
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
index 1f8e8ff6fe8..c1fc484e23c 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java
@@ -10,11 +10,19 @@ import com.yahoo.vespa.curator.Curator;
 
 import java.time.Duration;
 
+/**
+ * Maintenance jobs of the config server.
+ * Each maintenance job is a singleton instance of its implementing class, created and owned by this,
+ * and running its own dedicated thread.
+ *
+ * @author hmusum
+ */
 public class ConfigServerMaintenance extends AbstractComponent {
 
     private final TenantsMaintainer tenantsMaintainer;
     private final ZooKeeperDataMaintainer zooKeeperDataMaintainer;
     private final FileDistributionMaintainer fileDistributionMaintainer;
+    private final SessionsMaintainer sessionsMaintainer;
 
     @SuppressWarnings("unused") // instantiated by Dependency Injection
     public ConfigServerMaintenance(ConfigserverConfig configserverConfig,
@@ -25,6 +33,7 @@ public class ConfigServerMaintenance extends AbstractComponent {
         tenantsMaintainer = new TenantsMaintainer(applicationRepository, curator, defaults.tenantsMaintainerInterval);
         zooKeeperDataMaintainer = new ZooKeeperDataMaintainer(applicationRepository, curator, defaults.defaultInterval);
         fileDistributionMaintainer = new FileDistributionMaintainer(applicationRepository, curator, defaults.defaultInterval, configserverConfig);
+        sessionsMaintainer = new SessionsMaintainer(applicationRepository, curator, defaults.defaultInterval);
     }
 
     @Override
@@ -32,6 +41,7 @@ public class ConfigServerMaintenance extends AbstractComponent {
         tenantsMaintainer.deconstruct();
         zooKeeperDataMaintainer.deconstruct();
         fileDistributionMaintainer.deconstruct();
+        sessionsMaintainer.deconstruct();
     }
 
     /*
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
index 4f45b200111..a5d4aa226a6 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
@@ -9,7 +9,13 @@ import com.yahoo.vespa.defaults.Defaults;
 import java.io.File;
 import java.time.Duration;
 
-// Note: Unit test is in ApplicationRepositoryTest
+/**
+ * Removes unused file references from disk
+ * <p>
+ * Note: Unit test is in ApplicationRepositoryTest
+ *
+ * @author hmusum
+ */
 public class FileDistributionMaintainer extends Maintainer {
 
     private final ApplicationRepository applicationRepository;
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/Maintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/Maintainer.java
index ae000385dfd..513f70e4187 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/Maintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/Maintainer.java
@@ -16,6 +16,11 @@ import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+/**
+ * A maintainer is some job which runs at a fixed interval to perform some maintenance task in the config server.
+ *
+ * @author hmusum
+ */
 public abstract class Maintainer extends AbstractComponent implements Runnable {
 
     protected static final Logger log = Logger.getLogger(Maintainer.class.getName());
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java
new file mode 100644
index 00000000000..53f2553161f
--- /dev/null
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java
@@ -0,0 +1,26 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.config.server.maintenance;
+
+import com.yahoo.vespa.config.server.ApplicationRepository;
+import com.yahoo.vespa.curator.Curator;
+
+import java.time.Duration;
+
+/**
+ * Removes inactive sessions
+ * <p>
+ * Note: Unit test is in ApplicationRepositoryTest
+ *
+ * @author hmusum
+ */
+public class SessionsMaintainer extends Maintainer {
+
+    SessionsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval) {
+        super(applicationRepository, curator, interval);
+    }
+
+    @Override
+    protected void maintain() {
+        applicationRepository.deleteOldSessions();
+    }
+}
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java
index ad3b1f9b1ea..86bd4e8db8e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java
@@ -7,6 +7,11 @@ import com.yahoo.vespa.curator.Curator;
 import java.time.Duration;
 import java.time.Instant;
 
+/**
+ * Removes unused tenants (has no applications and was created more than 7 days ago)
+ *
+ * @author hmusum
+ */
 public class TenantsMaintainer extends Maintainer {
 
     private final Duration ttlForUnusedTenant;
@@ -21,7 +26,6 @@ public class TenantsMaintainer extends Maintainer {
     }
 
     @Override
-    // Delete unused tenants that were created more than ttlForUnusedTenant ago
     protected void maintain() {
         applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, Instant.now());
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ZooKeeperDataMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ZooKeeperDataMaintainer.java
index c906423d991..625665312fd 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ZooKeeperDataMaintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ZooKeeperDataMaintainer.java
@@ -1,5 +1,5 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
-        package com.yahoo.vespa.config.server.maintenance;
+package com.yahoo.vespa.config.server.maintenance;
 
 import com.yahoo.path.Path;
 import com.yahoo.vespa.config.server.ApplicationRepository;
@@ -9,6 +9,8 @@ import java.time.Duration;
 
 /**
  * Removes unused zookeeper data (for now only data used by old file distribution code is removed)
+ *
+ * @author hmusum
  */
 public class ZooKeeperDataMaintainer extends Maintainer {
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSessionRepo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSessionRepo.java
index dbb581eb16d..acfd81b33bf 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSessionRepo.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSessionRepo.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.session;
 
-import com.yahoo.concurrent.ThreadFactoryFactory;
 import com.yahoo.log.LogLevel;
 import com.yahoo.path.Path;
 import com.yahoo.transaction.NestedTransaction;
@@ -13,13 +12,10 @@ import com.yahoo.vespa.curator.Curator;
 import java.io.File;
 import java.io.FilenameFilter;
 import java.time.Clock;
-import java.time.Duration;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
@@ -32,11 +28,7 @@ public class LocalSessionRepo extends SessionRepo<LocalSession> {
 
     private static final Logger log = Logger.getLogger(LocalSessionRepo.class.getName());
     private static final FilenameFilter sessionApplicationsFilter = (dir, name) -> name.matches("\\d+");
-    private static final Duration delay = Duration.ofMinutes(5);
 
-    // One executor for all instances of this class
-    private static final ScheduledExecutorService purgeOldSessionsExecutor =
-            new ScheduledThreadPoolExecutor(1, ThreadFactoryFactory.getDaemonThreadFactory("purge-old-sessions"));
     private final Map<Long, LocalSessionStateWatcher> sessionStateWatchers = new HashMap<>();
     private final long sessionLifetime; // in seconds
     private final Clock clock;
@@ -46,7 +38,6 @@ public class LocalSessionRepo extends SessionRepo<LocalSession> {
                             Clock clock, long sessionLifeTime, Curator curator) {
         this(clock, curator, sessionLifeTime);
         loadSessions(tenantFileSystemDirs.sessionsPath(), loader);
-        purgeOldSessionsExecutor.scheduleWithFixedDelay(this::purgeOldSessions, delay.getSeconds(), delay.getSeconds(), TimeUnit.SECONDS);
     }
 
     // Constructor public only for testing
@@ -85,7 +76,6 @@ public class LocalSessionRepo extends SessionRepo<LocalSession> {
         }
     }
 
-    // public for testing
     public void purgeOldSessions() {
         log.log(LogLevel.DEBUG, "Purging old sessions");
         try {
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
index 18e2525f61a..b0320dad88b 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
@@ -1,11 +1,14 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server;
 
+import com.google.common.io.Files;
+import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.component.Version;
 import com.yahoo.component.Vtag;
 import com.yahoo.config.model.application.provider.FilesApplicationPackage;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
+import com.yahoo.config.provision.Deployment;
 import com.yahoo.config.provision.Environment;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.Provisioner;
@@ -13,6 +16,7 @@ import com.yahoo.config.provision.TenantName;
 import com.yahoo.io.IOUtils;
 import com.yahoo.test.ManualClock;
 import com.yahoo.text.Utf8;
+import com.yahoo.vespa.config.server.deploy.DeployTester;
 import com.yahoo.vespa.config.server.http.SessionHandlerTest;
 import com.yahoo.vespa.config.server.http.v2.PrepareResult;
 import com.yahoo.vespa.config.server.session.LocalSession;
@@ -31,12 +35,16 @@ import java.io.IOException;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
+import java.util.Optional;
 import java.util.Set;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertThat;
@@ -234,6 +242,45 @@ public class ApplicationRepositoryTest {
         assertFalse(applicationRepository.deleteApplicationLegacy(applicationId()));
     }
 
+    @Test
+    public void testDeletingInactiveSessions() {
+        ManualClock clock = new ManualClock(Instant.now());
+        ConfigserverConfig configserverConfig = new ConfigserverConfig(new ConfigserverConfig.Builder()
+                                                                               .configServerDBDir(Files.createTempDir()
+                                                                                                          .getAbsolutePath())
+                                                                               .configDefinitionsDir(Files.createTempDir()
+                                                                                                             .getAbsolutePath())
+                                                                               .sessionLifetime(60));
+        DeployTester tester = new DeployTester(configserverConfig, clock);
+        tester.deployApp("src/test/apps/app", "myapp", Instant.now()); // session 2 (numbering starts at 2)
+
+        clock.advance(Duration.ofSeconds(10));
+        Optional<Deployment> deployment2 = tester.redeployFromLocalActive();
+
+        assertTrue(deployment2.isPresent());
+        deployment2.get().activate(); // session 3
+        long activeSessionId = tester.tenant().getApplicationRepo().getSessionIdForApplication(tester.applicationId());
+
+        clock.advance(Duration.ofSeconds(10));
+        Optional<com.yahoo.config.provision.Deployment> deployment3 = tester.redeployFromLocalActive();
+        assertTrue(deployment3.isPresent());
+        deployment3.get().prepare();  // session 4 (not activated)
+
+        LocalSession deployment3session = ((com.yahoo.vespa.config.server.deploy.Deployment) deployment3.get()).session();
+        assertNotEquals(activeSessionId, deployment3session);
+        // No change to active session id
+        assertEquals(activeSessionId, tester.tenant().getApplicationRepo().getSessionIdForApplication(tester.applicationId()));
+        assertEquals(3, tester.tenant().getLocalSessionRepo().listSessions().size());
+
+        clock.advance(Duration.ofHours(1)); // longer than session lifetime
+
+        // All sessions except 3 should be removed after the call to deleteOldSessions
+        tester.applicationRepository().deleteOldSessions();
+        final Collection<LocalSession> sessions = tester.tenant().getLocalSessionRepo().listSessions();
+        assertEquals(1, sessions.size());
+        assertEquals(3, new ArrayList<>(sessions).get(0).getSessionId());
+    }
+
     private PrepareResult prepareAndActivateApp(File application) throws IOException {
         FilesApplicationPackage appDir = FilesApplicationPackage.fromFile(application);
         ApplicationId applicationId = applicationId();
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/RedeployTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/RedeployTest.java
index 9bc9a93e9fa..c6f4df74049 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/RedeployTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/RedeployTest.java
@@ -1,28 +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.vespa.config.server.deploy;
 
-import com.google.common.io.Files;
-import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.model.api.ModelFactory;
 import com.yahoo.config.provision.ApplicationId;
 import com.yahoo.config.provision.ApplicationName;
 import com.yahoo.config.provision.InstanceName;
 import com.yahoo.config.provision.Version;
-import com.yahoo.test.ManualClock;
-import com.yahoo.vespa.config.server.session.LocalSession;
 import org.junit.Test;
 
 import java.time.Clock;
-import java.time.Duration;
 import java.time.Instant;
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
 /**
@@ -61,43 +54,4 @@ public class RedeployTest {
         assertFalse(tester.redeployFromLocalActive(id).isPresent());
     }
 
-    @Test
-    public void testPurgingOfOldNonActiveDeployments() {
-        ManualClock clock = new ManualClock(Instant.now());
-        ConfigserverConfig configserverConfig = new ConfigserverConfig(new ConfigserverConfig.Builder()
-                                                                               .configServerDBDir(Files.createTempDir()
-                                                                                                       .getAbsolutePath())
-                                                                               .configDefinitionsDir(Files.createTempDir()
-                                                                                                          .getAbsolutePath())
-                                                                               .sessionLifetime(60));
-        DeployTester tester = new DeployTester(configserverConfig, clock);
-        tester.deployApp("src/test/apps/app", "myapp", Instant.now()); // session 2 (numbering starts at 2)
-
-        clock.advance(Duration.ofSeconds(10));
-        Optional<com.yahoo.config.provision.Deployment> deployment2 = tester.redeployFromLocalActive();
-
-        assertTrue(deployment2.isPresent());
-        deployment2.get().activate(); // session 3
-        long activeSessionId = tester.tenant().getApplicationRepo().getSessionIdForApplication(tester.applicationId());
-
-        clock.advance(Duration.ofSeconds(10));
-        Optional<com.yahoo.config.provision.Deployment> deployment3 = tester.redeployFromLocalActive();
-        assertTrue(deployment3.isPresent());
-        deployment3.get().prepare();  // session 4 (not activated)
-
-        LocalSession deployment3session = ((Deployment) deployment3.get()).session();
-        assertNotEquals(activeSessionId, deployment3session);
-        // No change to active session id
-        assertEquals(activeSessionId, tester.tenant().getApplicationRepo().getSessionIdForApplication(tester.applicationId()));
-        assertEquals(3, tester.tenant().getLocalSessionRepo().listSessions().size());
-
-        clock.advance(Duration.ofHours(1)); // longer than session lifetime
-
-        // All sessions except 3 should be removed after the call to purgeOldSessions
-        tester.tenant().getLocalSessionRepo().purgeOldSessions();
-        final Collection<LocalSession> sessions = tester.tenant().getLocalSessionRepo().listSessions();
-        assertEquals(1, sessions.size());
-        assertEquals(3, new ArrayList<>(sessions).get(0).getSessionId());
-    }
-
 }