Revert "Delete expired non-active sessions"

5 files changed, 43 insertions, 42 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 bcdb2618bda..6226af08a09 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,7 +76,6 @@ 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;
@@ -677,16 +676,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     }
 
     public void deleteExpiredLocalSessions() {
-        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));
+        tenantRepository.getAllTenants().forEach(tenant -> tenant.getLocalSessionRepo().purgeOldSessions());
     }
 
     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 f9b851fe197..97c4530e447 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,8 +1,8 @@
 // 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;
 import com.yahoo.transaction.NestedTransaction;
 import com.yahoo.vespa.config.server.GlobalComponentRegistry;
@@ -10,21 +10,16 @@ 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 com.yahoo.vespa.flags.Flags;
-import com.yahoo.vespa.flags.LongFlag;
 
 import java.io.File;
 import java.io.FilenameFilter;
 import java.time.Clock;
-import java.time.Duration;
-import java.time.Instant;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
-import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -43,7 +38,6 @@ public class LocalSessionRepo extends SessionRepo<LocalSession> {
     private final Curator curator;
     private final Executor zkWatcherExecutor;
     private final TenantFileSystemDirs tenantFileSystemDirs;
-    private final LongFlag expiryTimeFlag;
 
     public LocalSessionRepo(TenantName tenantName, GlobalComponentRegistry componentRegistry, LocalSessionLoader loader) {
         this(tenantName, componentRegistry);
@@ -57,7 +51,6 @@ public class LocalSessionRepo extends SessionRepo<LocalSession> {
         this.sessionLifetime = componentRegistry.getConfigserverConfig().sessionLifetime();
         this.zkWatcherExecutor = command -> componentRegistry.getZkWatcherExecutor().execute(tenantName, command);
         this.tenantFileSystemDirs = new TenantFileSystemDirs(componentRegistry.getConfigServerDB(), tenantName);
-        this.expiryTimeFlag = Flags.CONFIGSERVER_LOCAL_SESSIONS_EXPIRY_INTERVAL_IN_DAYS.bindTo(componentRegistry.getFlagSource());
     }
 
     @Override
@@ -84,25 +77,13 @@ public class LocalSessionRepo extends SessionRepo<LocalSession> {
         }
     }
 
-    public void deleteExpiredSessions(Map<ApplicationId, Long> activeSessions) {
+    public void purgeOldSessions() {
         log.log(Level.FINE, "Purging old sessions");
         try {
-            for (LocalSession candidate : listSessions()) {
-                Instant createTime = Instant.ofEpochSecond(candidate.getCreateTime());
-                log.log(Level.FINE, "Candidate session for deletion: " + candidate.getSessionId() + ", created: " + createTime);
-
-                // Sessions with state other than ACTIVATED
+            List<LocalSession> sessions = new ArrayList<>(listSessions());
+            for (LocalSession candidate : sessions) {
                 if (hasExpired(candidate) && !isActiveSession(candidate)) {
                     deleteSession(candidate);
-                } else if (createTime.plus(Duration.ofDays(expiryTimeFlag.value())).isBefore(clock.instant())) {
-                    //  Sessions with state ACTIVATE, but which are not actually active
-                    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() + " created " +
-                                            createTime + " 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 0cc166dc6e3..3400504fb58 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 List<SESSIONTYPE> listSessions() {
+    public synchronized Collection<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 69132186abc..9232109a89b 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,14 +3,16 @@ 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;
@@ -20,6 +22,8 @@ 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;
@@ -32,6 +36,7 @@ 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
@@ -50,8 +55,10 @@ 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())
@@ -74,6 +81,30 @@ 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));
@@ -81,6 +112,10 @@ 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"
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index c328b5ae151..416b6f59345 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -253,11 +253,6 @@ public class Flags {
             "Expiry time for unused sessions in config server",
             "Takes effect on next run of config server maintainer SessionsMaintainer");
 
-    public static final UnboundLongFlag CONFIGSERVER_LOCAL_SESSIONS_EXPIRY_INTERVAL_IN_DAYS = defineLongFlag(
-            "configserver-local-sessions-expiry-interval-in-days", 21,
-            "Expiry time for expired local sessions in config server",
-            "Takes effect on next run of config server maintainer SessionsMaintainer");
-
     public static final UnboundBooleanFlag USE_CLOUD_INIT_FORMAT = defineFeatureFlag(
             "use-cloud-init", false,
             "Use the cloud-init format when provisioning hosts",