diff options
author | Harald Musum <musum@verizonmedia.com> | 2020-05-25 16:06:57 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-05-25 16:06:57 +0200 |
commit | 1587069650e1ae2eef712d47b6e0454e65c5db1b (patch) | |
tree | 6014db5a589803166fdfcdecc17f58d9c5434f42 /configserver | |
parent | 05a8443b0ab6e2acb2c49f15df455b86f1638653 (diff) | |
parent | 4841eea31c50de7df6928b7f889b00fefcfc53a2 (diff) |
Merge pull request #13359 from vespa-engine/hmusum/delete-expired-non-active-sessions
Delete expired non-active sessions
Diffstat (limited to 'configserver')
4 files changed, 37 insertions, 43 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..f9b851fe197 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,16 +10,21 @@ 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; /** @@ -38,6 +43,7 @@ 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); @@ -51,6 +57,7 @@ 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 @@ -77,13 +84,25 @@ 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) { + 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 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 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" |