diff options
11 files changed, 101 insertions, 37 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployment.java b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployment.java index a5533726800..49ab80c5867 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/Deployment.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/Deployment.java @@ -24,5 +24,4 @@ public interface Deployment { * doing prepare and activate in the same session. */ void restart(HostFilter filter); - } 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/<tenant>/applications/<application>, - * 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/<tenant>/locks/<application>. * * @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..aabd1fd72cb 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,23 @@ 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())) + return; // Application was deleted. + + validateSessionStatus(session, tenant.getApplicationRepo().preparing(session.getApplicationId())); NestedTransaction transaction = new NestedTransaction(); transaction.add(deactivateCurrentActivateNew(applicationRepository.getActiveSession(session.getApplicationId()), session, ignoreSessionStaleFailure)); @@ -156,14 +165,22 @@ 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, OptionalLong preparing) { + 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"); + + 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; diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java index 96c8fe21959..fde19f72191 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/ApplicationMaintainer.java @@ -51,11 +51,6 @@ public abstract class ApplicationMaintainer extends Maintainer { return pendingDeployments.size(); } - /** Returns whether given application should be deployed at this moment in time */ - protected boolean canDeployNow(ApplicationId application) { - return true; - } - /** * Redeploy this application. * @@ -75,19 +70,18 @@ public abstract class ApplicationMaintainer extends Maintainer { /** Returns the applications that should be maintained by this now. */ protected abstract Set<ApplicationId> applicationsNeedingMaintenance(); - /** Redeploy this application. A lock will be taken for the duration of the deployment activation */ + /** Redeploy this application. */ protected final void deployWithLock(ApplicationId application) { // An application might change its state between the time the set of applications is retrieved and the // time deployment happens. Lock the application and check if it's still active. - // - // Lock is acquired with a low timeout to reduce the chance of colliding with an external deployment. - try (Mutex lock = nodeRepository().lock(application, Duration.ofSeconds(1))) { - if ( ! isActive(application)) return; // became inactive since deployment was requested - if ( ! canDeployNow(application)) return; // redeployment is no longer needed - Optional<Deployment> deployment = deployer.deployFromLocalActive(application); - if ( ! deployment.isPresent()) return; // this will be done at another config server - log.log(LogLevel.DEBUG, this.getClass().getSimpleName() + " deploying " + application); - deployment.get().activate(); + try { + try (Mutex lock = nodeRepository().lock(application, Duration.ofSeconds(10))) { + if ( ! isActive(application)) return; // became inactive since deployment was requested + } + deployer.deployFromLocalActive(application).ifPresent(deployment -> { // if deployed on this config server + log.log(LogLevel.DEBUG, this.getClass().getSimpleName() + " deploying " + application); + deployment.activate(); + }); } catch (RuntimeException e) { log.log(LogLevel.WARNING, "Exception on maintenance redeploy", e); } finally { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java index 174591b0836..5474c8ab51d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java @@ -38,8 +38,7 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer { this.start = clock.instant(); } - @Override - protected boolean canDeployNow(ApplicationId application) { + private boolean canDeployNow(ApplicationId application) { // Don't deploy if a regular deploy just happened return getLastDeployTime(application).isBefore(nodeRepository().clock().instant().minus(minTimeBetweenRedeployments)); } |