Delete expired non-active sessions

Due to a bug fixed in https://github.com/vespa-engine/vespa/pull/13338
we ended up with sessions that are in state ACTIVATED in zookeeper,
but that was not the latest active session.  These would not be
cleaned up, the changes here will delete these sessions (if they are
expired and not the current active session for an application).

Also deletes a test that was covered by tests in ApplicationRepositoryTest.

4 files changed, 26 insertions, 44 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 6226af08a09..bcdb2618bda 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
@@ -76,6 +76,7 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -676,7 +677,16 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     }
 
     public void deleteExpiredLocalSessions() {
-        tenantRepository.getAllTenants().forEach(tenant -> tenant.getLocalSessionRepo().purgeOldSessions());
+        Map<Tenant, List<LocalSession>> sessionsPerTenant = new HashMap<>();
+        tenantRepository.getAllTenants().forEach(tenant -> sessionsPerTenant.put(tenant, tenant.getLocalSessionRepo().listSessions()));
+
+        Set<ApplicationId> applicationIds = new HashSet<>();
+        sessionsPerTenant.values().forEach(sessionList -> sessionList.forEach(s -> applicationIds.add(s.getApplicationId())));
+
+        Map<ApplicationId, Long> activeSessions = new HashMap<>();
+        applicationIds.forEach(applicationId -> activeSessions.put(applicationId, getActiveSession(applicationId).getSessionId()));
+
+        sessionsPerTenant.keySet().forEach(tenant -> tenant.getLocalSessionRepo().deleteExpiredSessions(activeSessions));
     }
 
     public int deleteExpiredRemoteSessions(Duration expiryTime) {
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 97c4530e447..a112eb7087c 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,6 +1,7 @@
 // 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.config.provision.ApplicationId;
 import com.yahoo.config.provision.TenantName;
 import java.util.logging.Level;
 import com.yahoo.path.Path;
@@ -10,6 +11,7 @@ import com.yahoo.vespa.config.server.deploy.TenantFileSystemDirs;
 import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.config.server.zookeeper.ConfigCurator;
 import com.yahoo.vespa.curator.Curator;
+import org.glassfish.jersey.jaxb.internal.XmlJaxbElementProvider;
 
 import java.io.File;
 import java.io.FilenameFilter;
@@ -77,13 +79,18 @@ public class LocalSessionRepo extends SessionRepo<LocalSession> {
         }
     }
 
-    public void purgeOldSessions() {
+    public void deleteExpiredSessions(Map<ApplicationId, Long> activeSessions) {
         log.log(Level.FINE, "Purging old sessions");
         try {
-            List<LocalSession> sessions = new ArrayList<>(listSessions());
-            for (LocalSession candidate : sessions) {
-                if (hasExpired(candidate) && !isActiveSession(candidate)) {
-                    deleteSession(candidate);
+            for (LocalSession candidate : listSessions()) {
+                log.log(Level.FINE, "Candidate session for deletion: " + candidate.getSessionId() + ", created: " + candidate.getCreateTime());
+                if (hasExpired(candidate)) {
+                    ApplicationId applicationId = candidate.getApplicationId();
+                    Long activeSession = activeSessions.get(applicationId);
+                    if (activeSession == null || activeSession != candidate.getSessionId()) {
+                        deleteSession(candidate);
+                        log.log(Level.INFO, "Deleted inactive session " + candidate.getSessionId() + " for '" + applicationId + "'");
+                    }
                 }
             }
             // Make sure to catch here, to avoid executor just dying in case of issues ...
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepo.java
index 3400504fb58..0cc166dc6e3 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepo.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepo.java
@@ -2,8 +2,8 @@
 package com.yahoo.vespa.config.server.session;
 
 import java.util.ArrayList;
-import java.util.Collection;
 import java.util.HashMap;
+import java.util.List;
 
 /**
  * A generic session repository that can store any type of session that extends the abstract interface.
@@ -37,7 +37,7 @@ public class SessionRepo<SESSIONTYPE extends Session> {
         return sessions.get(id);
     }
 
-    public synchronized Collection<SESSIONTYPE> listSessions() {
+    public synchronized List<SESSIONTYPE> listSessions() {
         return new ArrayList<>(sessions.values());
     }
     
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionRepoTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionRepoTest.java
index 9232109a89b..69132186abc 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionRepoTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionRepoTest.java
@@ -3,16 +3,14 @@ package com.yahoo.vespa.config.server.session;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.model.application.provider.FilesApplicationPackage;
-import com.yahoo.test.ManualClock;
 import com.yahoo.config.provision.TenantName;
+import com.yahoo.io.IOUtils;
 import com.yahoo.vespa.config.server.GlobalComponentRegistry;
 import com.yahoo.vespa.config.server.MockReloadHandler;
 import com.yahoo.vespa.config.server.TestComponentRegistry;
 import com.yahoo.vespa.config.server.application.TenantApplications;
-import com.yahoo.io.IOUtils;
 import com.yahoo.vespa.config.server.host.HostRegistry;
 import com.yahoo.vespa.config.server.http.SessionHandlerTest;
-
 import com.yahoo.vespa.curator.mock.MockCurator;
 import org.junit.Before;
 import org.junit.Rule;
@@ -22,8 +20,6 @@ import org.junit.rules.TemporaryFolder;
 import java.io.File;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.time.Duration;
-import java.time.Instant;
 
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
@@ -36,7 +32,6 @@ public class LocalSessionRepoTest {
 
     private File testApp = new File("src/test/apps/app");
     private LocalSessionRepo repo;
-    private ManualClock clock;
     private static final TenantName tenantName = TenantName.defaultName();
 
     @Rule
@@ -55,10 +50,8 @@ public class LocalSessionRepoTest {
             IOUtils.copyDirectory(testApp, sessionsPath.resolve("2").toFile());
             IOUtils.copyDirectory(testApp, sessionsPath.resolve("3").toFile());
         }
-        clock = new ManualClock(Instant.ofEpochSecond(1));
         GlobalComponentRegistry globalComponentRegistry = new TestComponentRegistry.Builder()
                 .curator(new MockCurator())
-                .clock(clock)
                 .configServerConfig(new ConfigserverConfig.Builder()
                                             .configServerDBDir(configserverDbDir.getAbsolutePath())
                                             .configDefinitionsDir(temporaryFolder.newFolder().getAbsolutePath())
@@ -81,30 +74,6 @@ public class LocalSessionRepoTest {
     }
 
     @Test
-    public void require_that_old_sessions_are_purged() {
-        clock.advance(Duration.ofSeconds(1));
-        assertNotNull(repo.getSession(1L));
-        assertNotNull(repo.getSession(2L));
-        assertNotNull(repo.getSession(3L));
-        clock.advance(Duration.ofSeconds(1));
-        assertNotNull(repo.getSession(1L));
-        assertNotNull(repo.getSession(2L));
-        assertNotNull(repo.getSession(3L));
-        clock.advance(Duration.ofSeconds(1));
-        addSession(4L, 6);
-        assertNotNull(repo.getSession(1L));
-        assertNotNull(repo.getSession(2L));
-        assertNotNull(repo.getSession(3L));
-        assertNotNull(repo.getSession(4L));
-        clock.advance(Duration.ofSeconds(1));
-        addSession(5L, 10);
-        repo.purgeOldSessions();
-        assertNull(repo.getSession(1L));
-        assertNull(repo.getSession(2L));
-        assertNull(repo.getSession(3L));
-    }
-
-    @Test
     public void require_that_all_sessions_are_deleted() {
         repo.close();
         assertNull(repo.getSession(1L));
@@ -112,10 +81,6 @@ public class LocalSessionRepoTest {
         assertNull(repo.getSession(3L));
     }
 
-    private void addSession(long sessionId, long createTime) {
-        repo.addSession(new SessionHandlerTest.MockSession(sessionId, FilesApplicationPackage.fromFile(testApp), createTime));
-    }
-
     @Test
     public void require_that_sessions_belong_to_a_tenant() {
         // tenant is "default"