Merge pull request #15299 from vespa-engine/hmusum/set-state-for-sessions-when-deleting

Set state for session instead of deleting it when deleting an applica…

7 files changed, 22 insertions, 19 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 b22e386aac4..98772c73b49 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
@@ -484,14 +484,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
             Optional<Long> activeSession = tenantApplications.activeSessionOf(applicationId);
             if (activeSession.isEmpty()) return false;
 
-            // Deleting an application is done by deleting the remote session, other config
-            // servers will pick this up and clean up through the watcher in this class
-            try {
-                Session session = getRemoteSession(tenant, activeSession.get());
-                tenant.getSessionRepository().delete(session);
-            } catch (NotFoundException e) {
-                log.log(Level.INFO, TenantRepository.logPre(applicationId) + "Active session exists, but has not been deleted properly. Trying to cleanup");
-            }
+            Session session = getRemoteSession(tenant, activeSession.get());
+            tenant.getSessionRepository().createSetStatusTransaction(session, Session.Status.DELETE).commit();
 
             Curator curator = tenantRepository.getCurator();
             transaction.add(new ContainerEndpointsCache(tenant.getPath(), curator).delete(applicationId)); // TODO: Not unit tested
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java
index de5f1392242..95f00c5f0e9 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSession.java
@@ -42,7 +42,7 @@ public class RemoteSession extends Session {
     }
 
     @Override
-    Optional<ApplicationSet> applicationSet() { return applicationSet; }
+    public Optional<ApplicationSet> applicationSet() { return applicationSet; }
 
     public synchronized RemoteSession activated(ApplicationSet applicationSet) {
         Objects.requireNonNull(applicationSet, "applicationSet cannot be null");
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 89d4fb23002..9b74eac5631 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
@@ -74,7 +74,7 @@ public abstract class Session implements Comparable<Session>  {
      * The status of this session.
      */
     public enum Status {
-        NEW, PREPARE, ACTIVATE, DEACTIVATE, NONE;
+        NEW, PREPARE, ACTIVATE, DEACTIVATE, NONE, 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 59146c339d3..40cf4f31cf8 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
@@ -299,6 +299,8 @@ public class SessionRepository {
      * @param sessionId session id for the new session
      */
     public synchronized void sessionAdded(long sessionId) {
+        if (hasStatusDeleted(sessionId)) return;
+
         log.log(Level.FINE, () -> "Adding remote session " + sessionId);
         Session session = createRemoteSession(sessionId);
         if (session.getStatus() == Session.Status.NEW) {
@@ -308,6 +310,12 @@ public class SessionRepository {
         createLocalSessionFromDistributedApplicationPackage(sessionId);
     }
 
+    private boolean hasStatusDeleted(long sessionId) {
+        SessionZooKeeperClient sessionZKClient = createSessionZooKeeperClient(sessionId);
+        RemoteSession session = new RemoteSession(tenantName, sessionId, sessionZKClient);
+        return session.getStatus() == Session.Status.DELETE;
+    }
+
     void activate(RemoteSession session) {
         long sessionId = session.getSessionId();
         Curator.CompletionWaiter waiter = createSessionZooKeeperClient(sessionId).getActiveWaiter();
@@ -324,6 +332,7 @@ public class SessionRepository {
         long sessionId = remoteSession.getSessionId();
         // TODO: Change log level to FINE when debugging is finished
         log.log(Level.INFO, () -> remoteSession.logPre() + "Deactivating and deleting remote session " + sessionId);
+        createSetStatusTransaction(remoteSession, Session.Status.DELETE).commit();
         deleteRemoteSessionFromZooKeeper(remoteSession);
         remoteSessionCache.remove(sessionId);
         LocalSession localSession = getLocalSession(sessionId);
@@ -724,7 +733,7 @@ public class SessionRepository {
         return transaction;
     }
 
-    private Transaction createSetStatusTransaction(Session session, Session.Status status) {
+    public Transaction createSetStatusTransaction(Session session, Session.Status status) {
         return session.sessionZooKeeperClient.createWriteStatusTransaction(status);
     }
 
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 0f433f53c77..b27cdbdc341 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
@@ -49,6 +49,9 @@ public class SessionStateWatcher {
             case NEW:
             case NONE:
                 break;
+            case DELETE:
+                sessionRepository.deactivateAndUpdateCache(session);
+                break;
             case PREPARE:
                 createLocalSession(sessionId);
                 sessionRepository.prepareRemoteSession(session);
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 e45839bc00c..d153c63b537 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
@@ -322,16 +322,16 @@ public class ApplicationRepositoryTest {
             File sessionFile = new File(tenantFileSystemDirs.sessionsPath(), String.valueOf(sessionId));
             assertTrue(sessionFile.exists());
 
-            // Delete app and verify that it has been deleted from repos and provisioner
+            // Delete app and verify that it has been deleted from repos and provisioner and no application set exists
             assertTrue(applicationRepository.delete(applicationId()));
             assertNull(applicationRepository.getActiveSession(applicationId()));
-            assertNull(sessionRepository.getLocalSession(sessionId));
-            assertNull(sessionRepository.getLocalSession(sessionId));
+            assertEquals(Optional.empty(), sessionRepository.getRemoteSession(sessionId).applicationSet());
             assertTrue(provisioner.removed());
             assertEquals(tenant.getName(), provisioner.lastApplicationId().tenant());
             assertEquals(applicationId(), provisioner.lastApplicationId());
-            assertFalse(configCurator.exists(sessionNode));
-            assertFalse(sessionFile.exists());
+            assertTrue(configCurator.exists(sessionNode));
+            assertEquals(Session.Status.DELETE.name(), configCurator.getData(sessionNode + "/sessionState"));
+            assertTrue(sessionFile.exists());
 
             assertFalse(applicationRepository.delete(applicationId()));
         }
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java
index e9bf4f7c12e..49a40e7e33c 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandlerTest.java
@@ -63,7 +63,6 @@ import static com.yahoo.vespa.config.server.http.HandlerTest.assertHttpStatusCod
 import static com.yahoo.vespa.config.server.http.SessionHandlerTest.getRenderedString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
@@ -455,13 +454,11 @@ public class ApplicationHandlerTest {
         Tenant tenant = applicationRepository.getTenant(applicationId);
         long sessionId = tenant.getApplicationRepo().requireActiveSessionOf(applicationId);
         deleteAndAssertResponse(applicationId, Zone.defaultZone(), Response.Status.OK, null, fullAppIdInUrl);
-        assertNull(tenant.getSessionRepository().getLocalSession(sessionId));
     }
 
     private void deleteAndAssertOKResponse(Tenant tenant, ApplicationId applicationId) throws IOException {
         long sessionId = tenant.getApplicationRepo().requireActiveSessionOf(applicationId);
         deleteAndAssertResponse(applicationId, Zone.defaultZone(), Response.Status.OK, null, true);
-        assertNull(tenant.getSessionRepository().getLocalSession(sessionId));
     }
 
     private void deleteAndAssertResponse(ApplicationId applicationId, Zone zone, int expectedStatus, HttpErrorResponse.errorCodes errorCode, boolean fullAppIdInUrl) throws IOException {