Merge pull request #32927 from vespa-engine/hmusum/use-created-time-f…

…or-local-session-if-remote-session-is-missing Use created time for local session if remote session is missing
vespa-engine · Nov 22, 2024 · e261529 · e261529
2 parents 414ac2e + 97fed77
commit e261529
Showing 1 changed file with 38 additions and 21 deletions.
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
@@ -624,31 +624,44 @@ public void deleteExpiredRemoteAndLocalSessions(Predicate<Session> sessionIsActi
         for (Long sessionId : sessions) {
             try {
                 Session session = remoteSessionCache.get(sessionId);
-                if (session == null)
-                    session = new RemoteSession(tenantName, sessionId, createSessionZooKeeperClient(sessionId));
+                Instant createTime;
+                Optional<Instant> localSessionCreateTime = Optional.empty();
+                boolean deleteRemoteSession = true;
+                if (session == null) {
+                    // If remote session is missing (deleted from zookeeper) it will only be present in file system,
+                    // so use local session and its creation time from file system
+                    var localSession = getOptionalSessionFromFileSystem(sessionId);
+                    if (localSession.isEmpty()) continue;
+
+                    session = localSession.get();
+                    createTime = localSessionCreated((LocalSession) session);
+                    localSessionCreateTime= Optional.of(createTime);
+                    deleteRemoteSession = false;
+                } else {
+                    createTime = session.getCreateTime();
+                }
 
                 Optional<ApplicationId> applicationId = session.getOptionalApplicationId();
                 try (var ignored = lockApplication(applicationId)) {
                     Session.Status status = session.getStatus();
                     boolean activeForApplication = sessionIsActiveForApplication.test(session);
-                    log.log(Level.FINE, () -> "local session " + sessionId +
-                            ", status " + status + (status == UNKNOWN ? "" : ", activeForApplication " + activeForApplication));
                     if (status == ACTIVATE && activeForApplication) continue;
 
-                    Instant createTime = session.getCreateTime();
                     boolean hasExpired = hasExpired(createTime);
-                    log.log(Level.FINE, () -> "local session " + sessionId +
-                            ", status " + status + (status == UNKNOWN ? "" : ", created " + createTime +
-                            ", has expired: " + hasExpired));
+                    log.log(Level.FINE, "Session " + sessionId + ", status " + status + ", has expired: " + hasExpired);
                     if (! hasExpired) continue;
 
-                    log.log(Level.FINE, () -> "Remote session " + sessionId + " for " + tenantName + " has expired, deleting it");
-                    deleteRemoteSessionFromZooKeeper(session);
-                    deletedRemoteSessions++;
+                    log.log(Level.FINE, "session " + sessionId + ", status " + status +
+                            ", remote session created " + createTime +
+                            ", local session created " + localSessionCreateTime);
+                    if (deleteRemoteSession) {
+                        log.log(Level.FINE, () -> "Remote session " + sessionId + " for " + tenantName + " has expired, deleting it");
+                        deleteRemoteSessionFromZooKeeper(session);
+                        deletedRemoteSessions++;
+                    }
 
-                    var localSessionCanBeDeleted = canBeDeleted(sessionId, status, createTime, activeForApplication);
-                    if (localSessionCanBeDeleted) {
-                        log.log(Level.FINE, () -> "Expired local session " + sessionId + " can be deleted");
+                    if (localSessionCanBeDeleted(status, createTime, activeForApplication)) {
+                        log.log(Level.FINE, () -> "Local session " + sessionId + " for " + tenantName + " has expired, deleting it");
                         deleteLocalSession(sessionId);
                         deletedLocalSessions++;
                     }
@@ -698,21 +711,25 @@ private boolean hasExpired(Instant created) {
 
     private long sessionLifeTimeInSeconds() { return configserverConfig.sessionLifetime(); }
 
-    private boolean canBeDeleted(long sessionId, Session.Status status, Instant createTime, boolean activeForApplication) {
-        // Delete Sessions with state other than UNKNOWN or ACTIVATE or old sessions in UNKNOWN state
-        if ( ! List.of(UNKNOWN, ACTIVATE).contains(status) || oldSessionDirWithUnknownStatus(sessionId, status))
+    private boolean localSessionCanBeDeleted(Session.Status status, Instant createTime, boolean activeForApplication) {
+        // Delete sessions with state other than UNKNOWN or ACTIVATE or old sessions in UNKNOWN state
+        if ( ! List.of(UNKNOWN, ACTIVATE).contains(status) || oldSessionDirWithUnknownStatus(createTime, status))
             return true;
 
         // This might happen if remote session is gone, but local session is not
         return isOldAndCanBeDeleted(createTime) && !activeForApplication;
     }
 
-    private boolean oldSessionDirWithUnknownStatus(long sessionId, Session.Status status) {
+    private boolean oldSessionDirWithUnknownStatus(Instant created, Session.Status status) {
         Duration expiryTime = Duration.ofHours(configserverConfig.keepSessionsWithUnknownStatusHours());
-        File sessionDir = tenantFileSystemDirs.getUserApplicationDir(sessionId);
-        return sessionDir.exists()
+        return created != Instant.EPOCH // We don't know anything about creation time for this session
                 && status == UNKNOWN
-                && created(sessionDir).plus(expiryTime).isBefore(clock.instant());
+                && created.plus(expiryTime).isBefore(clock.instant());
+    }
+
+    private Instant localSessionCreated(LocalSession session) {
+        File sessionDir = tenantFileSystemDirs.getUserApplicationDir(session.getSessionId());
+        return sessionDir.exists() ? created(sessionDir) : Instant.EPOCH;
     }
 
     private Set<Long> findNewSessionsInFileSystem() {