diff options
author | Harald Musum <musum@yahoo-inc.com> | 2018-01-16 09:17:45 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-01-16 09:17:45 +0100 |
commit | 22b1a85a65c88559446a6919bb6828ed2665e315 (patch) | |
tree | dd4424473e1a7f08edeb15d50c30391f22270ff5 /configserver | |
parent | d4702a8342176438a2d5365f040ba42fb41ad75a (diff) | |
parent | b27fedc786c3faa9961969cb34cb35dd6de2d327 (diff) |
Merge pull request #4665 from vespa-engine/hmusum/purge-old-sessions-as-a-maintenance-task
Purge old session in its own thread instead of every time we add a se…
Diffstat (limited to 'configserver')
3 files changed, 25 insertions, 31 deletions
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 bbb9aa58e05..af809bea127 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 @@ -7,8 +7,11 @@ import com.yahoo.vespa.config.server.deploy.TenantFileSystemDirs; import java.io.File; import java.io.FilenameFilter; import java.time.Clock; +import java.time.Duration; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.logging.Logger; @@ -20,9 +23,10 @@ import java.util.logging.Logger; 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(1); - private final static FilenameFilter sessionApplicationsFilter = (dir, name) -> name.matches("\\d+"); - + private final ScheduledExecutorService purgeOldSessionsExecutor = new ScheduledThreadPoolExecutor(1); private final long sessionLifetime; // in seconds private final Clock clock; @@ -30,6 +34,7 @@ public class LocalSessionRepo extends SessionRepo<LocalSession> { Clock clock, long sessionLifeTime) { this(clock, sessionLifeTime); loadSessions(tenantFileSystemDirs.sessionsPath(), loader); + purgeOldSessionsExecutor.scheduleWithFixedDelay(this::purgeOldSessions, delay.getSeconds(), delay.getSeconds(), TimeUnit.SECONDS); } // Constructor public only for testing @@ -58,18 +63,18 @@ public class LocalSessionRepo extends SessionRepo<LocalSession> { } } - @Override - public synchronized void addSession(LocalSession session) { - purgeOldSessions(); - super.addSession(session); - } - - private void purgeOldSessions() { - List<LocalSession> sessions = new ArrayList<>(listSessions()); - for (LocalSession candidate : sessions) { - if (hasExpired(candidate) && !isActiveSession(candidate)) { - deleteSession(candidate); + // public for testing + public void purgeOldSessions() { + try { + List<LocalSession> sessions = new ArrayList<>(listSessions()); + for (LocalSession candidate : sessions) { + if (hasExpired(candidate) && !isActiveSession(candidate)) { + deleteSession(candidate); + } } + // Make sure to catch here, to avoid executor just dying in case of issues ... + } catch (Throwable e) { + log.log(LogLevel.WARNING, "Error when purging old sessions ", e); } } 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 50e8c711330..fd023a19617 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 @@ -10,10 +10,8 @@ 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 com.yahoo.vespa.config.server.session.Session; import org.junit.Test; -import java.io.IOException; import java.time.Clock; import java.time.Duration; import java.time.Instant; @@ -21,8 +19,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Optional; -import java.util.Set; -import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -37,7 +33,7 @@ import static org.junit.Assert.assertTrue; public class RedeployTest { @Test - public void testRedeploy() throws InterruptedException, IOException { + public void testRedeploy() { DeployTester tester = new DeployTester("src/test/apps/app"); tester.deployApp("myapp", Instant.now()); Optional<com.yahoo.config.provision.Deployment> deployment = tester.redeployFromLocalActive(); @@ -66,7 +62,7 @@ public class RedeployTest { } @Test - public void testRedeployWillPurgeOldNonActiveDeployments() { + public void testPurgingOfOldNonActiveDeployments() { ManualClock clock = new ManualClock(Instant.now()); ConfigserverConfig configserverConfig = new ConfigserverConfig(new ConfigserverConfig.Builder() .configServerDBDir(Files.createTempDir() @@ -97,18 +93,11 @@ public class RedeployTest { clock.advance(Duration.ofHours(1)); // longer than session lifetime - // Need another deployment to get old sessions purged - Optional<com.yahoo.config.provision.Deployment> deployment4 = tester.redeployFromLocalActive(); - assertTrue(deployment4.isPresent()); - deployment4.get().activate(); // session 5 - - // Both session 2 (deactivated) and session 4 (never activated) should have been removed + // All sessions except 3 should be removed after the call to purgeOldSessions + tester.tenant().getLocalSessionRepo().purgeOldSessions(); final Collection<LocalSession> sessions = tester.tenant().getLocalSessionRepo().listSessions(); - System.out.println(sessions); - assertEquals(2, sessions.size()); - final Set<Long> sessionIds = sessions.stream().map(Session::getSessionId).collect(Collectors.toSet()); - assertTrue(sessionIds.contains(3L)); - assertTrue(sessionIds.contains(5L)); + assertEquals(1, sessions.size()); + assertEquals(3, new ArrayList<>(sessions).get(0).getSessionId()); } } 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 3d34d08edeb..ac68307f42d 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 @@ -27,7 +27,6 @@ import static org.junit.Assert.fail; /** * @author Ulf Lilleengen - * @since 5.1 */ public class LocalSessionRepoTest extends TestWithCurator { @@ -84,6 +83,7 @@ public class LocalSessionRepoTest extends TestWithCurator { 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)); |