diff options
3 files changed, 25 insertions, 20 deletions
diff --git a/configdefinitions/src/vespa/configserver.def b/configdefinitions/src/vespa/configserver.def index da383ecb6f5..b13c9ba1932 100644 --- a/configdefinitions/src/vespa/configserver.def +++ b/configdefinitions/src/vespa/configserver.def @@ -55,9 +55,10 @@ loadBalancerAddress string default="" athenzDnsSuffix string default="" ztsUrl string default="" -# Maintainers +# Maintenance settings maintainerIntervalMinutes int default=30 keepUnusedFileReferencesMinutes int default=300 +keepSessionsWithUnknownStatusHours int default=48 # Bootstrapping # How long bootstrapping can take before giving up (in seconds) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java index 059d192e7d2..7d10acaf3ae 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java @@ -575,32 +575,31 @@ public class SessionRepository { // ---------------- Common stuff ---------------------------------------------------------------- public void deleteExpiredSessions(Map<ApplicationId, Long> activeSessions) { - log.log(Level.FINE, () -> "Purging old sessions for tenant '" + tenantName + "'"); + log.log(Level.FINE, () -> "Deleting expired local sessions for tenant '" + tenantName + "'"); Set<LocalSession> toDelete = new HashSet<>(); Set<Long> newSessions = findNewSessionsInFileSystem(); try { for (LocalSession candidate : getLocalSessionsFromFileSystem()) { // Skip sessions newly added (we might have a session in the file system, but not in ZooKeeper, // we don't want to touch any of them) - if (newSessions.contains(candidate.getSessionId())) { - log.log(Level.FINE, () -> "Skipping expiring newly created session " + candidate.getSessionId()); + if (newSessions.contains(candidate.getSessionId())) continue; - } Instant createTime = candidate.getCreateTime(); - log.log(Level.FINE, () -> "Candidate session for deletion: " + candidate.getSessionId() + ", created: " + createTime); + log.log(Level.FINE, () -> "Candidate local session for deletion: " + candidate.getSessionId() + + ", created: " + createTime + ", state " + candidate.getStatus() + ", can be deleted: " + canBeDeleted(candidate)); - if (hasExpired(candidate) && canBeDeleted(candidate)) { + if (hasExpired(createTime) && canBeDeleted(candidate)) { toDelete.add(candidate); } else if (createTime.plus(Duration.ofDays(1)).isBefore(clock.instant())) { - // Sessions with state ACTIVATE, but which are not actually active Optional<ApplicationId> applicationId = candidate.getOptionalApplicationId(); if (applicationId.isEmpty()) continue; + Long activeSession = activeSessions.get(applicationId.get()); if (activeSession == null || activeSession != candidate.getSessionId()) { toDelete.add(candidate); - log.log(Level.INFO, "Deleted inactive session " + candidate.getSessionId() + " created " + - createTime + " for '" + applicationId + "'"); + log.log(Level.FINE, () -> "Will delete inactive session " + candidate.getSessionId() + " created " + + createTime + " for '" + applicationId + "'"); } } } @@ -614,21 +613,22 @@ public class SessionRepository { log.log(Level.FINE, () -> "Done purging old sessions"); } - private boolean hasExpired(LocalSession candidate) { - return candidate.getCreateTime().plus(sessionLifetime).isBefore(clock.instant()); + private boolean hasExpired(Instant created) { + return created.plus(sessionLifetime).isBefore(clock.instant()); } // Sessions with state other than UNKNOWN or ACTIVATE or old sessions in UNKNOWN state private boolean canBeDeleted(LocalSession candidate) { - return ! List.of(Session.Status.UNKNOWN, Session.Status.ACTIVATE).contains(candidate.getStatus()) - || oldSessionDirWithNonExistingSession(candidate); + return ( ! List.of(Session.Status.UNKNOWN, Session.Status.ACTIVATE).contains(candidate.getStatus())) + || oldSessionDirWithUnknownStatus(candidate); } - private boolean oldSessionDirWithNonExistingSession(LocalSession session) { + private boolean oldSessionDirWithUnknownStatus(LocalSession session) { + Duration expiryTime = Duration.ofHours(configserverConfig.keepSessionsWithUnknownStatusHours()); File sessionDir = tenantFileSystemDirs.getUserApplicationDir(session.getSessionId()); return sessionDir.exists() && session.getStatus() == Session.Status.UNKNOWN - && created(sessionDir).plus(Duration.ofDays(30)).isBefore(clock.instant()); + && created(sessionDir).plus(expiryTime).isBefore(clock.instant()); } private Set<Long> findNewSessionsInFileSystem() { 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 ee2a979be7a..46cf84e19c1 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 @@ -464,7 +464,8 @@ public class ApplicationRepositoryTest { // and check that expiring local sessions still works int sessionId = 6; TenantName tenantName = tester.tenant().getName(); - java.nio.file.Path dir = Files.createDirectory(new TenantFileSystemDirs(serverdb, tenantName).getUserApplicationDir(sessionId).toPath()); + Instant session6CreateTime = clock.instant(); + Files.createDirectory(new TenantFileSystemDirs(serverdb, tenantName).getUserApplicationDir(sessionId).toPath()); LocalSession localSession2 = new LocalSession(tenant1, sessionId, FilesApplicationPackage.fromFile(testApp), @@ -479,7 +480,7 @@ public class ApplicationRepositoryTest { // so will be candidate for expiry) Session session = sessionRepository.createRemoteSession(7); sessionRepository.createSetStatusTransaction(session, Session.Status.UNKNOWN); - assertEquals(2, sessionRepository.getLocalSessions().size()); // Still 2, no new local session + assertEquals(2, sessionRepository.getLocalSessions().size()); // Still 2, no new local session // Check that trying to expire local session when there exists a local session without any data in zookeeper // should not delete session if this is a new file ... @@ -489,8 +490,11 @@ public class ApplicationRepositoryTest { clock.advance(Duration.ofSeconds(60)); deleteExpiredLocalSessionsAndAssertNumberOfSessions(1, tester, sessionRepository); - // Set older created timestamp for session dir for local session without any data in zookeeper, should be deleted - setCreatedTime(dir, Instant.now().minus(Duration.ofDays(31))); + // Reset clock to the time session was created, session should NOT be deleted + clock.setInstant(session6CreateTime); + deleteExpiredLocalSessionsAndAssertNumberOfSessions(1, tester, sessionRepository); + // Advance time, session SHOULD be deleted + clock.advance(Duration.ofHours(configserverConfig.keepSessionsWithUnknownStatusHours()).plus(Duration.ofMinutes(1))); deleteExpiredLocalSessionsAndAssertNumberOfSessions(0, tester, sessionRepository); } |