From d4ea0c2444d246ddc5ca902aed615200477eea30 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Wed, 29 May 2019 08:04:00 +0200 Subject: Revert "Jvenstad/config super model last take" --- .../config/server/ActivationConflictException.java | 7 ---- .../vespa/config/server/ApplicationRepository.java | 2 +- .../server/application/TenantApplications.java | 36 ++----------------- .../vespa/config/server/deploy/Deployment.java | 40 ++++++---------------- .../vespa/config/server/session/LocalSession.java | 16 +-------- .../config/server/http/SessionHandlerTest.java | 1 - .../server/http/v2/SessionActiveHandlerTest.java | 8 ++--- .../server/http/v2/SessionPrepareHandlerTest.java | 1 - 8 files changed, 18 insertions(+), 93 deletions(-) (limited to 'configserver') 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 d39168f9427..48023293cc4 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,18 +1,11 @@ // 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 cfe4de34ab5..3aea22ce8b2 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,7 +202,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye Optional 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. "); @@ -574,6 +573,7 @@ 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 bc33336a79b..f7a26ebd76c 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 @@ -14,7 +14,6 @@ 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; @@ -37,8 +36,7 @@ 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, and the /preparing child node contains the - * session ID of the session to activate next. Locks for synchronising writes to these paths, and changes + * the root contains the currently active session, if any. 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 @@ -99,18 +97,13 @@ public class TenantApplications { } /** - * 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. + * Returns a transaction which writes the given session id as the currently active for the given application. * * @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) { - 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; + return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId))); } /** @@ -122,25 +115,6 @@ 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. * @@ -217,10 +191,6 @@ 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 ea54bbe0c9c..b6e1d1873c9 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,7 +22,6 @@ 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; /** @@ -98,7 +97,6 @@ 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, @@ -114,24 +112,16 @@ 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. - * - * If the session is no longer supposed to be activated by the time prepare is done, throws a - * - */ + /** Activates this. If it is not already prepared, this will call prepare first. */ @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)); @@ -166,24 +156,14 @@ public class Deployment implements com.yahoo.config.provision.Deployment { /** Exposes the session of this for testing only */ public LocalSession session() { return session; } - 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 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 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 e19f8c8b273..7fd29368ab3 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,15 +18,12 @@ 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 @@ -65,18 +62,7 @@ public class LocalSession extends Session implements Comparable { Optional currentActiveApplicationSet, Path tenantPath, Instant now) { - 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()); - } - + 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 :( 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 69fbfe446d3..0381af57cc3 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,7 +35,6 @@ 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 087a8c0cb8c..d94194e58d9 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,11 +323,9 @@ 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); - TenantApplications tenantApplications = tenantRepository.getTenant(tenantName).getApplicationRepo(); - ApplicationId id = ApplicationId.from(tenantName.value(), deployData.getApplicationName(), InstanceName.defaultName().value()); - tenantApplications.createApplication(id); - tenantApplications.prepare(id, sessionId); - + tenantRepository.getTenant(tenantName).getApplicationRepo().createApplication(ApplicationId.from(tenantName.value(), + deployData.getApplicationName(), + InstanceName.defaultName().value())); 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 a264b440af8..b6c3de8a1b1 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,7 +30,6 @@ 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; -- cgit v1.2.3