Merge pull request #9531 from vespa-engine/jvenstad/config-super-model-last-take

Jvenstad/config super model last take

8 files changed, 93 insertions, 18 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ActivationConflictException.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ActivationConflictException.java
index 48023293cc4..d39168f9427 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ActivationConflictException.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ActivationConflictException.java
@@ -1,11 +1,18 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server;
 
+import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.vespa.config.server.application.TenantApplications;
+
 /**
  * Exception used when activation cannot be done because activation is for
  * an older session than the one that is active now or because current active
  * session has changed since the session to be activated was created
  *
+ * Also used when a redeployment is requested, but another deployment is aready ongoing,
+ * or when attempting to activate a deployment which is no longer supposed to be activated.
+ * @see TenantApplications#preparing(ApplicationId)
+ *
  * @author hmusum
  */
 public class ActivationConflictException extends RuntimeException {
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 3aea22ce8b2..cfe4de34ab5 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
@@ -202,6 +202,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         Optional<ApplicationSet> currentActiveApplicationSet = getCurrentActiveApplicationSet(tenant, applicationId);
         Slime deployLog = createDeployLog();
         DeployLogger logger = new DeployHandlerLogger(deployLog.get().setArray("log"), prepareParams.isVerbose(), applicationId);
+
         ConfigChangeActions actions = session.prepare(logger, prepareParams, currentActiveApplicationSet, tenant.getPath(), now);
         logConfigChangeActions(actions, logger);
         log.log(LogLevel.INFO, TenantRepository.logPre(applicationId) + "Session " + sessionId + " prepared successfully. ");
@@ -573,7 +574,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
 
     public long createSession(ApplicationId applicationId, TimeoutBudget timeoutBudget, File applicationDirectory) {
         Tenant tenant = tenantRepository.getTenant(applicationId.tenant());
-        tenant.getApplicationRepo().createApplication(applicationId);
         LocalSessionRepo localSessionRepo = tenant.getLocalSessionRepo();
         SessionFactory sessionFactory = tenant.getSessionFactory();
         LocalSession session = sessionFactory.createSession(applicationDirectory, applicationId, timeoutBudget);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
index d27ef2bd9d5..91016e3c91b 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java
@@ -12,6 +12,7 @@ import com.yahoo.vespa.config.server.ReloadHandler;
 import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.Lock;
+import com.yahoo.vespa.curator.transaction.CuratorOperation;
 import com.yahoo.vespa.curator.transaction.CuratorOperations;
 import com.yahoo.vespa.curator.transaction.CuratorTransaction;
 import org.apache.curator.framework.CuratorFramework;
@@ -33,7 +34,8 @@ import java.util.stream.Collectors;
  * The applications of a tenant, backed by ZooKeeper.
  *
  * Each application is stored under /config/v2/tenants/&lt;tenant&gt;/applications/&lt;application&gt;,
- * the root contains the currently active session, if any. Locks for synchronising writes to these paths, and changes
+ * the root contains the currently active session, if any, and the /preparing child node contains the
+ * session ID of the session to activate next. Locks for synchronising writes to these paths, and changes
  * to the config of this application, are found under /config/v2/tenants/&lt;tenant&gt;/locks/&lt;application&gt;.
  *
  * @author Ulf Lilleengen
@@ -93,13 +95,18 @@ public class TenantApplications {
     }
 
     /**
-     * Returns a transaction which writes the given session id as the currently active for the given application.
+     * Returns a transaction which writes the given session id as the currently active for the given application,
+     * and clears the currently preparing session, as this is now becoming active.
      *
      * @param applicationId An {@link ApplicationId} that represents an active application.
      * @param sessionId Id of the session containing the application package for this id.
      */
     public Transaction createPutTransaction(ApplicationId applicationId, long sessionId) {
-        return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId)));
+        CuratorTransaction transaction = new CuratorTransaction(curator);
+        transaction.add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId)));
+        if (curator.exists(preparingPath(applicationId)))
+            transaction.add(CuratorOperations.delete(preparingPath(applicationId).getAbsolute()));
+        return transaction;
     }
 
     /**
@@ -111,6 +118,25 @@ public class TenantApplications {
         }
     }
 
+    /** Sets the given session id as the one currently being prepared for this application. */
+    public void prepare(ApplicationId id, long sessionId) {
+        try (Lock lock = lock(id)) {
+            if ( ! exists(id))
+                throw new IllegalStateException("Can't prepare for '" + id + "'; it doesn't exist");
+
+            curator.set(preparingPath(id), Long.toString(sessionId).getBytes());
+        }
+    }
+
+    /** Returns the id of the session currently being prepared for this application. */
+    public OptionalLong preparing(ApplicationId id) {
+        if ( ! exists(id))
+            throw new IllegalStateException("Application '" + id + "' can't have a prepared session; it doesn't exist");
+
+        return curator.getData(preparingPath(id)).map(Utf8::toString).map(Long::parseLong)
+                      .map(OptionalLong::of).orElse(OptionalLong.empty());
+    }
+
     /**
      * Return the active session id for a given application.
      *
@@ -185,6 +211,10 @@ public class TenantApplications {
         return applicationsPath.append(id.serializedForm());
     }
 
+    private Path preparingPath(ApplicationId id) {
+        return applicationPath(id).append("preparing");
+    }
+
     private Path lockPath(ApplicationId id) {
         return locksPath.append(id.serializedForm());
     }
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 b6e1d1873c9..ea54bbe0c9c 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
@@ -22,6 +22,7 @@ import com.yahoo.vespa.curator.Lock;
 import java.time.Clock;
 import java.time.Duration;
 import java.util.Optional;
+import java.util.OptionalLong;
 import java.util.logging.Logger;
 
 /**
@@ -97,6 +98,7 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
     @Override
     public void prepare() {
         if (prepared) return;
+
         TimeoutBudget timeoutBudget = new TimeoutBudget(clock, timeout);
 
         session.prepare(logger,
@@ -112,16 +114,24 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
         this.prepared = true;
     }
 
-    /** Activates this. If it is not already prepared, this will call prepare first. */
+    /**
+     * Activates this. If it is not already prepared, this will call prepare first.
+     *
+     * If the session is no longer supposed to be activated by the time prepare is done, throws a
+     *
+     */
     @Override
     public void activate() {
         if ( ! prepared)
             prepare();
 
         TimeoutBudget timeoutBudget = new TimeoutBudget(clock, timeout);
-
         try (Lock lock = tenant.getApplicationRepo().lock(session.getApplicationId())) {
             validateSessionStatus(session);
+            if ( ! tenant.getApplicationRepo().exists(session.getApplicationId()))
+                throw new IllegalStateException("Application " + session.getApplicationId() + " does not exist");
+            verifyApplicationIsPreparingSession(session, tenant.getApplicationRepo().preparing(session.getApplicationId()));
+
             NestedTransaction transaction = new NestedTransaction();
             transaction.add(deactivateCurrentActivateNew(applicationRepository.getActiveSession(session.getApplicationId()), session, ignoreSessionStaleFailure));
 
@@ -156,14 +166,24 @@ public class Deployment implements com.yahoo.config.provision.Deployment {
     /** Exposes the session of this for testing only */
     public LocalSession session() { return session; }
 
-    private long validateSessionStatus(LocalSession localSession) {
-        long sessionId = localSession.getSessionId();
-        if (Session.Status.NEW.equals(localSession.getStatus())) {
-            throw new IllegalStateException(localSession.logPre() + "Session " + sessionId + " is not prepared");
-        } else if (Session.Status.ACTIVATE.equals(localSession.getStatus())) {
-            throw new IllegalStateException(localSession.logPre() + "Session " + sessionId + " is already active");
-        }
-        return sessionId;
+    private void validateSessionStatus(LocalSession localSession) {
+        if (Session.Status.ACTIVATE.equals(localSession.getStatus()))
+            throw new IllegalStateException(localSession.logPre() + "Session " + localSession.getSessionId() + " is already active");
+
+        if (Session.Status.NEW.equals(localSession.getStatus()))
+            throw new IllegalStateException(localSession.logPre() + "Session " + localSession.getSessionId() + " is not prepared");
+    }
+
+    private void verifyApplicationIsPreparingSession(LocalSession localSession, OptionalLong preparing) {
+        if ( ! localSession.getMetaData().isInternalRedeploy())
+            return;
+
+        if (preparing.isEmpty())
+            throw new IllegalStateException("No session is currently being prepared for '" + session.getApplicationId() + "'");
+
+        if (preparing.getAsLong() != session.getSessionId())
+            throw new ActivationConflictException("Session " + session.getSessionId() + " is no longer supposed to be activated " +
+                                                  "for '" + session.getApplicationId() + "'; " + + preparing.getAsLong() + " is");
     }
 
     private Transaction deactivateCurrentActivateNew(LocalSession active, LocalSession prepared, boolean ignoreStaleSessionFailure) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java
index 7fd29368ab3..e19f8c8b273 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/LocalSession.java
@@ -18,12 +18,15 @@ import com.yahoo.config.provision.TenantName;
 import com.yahoo.vespa.config.server.application.ApplicationSet;
 import com.yahoo.vespa.config.server.application.TenantApplications;
 import com.yahoo.vespa.config.server.configchange.ConfigChangeActions;
+import com.yahoo.vespa.config.server.tenant.Tenant;
 import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.curator.Curator;
+import com.yahoo.vespa.curator.Lock;
 
 import java.io.File;
 import java.time.Instant;
 import java.util.Optional;
+import java.util.OptionalLong;
 
 /**
  * A LocalSession is a session that has been created locally on this configserver. A local session can be edited and
@@ -62,7 +65,18 @@ public class LocalSession extends Session implements Comparable<LocalSession> {
                                        Optional<ApplicationSet> currentActiveApplicationSet, 
                                        Path tenantPath,
                                        Instant now) {
-        applicationRepo.createApplication(params.getApplicationId()); // TODO jvenstad: This is wrong, but it has to be done now, since preparation can change the application ID of a session :(
+        ApplicationId newOwner = params.getApplicationId();
+
+        // If redeployment, and something else is preparing, bail out; otherwise, mark this as currently preparing.
+        try (Lock lock = applicationRepo.lock(newOwner)) {
+            applicationRepo.createApplication(newOwner);
+            OptionalLong preparing = applicationRepo.preparing(newOwner);
+            if (preparing.isPresent() && getMetaData().isInternalRedeploy())
+                throw new ActivationConflictException("Session " + preparing.getAsLong() + " is already being prepared for '"
+                                                      + newOwner + "'");
+            applicationRepo.prepare(newOwner, getSessionId());
+        }
+
         Curator.CompletionWaiter waiter = zooKeeperClient.createPrepareWaiter();
         ConfigChangeActions actions = sessionPreparer.prepare(sessionContext, logger, params,
                                                               currentActiveApplicationSet, tenantPath, now);
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java
index 0381af57cc3..69fbfe446d3 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/SessionHandlerTest.java
@@ -35,6 +35,7 @@ import com.yahoo.vespa.config.server.session.SessionContext;
 import com.yahoo.vespa.config.server.session.SessionFactory;
 import com.yahoo.vespa.config.server.session.SessionPreparer;
 import com.yahoo.vespa.config.server.session.SessionTest;
+import com.yahoo.vespa.config.server.tenant.Tenant;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 
 import java.io.ByteArrayOutputStream;
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java
index 3e6e48efca0..a62a2841995 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java
@@ -323,9 +323,11 @@ public class SessionActiveHandlerTest extends SessionHandlerTest {
                                             Optional.of(AllocatedHosts.withHosts(Collections.singleton(new HostSpec("bar", Collections.emptyList())))));
             session = createRemoteSession(sessionId, initialStatus, zkClient);
             addLocalSession(sessionId, deployData, zkClient);
-            tenantRepository.getTenant(tenantName).getApplicationRepo().createApplication(ApplicationId.from(tenantName.value(),
-                                                                                                             deployData.getApplicationName(),
-                                                                                                             InstanceName.defaultName().value()));
+            TenantApplications tenantApplications = tenantRepository.getTenant(tenantName).getApplicationRepo();
+            ApplicationId id = ApplicationId.from(tenantName.value(), deployData.getApplicationName(), InstanceName.defaultName().value());
+            tenantApplications.createApplication(id);
+            tenantApplications.prepare(id, sessionId);
+
             metaData = localRepo.getSession(sessionId).getMetaData();
             actResponse = handler.handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.ACTIVE, sessionId, subPath));
             return this;
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java
index 9cd2ac6d7b9..7b0b00caa8b 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java
@@ -30,6 +30,7 @@ import com.yahoo.vespa.config.server.configchange.MockRefeedAction;
 import com.yahoo.vespa.config.server.configchange.MockRestartAction;
 import com.yahoo.vespa.config.server.http.*;
 import com.yahoo.vespa.config.server.session.*;
+import com.yahoo.vespa.config.server.tenant.Tenant;
 import com.yahoo.vespa.config.server.tenant.TenantBuilder;
 import com.yahoo.vespa.config.server.tenant.TenantRepository;
 import com.yahoo.vespa.curator.Curator;