diff options
author | Harald Musum <musum@verizonmedia.com> | 2021-07-16 11:07:41 +0200 |
---|---|---|
committer | Harald Musum <musum@verizonmedia.com> | 2021-07-16 11:07:41 +0200 |
commit | 27e50797702593777aca83848277dba727980e91 (patch) | |
tree | ac759f46f9449770878ff7226bf916001ed5984e | |
parent | e6d8a8869e9c44d1b8180f1390a09df896e1c299 (diff) |
Do not expire sessions with state UNKNOWN
5 files changed, 18 insertions, 10 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java index 542b54d877e..b8f688e4998 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java @@ -77,7 +77,7 @@ public abstract class Session implements Comparable<Session> { * The status of this session. */ public enum Status { - NEW, PREPARE, ACTIVATE, DEACTIVATE, NONE, DELETE; + NEW, PREPARE, ACTIVATE, DEACTIVATE, UNKNOWN, DELETE; public static Status parse(String data) { for (Status status : Status.values()) { 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 de2e4d54eec..3c8437cfe18 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 @@ -551,8 +551,7 @@ public class SessionRepository { Instant createTime = candidate.getCreateTime(); log.log(Level.FINE, () -> "Candidate session for deletion: " + candidate.getSessionId() + ", created: " + createTime); - // Sessions with state other than ACTIVATE - if (hasExpired(candidate) && !isActiveSession(candidate)) { + if (hasExpired(candidate) && 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 @@ -580,8 +579,9 @@ public class SessionRepository { return candidate.getCreateTime().plus(sessionLifetime).isBefore(clock.instant()); } - private boolean isActiveSession(LocalSession candidate) { - return candidate.getStatus() == Session.Status.ACTIVATE; + // Sessions with state other than UNKNOWN or ACTIVATE + private boolean canBeDeleted(LocalSession candidate) { + return ! List.of(Session.Status.UNKNOWN, Session.Status.ACTIVATE).contains(candidate.getStatus()); } private void ensureSessionPathDoesNotExist(long sessionId) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java index 880519b3d61..e198307f242 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java @@ -47,7 +47,7 @@ public class SessionStateWatcher { long sessionId = session.getSessionId(); switch (newStatus) { case NEW: - case NONE: + case UNKNOWN: break; case DELETE: sessionRepository.deactivateAndUpdateCache(session); @@ -86,7 +86,7 @@ public class SessionStateWatcher { private void nodeChanged() { zkWatcherExecutor.execute(() -> { - Status newStatus = Status.NONE; + Status newStatus = Status.UNKNOWN; try { ChildData node = fileCache.getCurrentData(); if (node != null) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java index c7b19968f0a..1b23c3bf37a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionZooKeeperClient.java @@ -93,11 +93,11 @@ public class SessionZooKeeperClient { public Session.Status readStatus() { try { Optional<byte[]> data = curator.getData(sessionStatusPath); - return data.map(d -> Session.Status.parse(Utf8.toString(d))).orElse(Session.Status.NONE); + return data.map(d -> Session.Status.parse(Utf8.toString(d))).orElse(Session.Status.UNKNOWN); } catch (Exception e) { log.log(Level.INFO, "Failed to read session status at " + sessionStatusPath.getAbsolute() + ", will assume session has been removed: ", e); - return Session.Status.NONE; + return Session.Status.UNKNOWN; } } 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 8f8005894e5..8d659e02a52 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 @@ -477,9 +477,17 @@ public class ApplicationRepositoryTest { sessionRepository.addLocalSession(localSession2); assertEquals(2, sessionRepository.getLocalSessions().size()); + // Create a session, set status to UNKNOWN, we don't want to expire those (creation time is then EPOCH, + // so will be candidate for expiry) + Session session = sessionRepository.createRemoteSession(7); + sessionRepository.createSetStatusTransaction(session, Session.Status.UNKNOWN); + + sessionRepository.addLocalSession(localSession2); + assertEquals(2, sessionRepository.getLocalSessions().size()); + // Check that trying to expire local session when there exists a local session with no zookeeper data works tester.applicationRepository().deleteExpiredLocalSessions(); - assertEquals(1, sessionRepository.getLocalSessions().size()); + assertEquals(2, sessionRepository.getLocalSessions().size()); // Check that trying to expire when there are no active sessions works tester.applicationRepository().deleteExpiredLocalSessions(); |