From 20d4a83577900d92c96272585b41960ebb4d4e40 Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@yahooinc.com>
Date: Tue, 29 Mar 2022 14:29:45 +0200
Subject: Delete session immediately if prepare or activate fails

---
 .../vespa/config/server/deploy/Deployment.java      | 21 +++++++++++++++++++--
 .../server/session/SessionRepositoryTest.java       | 11 +++++++++--
 2 files changed, 28 insertions(+), 4 deletions(-)

(limited to 'configserver')

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
index e6159cbfa33..1540629359a 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java
@@ -26,8 +26,10 @@ import com.yahoo.vespa.config.server.application.ConfigNotConvergedException;
 import com.yahoo.vespa.config.server.configchange.ConfigChangeActions;
 import com.yahoo.vespa.config.server.configchange.ReindexActions;
 import com.yahoo.vespa.config.server.configchange.RestartActions;
+import com.yahoo.vespa.config.server.session.LocalSession;
 import com.yahoo.vespa.config.server.session.PrepareParams;
 import com.yahoo.vespa.config.server.session.Session;
+import com.yahoo.vespa.config.server.session.SessionRepository;
 import com.yahoo.vespa.config.server.tenant.Tenant;
 
 import java.time.Clock;
@@ -106,9 +108,15 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
         if (prepared) return;
         PrepareParams params = this.params.get();
         ApplicationId applicationId = params.getApplicationId();
+        SessionRepository sessionRepository = tenant.getSessionRepository();
         try (ActionTimer timer = applicationRepository.timerFor(applicationId, "deployment.prepareMillis")) {
-            this.configChangeActions = tenant.getSessionRepository().prepareLocalSession(session, deployLogger, params, clock.instant());
+            this.configChangeActions = sessionRepository.prepareLocalSession(session, deployLogger, params, clock.instant());
             this.prepared = true;
+        } catch (Exception e) {
+            LocalSession localSession = sessionRepository.getLocalSession(session.getSessionId());
+            log.log(Level.FINE, "Preparing session " + session.getSessionId() + " failed, deleting it");
+            sessionRepository.deleteLocalSession(localSession);
+            throw e;
         }
 
         waitForResourcesOrTimeout(params, session, provisioner);
@@ -126,7 +134,16 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
             TimeoutBudget timeoutBudget = params.getTimeoutBudget();
             timeoutBudget.assertNotTimedOut(() -> "Timeout exceeded when trying to activate '" + applicationId + "'");
 
-            Activation activation = applicationRepository.activate(session, applicationId, tenant, params.force());
+            Activation activation;
+            try {
+                activation = applicationRepository.activate(session, applicationId, tenant, params.force());
+            } catch (Exception e) {
+                SessionRepository sessionRepository = tenant.getSessionRepository();
+                LocalSession localSession = sessionRepository.getLocalSession(session.getSessionId());
+                log.log(Level.FINE, "Activating session " + session.getSessionId() + " failed, deleting it");
+                sessionRepository.deleteLocalSession(localSession);
+                throw e;
+            }
             activation.awaitCompletion(timeoutBudget.timeLeft());
             logActivatedMessage(applicationId, activation);
 
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
index f128b71dd76..40f0b9c6e71 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
@@ -57,6 +57,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * @author Ulf Lilleengen
@@ -195,7 +196,7 @@ public class SessionRepositoryTest {
         assertTrue(sessionRepository.getRemoteSessionsFromZooKeeper().isEmpty());
     }
 
-    @Test(expected = InvalidApplicationException.class)
+    @Test
     public void require_that_new_invalid_application_throws_exception() throws Exception {
         MockModelFactory failingFactory = new MockModelFactory();
         failingFactory.vespaVersion = new Version(1, 2, 0);
@@ -207,7 +208,13 @@ public class SessionRepositoryTest {
 
         setup(new ModelFactoryRegistry(List.of(okFactory, failingFactory)));
 
-        deploy();
+        Collection<LocalSession> sessions = sessionRepository.getLocalSessions();
+        try {
+            deploy();
+            fail("deployment should have failed");
+        } catch (InvalidApplicationException e) {
+            assertEquals(sessions, sessionRepository.getLocalSessions());
+        }
     }
 
     @Test
-- 
cgit v1.2.3