diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2019-03-01 14:19:45 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-03-01 14:19:45 +0100 |
commit | 2dbc8b92946602803e8b2feb4d6143d32f40b8ca (patch) | |
tree | 235dce7d99f2aacfb11ed1b1d9f7360c76206913 | |
parent | 98e738577e3affa22450003db9d6f0449a42d26f (diff) |
Revert "Revert "Jvenstad/fix config model inconsitency""
31 files changed, 374 insertions, 293 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 ea7b6a88a9c..481f071a32e 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 @@ -53,6 +53,7 @@ import com.yahoo.vespa.config.server.session.SilentDeployLogger; import com.yahoo.vespa.config.server.tenant.Rotations; import com.yahoo.vespa.config.server.tenant.Tenant; import com.yahoo.vespa.config.server.tenant.TenantRepository; +import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.orchestrator.Orchestrator; import java.io.File; @@ -69,6 +70,7 @@ import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.OptionalLong; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -262,8 +264,10 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye @Override public Optional<Instant> lastDeployTime(ApplicationId application) { Tenant tenant = tenantRepository.getTenant(application.tenant()); - if (tenant == null) return Optional.empty(); - LocalSession activeSession = getActiveSession(tenant, application); + if (tenant == null || ! tenant.getApplicationRepo().exists(application)) return Optional.empty(); + OptionalLong activeSessionId = tenant.getApplicationRepo().activeSessionOf(application); + if ( ! activeSessionId.isPresent()) return Optional.empty(); + LocalSession activeSession = tenant.getLocalSessionRepo().getSession(activeSessionId.getAsLong()); if (activeSession == null) return Optional.empty(); return Optional.of(Instant.ofEpochSecond(activeSession.getCreateTime())); } @@ -296,34 +300,35 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye if (tenant == null) return false; TenantApplications tenantApplications = tenant.getApplicationRepo(); - if (!tenantApplications.listApplications().contains(applicationId)) return false; - - // Deleting an application is done by deleting the remote session and waiting - // until the config server where the deployment happened picks it up and deletes - // the local session - long sessionId = tenantApplications.getSessionIdForApplication(applicationId); - RemoteSession remoteSession = getRemoteSession(tenant, sessionId); - remoteSession.createDeleteTransaction().commit(); - - log.log(LogLevel.INFO, TenantRepository.logPre(applicationId) + "Waiting for session " + sessionId + " to be deleted"); - // TODO: Add support for timeout in request - Duration waitTime = Duration.ofSeconds(60); - if (localSessionHasBeenDeleted(applicationId, sessionId, waitTime)) { - log.log(LogLevel.INFO, TenantRepository.logPre(applicationId) + "Session " + sessionId + " deleted"); - } else { - log.log(LogLevel.ERROR, TenantRepository.logPre(applicationId) + "Session " + sessionId + " was not deleted (waited " + waitTime + ")"); - return false; - } + try (Lock lock = tenantApplications.lock(applicationId)) { + if ( ! tenantApplications.exists(applicationId)) return false; + // Deleting an application is done by deleting the remote session and waiting + // until the config server where the deployment happened picks it up and deletes + // the local session + long sessionId = tenantApplications.requireActiveSessionOf(applicationId); + RemoteSession remoteSession = getRemoteSession(tenant, sessionId); + remoteSession.createDeleteTransaction().commit(); + + log.log(LogLevel.INFO, TenantRepository.logPre(applicationId) + "Waiting for session " + sessionId + " to be deleted"); + // TODO: Add support for timeout in request + Duration waitTime = Duration.ofSeconds(60); + if (localSessionHasBeenDeleted(applicationId, sessionId, waitTime)) { + log.log(LogLevel.INFO, TenantRepository.logPre(applicationId) + "Session " + sessionId + " deleted"); + } else { + log.log(LogLevel.ERROR, TenantRepository.logPre(applicationId) + "Session " + sessionId + " was not deleted (waited " + waitTime + ")"); + return false; + } - NestedTransaction transaction = new NestedTransaction(); - transaction.add(new Rotations(tenant.getCurator(), tenant.getPath()).delete(applicationId)); // TODO: Not unit tested - // (When rotations are updated in zk, we need to redeploy the zone app, on the right config server - // this is done asynchronously in application maintenance by the node repository) - transaction.add(tenantApplications.deleteApplication(applicationId)); + NestedTransaction transaction = new NestedTransaction(); + transaction.add(new Rotations(tenant.getCurator(), tenant.getPath()).delete(applicationId)); // TODO: Not unit tested + // (When rotations are updated in zk, we need to redeploy the zone app, on the right config server + // this is done asynchronously in application maintenance by the node repository) + transaction.add(tenantApplications.createDeleteTransaction(applicationId)); - hostProvisioner.ifPresent(provisioner -> provisioner.remove(transaction, applicationId)); - transaction.onCommitted(() -> log.log(LogLevel.INFO, "Deleted " + applicationId)); - transaction.commit(); + hostProvisioner.ifPresent(provisioner -> provisioner.remove(transaction, applicationId)); + transaction.onCommitted(() -> log.log(LogLevel.INFO, "Deleted " + applicationId)); + transaction.commit(); + } return true; } @@ -419,7 +424,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye Set<ApplicationId> listApplications() { return tenantRepository.getAllTenants().stream() - .flatMap(tenant -> tenant.getApplicationRepo().listApplications().stream()) + .flatMap(tenant -> tenant.getApplicationRepo().activeApplications().stream()) .collect(Collectors.toSet()); } @@ -478,7 +483,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye if (applicationRepo == null) throw new IllegalArgumentException("Application repo for tenant '" + tenant.getName() + "' not found"); - return applicationRepo.getSessionIdForApplication(applicationId); + return applicationRepo.requireActiveSessionOf(applicationId); } public void validateThatRemoteSessionIsNotActive(Tenant tenant, long sessionId) { @@ -520,6 +525,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); @@ -560,7 +566,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } private List<ApplicationId> activeApplications(TenantName tenantName) { - return tenantRepository.getTenant(tenantName).getApplicationRepo().listApplications(); + return tenantRepository.getTenant(tenantName).getApplicationRepo().activeApplications(); } // ---------------- Misc operations ---------------------------------------------------------------- @@ -611,7 +617,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye Optional<ApplicationSet> currentActiveApplicationSet = Optional.empty(); TenantApplications applicationRepo = tenant.getApplicationRepo(); try { - long currentActiveSessionId = applicationRepo.getSessionIdForApplication(appId); + long currentActiveSessionId = applicationRepo.requireActiveSessionOf(appId); RemoteSession currentActiveSession = getRemoteSession(tenant, currentActiveSessionId); if (currentActiveSession != null) { currentActiveApplicationSet = Optional.ofNullable(currentActiveSession.ensureApplicationLoaded()); @@ -641,7 +647,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye private List<ApplicationId> listApplicationIds(Tenant tenant) { TenantApplications applicationRepo = tenant.getApplicationRepo(); - return applicationRepo.listApplications(); + return applicationRepo.activeApplications(); } private void cleanupTempDirectory(File tempDir) { @@ -653,13 +659,13 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye private LocalSession getExistingSession(Tenant tenant, ApplicationId applicationId) { TenantApplications applicationRepo = tenant.getApplicationRepo(); - return getLocalSession(tenant, applicationRepo.getSessionIdForApplication(applicationId)); + return getLocalSession(tenant, applicationRepo.requireActiveSessionOf(applicationId)); } private LocalSession getActiveSession(Tenant tenant, ApplicationId applicationId) { TenantApplications applicationRepo = tenant.getApplicationRepo(); - if (applicationRepo.listApplications().contains(applicationId)) { - return tenant.getLocalSessionRepo().getSession(applicationRepo.getSessionIdForApplication(applicationId)); + if (applicationRepo.activeApplications().contains(applicationId)) { + return tenant.getLocalSessionRepo().getSession(applicationRepo.requireActiveSessionOf(applicationId)); } return null; } 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 2c4d3d99408..81eb4107089 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 @@ -1,7 +1,6 @@ // 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.application; -import com.google.common.collect.ImmutableSet; import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; @@ -12,25 +11,32 @@ import com.yahoo.transaction.Transaction; 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.CuratorOperations; import com.yahoo.vespa.curator.transaction.CuratorTransaction; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; -import java.util.ArrayList; +import java.time.Duration; import java.util.List; -import java.util.Optional; +import java.util.Map; +import java.util.OptionalLong; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.logging.Logger; +import java.util.stream.Collectors; /** * The applications of a tenant, backed by ZooKeeper. * - * Each application is stored as a single node under /config/v2/tenants/<tenant>/applications/<applications>, - * named the same as the application id and containing the id of the session storing the content of the application. + * 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 + * to the config of this application, are found under /config/v2/tenants/<tenant>/locks/<application>. * * @author Ulf Lilleengen + * @author jonmv */ public class TenantApplications { @@ -38,17 +44,20 @@ public class TenantApplications { private final Curator curator; private final Path applicationsPath; + private final Path locksPath; // One thread pool for all instances of this class private static final ExecutorService pathChildrenExecutor = Executors.newCachedThreadPool(ThreadFactoryFactory.getDaemonThreadFactory(TenantApplications.class.getName())); private final Curator.DirectoryCache directoryCache; private final ReloadHandler reloadHandler; private final TenantName tenant; + private final Map<ApplicationId, Lock> locks; - private TenantApplications(Curator curator, Path applicationsPath, ReloadHandler reloadHandler, TenantName tenant) { + private TenantApplications(Curator curator, ReloadHandler reloadHandler, TenantName tenant) { this.curator = curator; - this.applicationsPath = applicationsPath; - curator.create(applicationsPath); + this.applicationsPath = TenantRepository.getApplicationsPath(tenant); + this.locksPath = TenantRepository.getLocksPath(tenant); + this.locks = new ConcurrentHashMap<>(2); this.reloadHandler = reloadHandler; this.tenant = tenant; this.directoryCache = curator.createDirectoryCache(applicationsPath.getAbsolute(), false, false, pathChildrenExecutor); @@ -57,11 +66,7 @@ public class TenantApplications { } public static TenantApplications create(Curator curator, ReloadHandler reloadHandler, TenantName tenant) { - try { - return new TenantApplications(curator, TenantRepository.getApplicationsPath(tenant), reloadHandler, tenant); - } catch (Exception e) { - throw new RuntimeException(TenantRepository.logPre(tenant) + "Error creating application repo", e); - } + return new TenantApplications(curator, reloadHandler, tenant); } /** @@ -69,75 +74,103 @@ public class TenantApplications { * * @return a list of {@link ApplicationId}s that are active. */ - public List<ApplicationId> listApplications() { + public List<ApplicationId> activeApplications() { + return curator.getChildren(applicationsPath).stream() + .filter(this::isValid) + .sorted() + .map(ApplicationId::fromSerializedForm) + .filter(id -> activeSessionOf(id).isPresent()) + .collect(Collectors.toUnmodifiableList()); + } + + private boolean isValid(String appNode) { // TODO jvenstad: Remove after it has run once everywhere. try { - List<String> appNodes = curator.framework().getChildren().forPath(applicationsPath.getAbsolute()); - List<ApplicationId> applicationIds = new ArrayList<>(); - for (String appNode : appNodes) { - parseApplication(appNode).ifPresent(applicationIds::add); + ApplicationId.fromSerializedForm(appNode); + return true; + } catch (IllegalArgumentException __) { + log.log(LogLevel.INFO, TenantRepository.logPre(tenant) + "Unable to parse application id from '" + + appNode + "'; deleting it as it shouldn't be here."); + try { + curator.delete(applicationsPath.append(appNode)); } - return applicationIds; - } catch (Exception e) { - throw new RuntimeException(TenantRepository.logPre(tenant)+"Unable to list applications", e); + catch (RuntimeException e) { + log.log(LogLevel.WARNING, TenantRepository.logPre(tenant) + "Failed to clean up stray node '" + appNode + "'!", e); + } + return false; } } - private Optional<ApplicationId> parseApplication(String appNode) { - try { - return Optional.of(ApplicationId.fromSerializedForm(appNode)); - } catch (IllegalArgumentException e) { - log.log(LogLevel.INFO, TenantRepository.logPre(tenant)+"Unable to parse application with id '" + appNode + "', ignoring."); - return Optional.empty(); - } + public boolean exists(ApplicationId id) { + return curator.exists(applicationPath(id)); + } + + /** Returns the id of the currently active session for the given application, if any. Throws on unknown applications. */ + public OptionalLong activeSessionOf(ApplicationId id) { + String data = curator.getData(applicationPath(id)).map(Utf8::toString) + .orElseThrow(() -> new IllegalArgumentException("Unknown application '" + id + "'.")); + return data.isEmpty() ? OptionalLong.empty() : OptionalLong.of(Long.parseLong(data)); } /** - * Register active application and adds it to the repo. If it already exists it is overwritten. + * 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 createPutApplicationTransaction(ApplicationId applicationId, long sessionId) { - if (listApplications().contains(applicationId)) { - return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationsPath.append(applicationId.serializedForm()).getAbsolute(), Utf8.toAsciiBytes(sessionId))); - } else { - return new CuratorTransaction(curator).add(CuratorOperations.create(applicationsPath.append(applicationId.serializedForm()).getAbsolute(), Utf8.toAsciiBytes(sessionId))); + public Transaction createPutTransaction(ApplicationId applicationId, long sessionId) { + return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId))); + } + + /** + * Creates a node for the given application, marking its existence. + */ + public void createApplication(ApplicationId id) { + try (Lock lock = lock(id)) { + curator.create(applicationPath(id)); } } /** - * Return the stored session id for a given application. + * Return the active session id for a given application. * * @param applicationId an {@link ApplicationId} * @return session id of given application id. - * @throws IllegalArgumentException if the application does not exist + * @throws IllegalArgumentException if the application has no active session */ - public long getSessionIdForApplication(ApplicationId applicationId) { - String path = applicationsPath.append(applicationId.serializedForm()).getAbsolute(); - try { - return Long.parseLong(Utf8.toString(curator.framework().getData().forPath(path))); - } catch (Exception e) { - throw new IllegalArgumentException(TenantRepository.logPre(applicationId) + "Unable to read the session id from '" + path + "'", e); - } + public long requireActiveSessionOf(ApplicationId applicationId) { + return activeSessionOf(applicationId) + .orElseThrow(() -> new IllegalArgumentException("Application '" + applicationId + "' has no active session.")); } /** - * Returns a transaction which deletes this application - * - * @param applicationId an {@link ApplicationId} to delete. + * Returns a transaction which deletes this application. + */ + public CuratorTransaction createDeleteTransaction(ApplicationId applicationId) { + return CuratorTransaction.from(CuratorOperations.deleteAll(applicationPath(applicationId).getAbsolute(), curator), curator); + } + + /** + * Removes all applications not known to this from the config server state. */ - public CuratorTransaction deleteApplication(ApplicationId applicationId) { - Path path = applicationsPath.append(applicationId.serializedForm()); - return CuratorTransaction.from(CuratorOperations.delete(path.getAbsolute()), curator); + public void removeUnusedApplications() { + reloadHandler.removeApplicationsExcept(Set.copyOf(activeApplications())); } /** - * Closes the application repo. Once a repo has been closed, it should not be used again. - */ + * Closes the application repo. Once a repo has been closed, it should not be used again. + */ public void close() { directoryCache.close(); } + /** Returns the lock for changing the session status of the given application. */ + public Lock lock(ApplicationId id) { + curator.create(lockPath(id)); + Lock lock = locks.computeIfAbsent(id, __ -> new Lock(lockPath(id).getAbsolute(), curator)); + lock.acquire(Duration.ofMinutes(1)); // These locks shouldn't be held for very long. + return lock; + } + private void childEvent(CuratorFramework client, PathChildrenCacheEvent event) { switch (event.getType()) { case CHILD_ADDED: @@ -167,13 +200,12 @@ public class TenantApplications { log.log(LogLevel.DEBUG, TenantRepository.logPre(applicationId) + "Application added: " + applicationId); } - /** - * Removes unused applications - * - */ - public void removeUnusedApplications() { - ImmutableSet<ApplicationId> activeApplications = ImmutableSet.copyOf(listApplications()); - reloadHandler.removeApplicationsExcept(activeApplications); + private Path applicationPath(ApplicationId id) { + return applicationsPath.append(id.serializedForm()); + } + + 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 21716730825..6d875c529a3 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 @@ -89,9 +89,8 @@ public class Deployment implements com.yahoo.config.provision.Deployment { timeout, clock, true, true, session.getVespaVersion(), isBootstrap); } - public Deployment setIgnoreSessionStaleFailure(boolean ignoreSessionStaleFailure) { + public void setIgnoreSessionStaleFailure(boolean ignoreSessionStaleFailure) { this.ignoreSessionStaleFailure = ignoreSessionStaleFailure; - return this; } /** Prepares this. This does nothing if this is already prepared */ @@ -116,13 +115,16 @@ public class Deployment implements com.yahoo.config.provision.Deployment { /** Activates this. If it is not already prepared, this will call prepare first. */ @Override public void activate() { - if (! prepared) + if ( ! prepared) prepare(); TimeoutBudget timeoutBudget = new TimeoutBudget(clock, timeout); - long sessionId = session.getSessionId(); - validateSessionStatus(session); - try (Lock lock = tenant.getSessionLock(timeout)) { + + try (Lock lock = tenant.getApplicationRepo().lock(session.getApplicationId())) { + if ( ! tenant.getApplicationRepo().exists(session.getApplicationId())) + return; // Application was deleted. + + validateSessionStatus(session); NestedTransaction transaction = new NestedTransaction(); transaction.add(deactivateCurrentActivateNew(applicationRepository.getActiveSession(session.getApplicationId()), session, ignoreSessionStaleFailure)); @@ -130,13 +132,15 @@ public class Deployment implements com.yahoo.config.provision.Deployment { hostProvisioner.get().activate(transaction, session.getApplicationId(), session.getAllocatedHosts().getHosts()); } transaction.commit(); - session.waitUntilActivated(timeoutBudget); } catch (RuntimeException e) { throw e; } catch (Exception e) { throw new InternalServerException("Error activating application", e); } - log.log(LogLevel.INFO, session.logPre() + "Session " + sessionId + + + session.waitUntilActivated(timeoutBudget); + + log.log(LogLevel.INFO, session.logPre() + "Session " + session.getSessionId() + " activated successfully using " + ( hostProvisioner.isPresent() ? hostProvisioner.get() : "no host provisioner" ) + ". Config generation " + session.getMetaData().getGeneration()); @@ -154,7 +158,7 @@ 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())) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistry.java b/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistry.java index 77572856ff5..a7f8b8164a5 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistry.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistry.java @@ -36,6 +36,7 @@ public class HostRegistry<T> implements HostValidator<T> { addHosts(key, newHosts); } + @Override public synchronized void verifyHosts(T key, Collection<String> newHosts) { for (String host : newHosts) { if (hostAlreadyTaken(host, key)) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandler.java index 6345532d4ff..cdf995b80bb 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandler.java @@ -56,7 +56,7 @@ public class ListApplicationsHandler extends HttpHandler { Utils.checkThatTenantExists(tenantRepository, tenantName); Tenant tenant = tenantRepository.getTenant(tenantName); TenantApplications applicationRepo = tenant.getApplicationRepo(); - return applicationRepo.listApplications(); + return applicationRepo.activeApplications(); } private static String createUrlStringFromId(String urlBase, ApplicationId id, Zone zone) { 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 0f9f8b72de1..af8956803ab 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 @@ -83,12 +83,6 @@ public class LocalSession extends Session implements Comparable<LocalSession> { setStatus(Session.Status.PREPARE); } - private Transaction setActive() { - Transaction transaction = createSetStatusTransaction(Status.ACTIVATE); - transaction.add(applicationRepo.createPutApplicationTransaction(zooKeeperClient.readApplicationId(), getSessionId()).operations()); - return transaction; - } - private Transaction createSetStatusTransaction(Status status) { return zooKeeperClient.createWriteStatusTransaction(status); } @@ -99,8 +93,10 @@ public class LocalSession extends Session implements Comparable<LocalSession> { public Transaction createActivateTransaction() { zooKeeperClient.createActiveWaiter(); - superModelGenerationCounter.increment(); - return setActive(); + superModelGenerationCounter.increment(); // TODO jvenstad: I hope this counter isn't used for serious things, as it's updated way ahead of activation. + Transaction transaction = createSetStatusTransaction(Status.ACTIVATE); + transaction.add(applicationRepo.createPutTransaction(zooKeeperClient.readApplicationId(), getSessionId()).operations()); + return transaction; } public Transaction createDeactivateTransaction() { 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 172d9c025d5..3f1619882cd 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 @@ -5,8 +5,8 @@ import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.TenantName; import com.yahoo.lang.SettableOptional; -import com.yahoo.transaction.Transaction; import com.yahoo.log.LogLevel; +import com.yahoo.transaction.Transaction; import com.yahoo.vespa.config.server.GlobalComponentRegistry; import com.yahoo.vespa.config.server.ReloadHandler; import com.yahoo.vespa.config.server.application.ApplicationSet; @@ -29,7 +29,7 @@ import java.util.logging.Logger; public class RemoteSession extends Session { private static final Logger log = Logger.getLogger(RemoteSession.class.getName()); - private volatile ApplicationSet applicationSet = null; + private ApplicationSet applicationSet = null; private final ActivatedModelsBuilder applicationLoader; private final Clock clock; @@ -69,18 +69,15 @@ public class RemoteSession extends Session { clock.instant())); } - public ApplicationSet ensureApplicationLoaded() { - if (applicationSet == null) { - applicationSet = loadApplication(); - } - return applicationSet; + public synchronized ApplicationSet ensureApplicationLoaded() { + return applicationSet == null ? applicationSet = loadApplication() : applicationSet; } public Session.Status getStatus() { return zooKeeperClient.readStatus(); } - public void deactivate() { + public synchronized void deactivate() { applicationSet = null; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java index 15182813a22..bbb06515bef 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/RemoteSessionRepo.java @@ -1,33 +1,36 @@ // 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.session; -import java.time.Duration; -import java.time.Instant; -import java.util.*; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; -import java.util.logging.Level; -import java.util.logging.Logger; -import java.util.stream.Collectors; - import com.google.common.collect.HashMultiset; import com.google.common.collect.Multiset; import com.yahoo.concurrent.ThreadFactoryFactory; +import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.log.LogLevel; import com.yahoo.path.Path; -import com.yahoo.vespa.config.server.application.ApplicationSet; +import com.yahoo.vespa.config.server.ReloadHandler; import com.yahoo.vespa.config.server.application.TenantApplications; +import com.yahoo.vespa.config.server.monitoring.MetricUpdater; import com.yahoo.vespa.config.server.tenant.TenantRepository; +import com.yahoo.vespa.config.server.zookeeper.ConfigCurator; import com.yahoo.vespa.curator.Curator; import com.yahoo.yolean.Exceptions; -import com.yahoo.vespa.config.server.ReloadHandler; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.vespa.config.server.monitoring.MetricUpdater; -import com.yahoo.vespa.config.server.zookeeper.ConfigCurator; - import org.apache.curator.framework.CuratorFramework; -import org.apache.curator.framework.recipes.cache.*; +import org.apache.curator.framework.recipes.cache.ChildData; +import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; + +import java.time.Duration; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.logging.Level; +import java.util.logging.Logger; +import java.util.stream.Collectors; /** * Will watch/prepare sessions (applications) based on watched nodes in ZooKeeper, set for example @@ -115,19 +118,6 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { return (created.plus(expiryTime).isBefore(Instant.now())); } - private void loadActiveSession(RemoteSession session) { - tryReload(session.ensureApplicationLoaded(), session.logPre()); - } - - private void tryReload(ApplicationSet applicationSet, String logPre) { - try { - reloadHandler.reloadConfig(applicationSet); - log.log(LogLevel.INFO, logPre + "Application activated successfully: " + applicationSet.getId()); - } catch (Exception e) { - log.log(LogLevel.WARNING, logPre + "Skipping loading of application '" + applicationSet.getId() + "': " + Exceptions.toMessageString(e)); - } - } - private List<Long> getSessionListFromDirectoryCache(List<ChildData> children) { return getSessionList(children.stream() .map(child -> Path.fromString(child.getPath()).getName()) @@ -190,15 +180,16 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { } private void loadSessionIfActive(RemoteSession session) { - for (ApplicationId applicationId : applicationRepo.listApplications()) { + for (ApplicationId applicationId : applicationRepo.activeApplications()) { try { - if (applicationRepo.getSessionIdForApplication(applicationId) == session.getSessionId()) { + if (applicationRepo.requireActiveSessionOf(applicationId) == session.getSessionId()) { log.log(LogLevel.DEBUG, "Found active application for session " + session.getSessionId() + " , loading it"); - loadActiveSession(session); - break; + reloadHandler.reloadConfig(session.ensureApplicationLoaded()); + log.log(LogLevel.INFO, session.logPre() + "Application activated successfully: " + applicationId); + return; } } catch (Exception e) { - log.log(LogLevel.WARNING, session.logPre() + " error reading session id for " + applicationId, e); + log.log(LogLevel.WARNING, session.logPre() + "Skipping loading of application '" + applicationId + "': " + Exceptions.toMessageString(e)); } } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java index a3dea83d50c..5527d3060f7 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactory.java @@ -17,8 +17,6 @@ public interface SessionFactory { /** * Creates a new deployment session from an application package. * - * - * * @param applicationDirectory a File pointing to an application. * @param applicationId application id for this new session. * @param timeoutBudget Timeout for creating session and waiting for other servers. @@ -29,10 +27,10 @@ public interface SessionFactory { /** * Creates a new deployment session from an already existing session. * - * @param existingSession The session to use as base + * @param existingSession the session to use as base * @param logger a deploy logger where the deploy log will be written. - * @param internalRedeploy if this session is for a system internal redeploy not an application package change - * @param timeoutBudget Timeout for creating session and waiting for other servers. + * @param internalRedeploy whether this session is for a system internal redeploy — not an application package change + * @param timeoutBudget timeout for creating session and waiting for other servers. * @return a new session */ LocalSession createSessionFromExisting(LocalSession existingSession, DeployLogger logger, diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java index b79ea720aea..d8ba5890545 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java @@ -188,10 +188,11 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { } private long getActiveSessionId(ApplicationId applicationId) { - List<ApplicationId> applicationIds = applicationRepo.listApplications(); + List<ApplicationId> applicationIds = applicationRepo.activeApplications(); if (applicationIds.contains(applicationId)) { - return applicationRepo.getSessionIdForApplication(applicationId); + return applicationRepo.requireActiveSessionOf(applicationId); } return nonExistingActiveSession; } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java index 43a5ff6d0c2..f0ceeb186fe 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionPreparer.java @@ -113,9 +113,8 @@ public class SessionPreparer { log.log(LogLevel.DEBUG, () -> "time used " + params.getTimeoutBudget().timesUsed() + " : " + params.getApplicationId()); return preparation.result(); - } catch (OutOfCapacityException e) { - throw e; - } catch (IllegalArgumentException e) { + } + catch (IllegalArgumentException e) { throw new InvalidApplicationException("Invalid application package", e); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java index a68f4a396cd..88e71d7ddd1 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java @@ -11,10 +11,8 @@ import com.yahoo.vespa.config.server.session.LocalSessionRepo; import com.yahoo.vespa.config.server.session.RemoteSessionRepo; import com.yahoo.vespa.config.server.session.SessionFactory; import com.yahoo.vespa.curator.Curator; -import com.yahoo.vespa.curator.Lock; import org.apache.zookeeper.data.Stat; -import java.time.Duration; import java.time.Instant; import java.util.Optional; @@ -30,7 +28,7 @@ public class Tenant implements TenantHandlerProvider { static final String SESSIONS = "sessions"; static final String APPLICATIONS = "applications"; - static final String SESSION_LOCK_PATH = "activateLock"; + static final String LOCKS = "locks"; private final TenantName name; private final RemoteSessionRepo remoteSessionRepo; @@ -38,7 +36,6 @@ public class Tenant implements TenantHandlerProvider { private final SessionFactory sessionFactory; private final LocalSessionRepo localSessionRepo; private final TenantApplications applicationRepo; - private final Lock sessionLock; private final RequestHandler requestHandler; private final ReloadHandler reloadHandler; private final TenantFileSystemDirs tenantFileSystemDirs; @@ -61,7 +58,6 @@ public class Tenant implements TenantHandlerProvider { this.remoteSessionRepo = remoteSessionRepo; this.sessionFactory = sessionFactory; this.localSessionRepo = localSessionRepo; - this.sessionLock = createLock(curator, path); this.applicationRepo = applicationRepo; this.tenantFileSystemDirs = tenantFileSystemDirs; this.curator = curator; @@ -110,14 +106,6 @@ public class Tenant implements TenantHandlerProvider { return localSessionRepo; } - /** - * This lock allows activation and deactivation of sessions under this tenant. - */ - public Lock getSessionLock(Duration timeout) { - sessionLock.acquire(timeout); - return sessionLock; - } - @Override public String toString() { return getName().value(); @@ -165,10 +153,4 @@ public class Tenant implements TenantHandlerProvider { curator.delete(path); } - private static Lock createLock(Curator curator, Path tenantPath) { - Path lockPath = tenantPath.append(SESSION_LOCK_PATH); - curator.create(lockPath); - return new Lock(lockPath.getAbsolute(), curator); - } - } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantBuilder.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantBuilder.java index 078b6e861a9..943ae6248fc 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantBuilder.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantBuilder.java @@ -31,7 +31,7 @@ public class TenantBuilder { private SessionFactory sessionFactory; private LocalSessionLoader localSessionLoader; private TenantApplications applicationRepo; - private ReloadHandler reloadHandler; + private TenantRequestHandler reloadHandler; private RequestHandler requestHandler; private RemoteSessionFactory remoteSessionFactory; private TenantFileSystemDirs tenantFileSystemDirs; @@ -120,7 +120,7 @@ public class TenantBuilder { private void createApplicationRepo() { if (applicationRepo == null) { - applicationRepo = TenantApplications.create(componentRegistry.getCurator(), reloadHandler, tenant); + applicationRepo = reloadHandler.applications(); } } @@ -130,7 +130,8 @@ public class TenantBuilder { tenant, Collections.singletonList(componentRegistry.getReloadListener()), ConfigResponseFactory.create(componentRegistry.getConfigserverConfig()), - componentRegistry.getHostRegistries()); + componentRegistry.getHostRegistries(), + componentRegistry.getCurator()); if (hostValidator == null) { this.hostValidator = impl; } @@ -164,9 +165,5 @@ public class TenantBuilder { } } - public TenantApplications getApplicationRepo() { - return applicationRepo; - } - public TenantName getTenantName() { return tenant; } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java index 9c74c9c1e67..c37db0e0df5 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java @@ -262,7 +262,10 @@ public class TenantRepository { */ private synchronized void writeTenantPath(TenantName name) { Path tenantPath = getTenantPath(name); - curator.createAtomically(tenantPath, tenantPath.append(Tenant.SESSIONS), tenantPath.append(Tenant.APPLICATIONS)); + curator.createAtomically(tenantPath, + tenantPath.append(Tenant.SESSIONS), + tenantPath.append(Tenant.APPLICATIONS), + tenantPath.append(Tenant.LOCKS)); } /** @@ -406,4 +409,11 @@ public class TenantRepository { return getTenantPath(tenantName).append(Tenant.APPLICATIONS); } + /** + * Gets zookeeper path for locks for a tenant's applications + */ + public static Path getLocksPath(TenantName tenantName) { + return getTenantPath(tenantName).append(Tenant.LOCKS); + } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandler.java index a341061bd9a..3f2b8c76e44 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandler.java @@ -6,6 +6,7 @@ import java.util.Collection; import java.util.LinkedHashSet; import java.util.List; import java.util.Optional; +import java.util.OptionalLong; import java.util.Set; import com.yahoo.component.Version; @@ -16,6 +17,7 @@ import com.yahoo.vespa.config.protocol.ConfigResponse; import com.yahoo.vespa.config.server.NotFoundException; import com.yahoo.vespa.config.server.application.ApplicationMapper; import com.yahoo.vespa.config.server.application.ApplicationSet; +import com.yahoo.vespa.config.server.application.TenantApplications; import com.yahoo.vespa.config.server.rpc.ConfigResponseFactory; import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.host.HostRegistry; @@ -29,6 +31,8 @@ import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; import com.yahoo.vespa.config.server.monitoring.MetricUpdater; import com.yahoo.vespa.config.server.monitoring.Metrics; +import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.curator.Lock; /** * A per tenant request handler, for handling reload (activate application) and getConfig requests for @@ -44,23 +48,25 @@ public class TenantRequestHandler implements RequestHandler, ReloadHandler, Host private final TenantName tenant; private final List<ReloadListener> reloadListeners; private final ConfigResponseFactory responseFactory; - private final HostRegistry<ApplicationId> hostRegistry; private final ApplicationMapper applicationMapper = new ApplicationMapper(); private final MetricUpdater tenantMetricUpdater; private final Clock clock = Clock.systemUTC(); + private final TenantApplications applications; public TenantRequestHandler(Metrics metrics, TenantName tenant, List<ReloadListener> reloadListeners, ConfigResponseFactory responseFactory, - HostRegistries hostRegistries) { + HostRegistries hostRegistries, + Curator curator) { // TODO jvenstad: Merge this class with TenantApplications, and straighten this out. this.metrics = metrics; this.tenant = tenant; - this.reloadListeners = reloadListeners; + this.reloadListeners = List.copyOf(reloadListeners); this.responseFactory = responseFactory; - tenantMetricUpdater = metrics.getOrCreateMetricUpdater(Metrics.createDimensions(tenant)); - hostRegistry = hostRegistries.createApplicationHostRegistry(tenant); + this.tenantMetricUpdater = metrics.getOrCreateMetricUpdater(Metrics.createDimensions(tenant)); + this.hostRegistry = hostRegistries.createApplicationHostRegistry(tenant); + this.applications = TenantApplications.create(curator, this, tenant); } /** @@ -93,26 +99,40 @@ public class TenantRequestHandler implements RequestHandler, ReloadHandler, Host * * @param applicationSet the {@link ApplicationSet} to be reloaded */ + @Override public void reloadConfig(ApplicationSet applicationSet) { - setLiveApp(applicationSet); - notifyReloadListeners(applicationSet); + ApplicationId id = applicationSet.getId(); + try (Lock lock = applications.lock(id)) { + if ( ! applications.exists(id)) + return; // Application was deleted before activation. + if (applicationSet.getApplicationGeneration() != applications.requireActiveSessionOf(id)) + return; // Application activated a new session before we got here. + + setLiveApp(applicationSet); + notifyReloadListeners(applicationSet); + } } @Override public void removeApplication(ApplicationId applicationId) { - if (applicationMapper.hasApplication(applicationId, clock.instant())) { - applicationMapper.remove(applicationId); - hostRegistry.removeHostsForKey(applicationId); - reloadListenersOnRemove(applicationId); - tenantMetricUpdater.setApplications(applicationMapper.numApplications()); - metrics.removeMetricUpdater(Metrics.createDimensions(applicationId)); + try (Lock lock = applications.lock(applicationId)) { + if (applications.exists(applicationId)) + return; // Application was deployed again. + + if (applicationMapper.hasApplication(applicationId, clock.instant())) { + applicationMapper.remove(applicationId); + hostRegistry.removeHostsForKey(applicationId); + reloadListenersOnRemove(applicationId); + tenantMetricUpdater.setApplications(applicationMapper.numApplications()); + metrics.removeMetricUpdater(Metrics.createDimensions(applicationId)); + } } } @Override public void removeApplicationsExcept(Set<ApplicationId> applications) { for (ApplicationId activeApplication : applicationMapper.listApplicationIds()) { - if (! applications.contains(activeApplication)) { + if ( ! applications.contains(activeApplication)) { log.log(LogLevel.INFO, "Will remove deleted application " + activeApplication.toShortString()); removeApplication(activeApplication); } @@ -237,4 +257,6 @@ public class TenantRequestHandler implements RequestHandler, ReloadHandler, Host } } + TenantApplications applications() { return applications; } + } 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 3d94dedf651..7a4998215e3 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 @@ -286,7 +286,7 @@ public class ApplicationRepositoryTest { assertTrue(deployment2.isPresent()); deployment2.get().activate(); // session 3 - long activeSessionId = tester.tenant().getApplicationRepo().getSessionIdForApplication(tester.applicationId()); + long activeSessionId = tester.tenant().getApplicationRepo().requireActiveSessionOf(tester.applicationId()); clock.advance(Duration.ofSeconds(10)); Optional<com.yahoo.config.provision.Deployment> deployment3 = tester.redeployFromLocalActive(); @@ -296,7 +296,7 @@ public class ApplicationRepositoryTest { LocalSession deployment3session = ((com.yahoo.vespa.config.server.deploy.Deployment) deployment3.get()).session(); assertNotEquals(activeSessionId, deployment3session); // No change to active session id - assertEquals(activeSessionId, tester.tenant().getApplicationRepo().getSessionIdForApplication(tester.applicationId())); + assertEquals(activeSessionId, tester.tenant().getApplicationRepo().requireActiveSessionOf(tester.applicationId())); assertEquals(3, tester.tenant().getLocalSessionRepo().listSessions().size()); clock.advance(Duration.ofHours(1)); // longer than session lifetime diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java index a708e4d8ace..69c88dc0275 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java @@ -39,12 +39,12 @@ public class TenantApplicationsTest { writeApplicationData(createApplicationId("foo"), 3L); writeApplicationData(createApplicationId("bar"), 4L); TenantApplications repo = createZKAppRepo(); - List<ApplicationId> applications = repo.listApplications(); + List<ApplicationId> applications = repo.activeApplications(); assertThat(applications.size(), is(2)); - assertThat(applications.get(0).application().value(), is("foo")); - assertThat(applications.get(1).application().value(), is("bar")); - assertThat(repo.getSessionIdForApplication(applications.get(0)), is(3L)); - assertThat(repo.getSessionIdForApplication(applications.get(1)), is(4L)); + assertThat(applications.get(0).application().value(), is("bar")); + assertThat(applications.get(1).application().value(), is("foo")); + assertThat(repo.requireActiveSessionOf(applications.get(0)), is(4L)); + assertThat(repo.requireActiveSessionOf(applications.get(1)), is(3L)); } @Test @@ -52,7 +52,7 @@ public class TenantApplicationsTest { writeApplicationData(createApplicationId("foo"), 3L); writeApplicationData("invalid", 3L); TenantApplications repo = createZKAppRepo(); - List<ApplicationId> applications = repo.listApplications(); + List<ApplicationId> applications = repo.activeApplications(); assertThat(applications.size(), is(1)); assertThat(applications.get(0).application().value(), is("foo")); } @@ -60,7 +60,7 @@ public class TenantApplicationsTest { @Test(expected = IllegalArgumentException.class) public void require_that_requesting_session_for_unknown_application_throws_exception() throws Exception { TenantApplications repo = createZKAppRepo(); - repo.getSessionIdForApplication(createApplicationId("nonexistent")); + repo.requireActiveSessionOf(createApplicationId("nonexistent")); } @Test(expected = IllegalArgumentException.class) @@ -70,18 +70,19 @@ public class TenantApplicationsTest { curatorFramework.create().creatingParentsIfNeeded() .forPath(TenantRepository.getApplicationsPath(tenantName).append(baz.serializedForm()).getAbsolute()); TenantApplications repo = createZKAppRepo(); - repo.getSessionIdForApplication(baz); + repo.requireActiveSessionOf(baz); } @Test public void require_that_application_ids_can_be_written() throws Exception { TenantApplications repo = createZKAppRepo(); ApplicationId myapp = createApplicationId("myapp"); - repo.createPutApplicationTransaction(myapp, 3l).commit(); + repo.createApplication(myapp); + repo.createPutTransaction(myapp, 3l).commit(); String path = TenantRepository.getApplicationsPath(tenantName).append(myapp.serializedForm()).getAbsolute(); assertTrue(curatorFramework.checkExists().forPath(path) != null); assertThat(Utf8.toString(curatorFramework.getData().forPath(path)), is("3")); - repo.createPutApplicationTransaction(myapp, 5l).commit(); + repo.createPutTransaction(myapp, 5l).commit(); assertTrue(curatorFramework.checkExists().forPath(path) != null); assertThat(Utf8.toString(curatorFramework.getData().forPath(path)), is("5")); } @@ -91,13 +92,15 @@ public class TenantApplicationsTest { TenantApplications repo = createZKAppRepo(); ApplicationId id1 = createApplicationId("myapp"); ApplicationId id2 = createApplicationId("myapp2"); - repo.createPutApplicationTransaction(id1, 1).commit(); - repo.createPutApplicationTransaction(id2, 1).commit(); - assertThat(repo.listApplications().size(), is(2)); - repo.deleteApplication(id1).commit(); - assertThat(repo.listApplications().size(), is(1)); - repo.deleteApplication(id2).commit(); - assertThat(repo.listApplications().size(), is(0)); + repo.createApplication(id1); + repo.createApplication(id2); + repo.createPutTransaction(id1, 1).commit(); + repo.createPutTransaction(id2, 1).commit(); + assertThat(repo.activeApplications().size(), is(2)); + repo.createDeleteTransaction(id1).commit(); + assertThat(repo.activeApplications().size(), is(1)); + repo.createDeleteTransaction(id2).commit(); + assertThat(repo.activeApplications().size(), is(0)); } @Test @@ -108,7 +111,7 @@ public class TenantApplicationsTest { MockReloadHandler reloadHandler = new MockReloadHandler(); TenantApplications repo = createZKAppRepo(reloadHandler); assertNull(reloadHandler.lastRemoved); - repo.deleteApplication(foo).commit(); + repo.createDeleteTransaction(foo).commit(); long endTime = System.currentTimeMillis() + 60_000; while (System.currentTimeMillis() < endTime && reloadHandler.lastRemoved == null) { Thread.sleep(100); @@ -126,11 +129,7 @@ public class TenantApplicationsTest { } private static ApplicationId createApplicationId(String name) { - return new ApplicationId.Builder() - .tenant(tenantName.value()) - .applicationName(name) - .instanceName("myinst") - .build(); + return ApplicationId.from(tenantName.value(), name, "myinst"); } private void writeApplicationData(ApplicationId applicationId, long sessionId) throws Exception { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java index 7ffb9552cf8..6b67dcc4e9a 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java @@ -183,7 +183,7 @@ public class DeployTester { public AllocatedHosts getAllocatedHostsOf(ApplicationId applicationId) { Tenant tenant = tenant(); LocalSession session = tenant.getLocalSessionRepo().getSession(tenant.getApplicationRepo() - .getSessionIdForApplication(applicationId)); + .requireActiveSessionOf(applicationId)); return session.getAllocatedHosts(); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java index b0bb3bf244f..c6a8e1f2f9d 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java @@ -56,12 +56,14 @@ public class ApplicationContentHandlerTest extends ContentHandlerTestBase { session2 = new MockSession(2l, FilesApplicationPackage.fromFile(new File("src/test/apps/content"))); Tenant tenant1 = tenantRepository.getTenant(tenantName1); tenant1.getLocalSessionRepo().addSession(session2); - tenant1.getApplicationRepo().createPutApplicationTransaction(idTenant1, 2l).commit(); + tenant1.getApplicationRepo().createApplication(idTenant1); + tenant1.getApplicationRepo().createPutTransaction(idTenant1, 2l).commit(); MockSession session3 = new MockSession(3l, FilesApplicationPackage.fromFile(new File("src/test/apps/content2"))); Tenant tenant2 = tenantRepository.getTenant(tenantName2); tenant2.getLocalSessionRepo().addSession(session3); - tenant2.getApplicationRepo().createPutApplicationTransaction(idTenant2, 3l).commit(); + tenant2.getApplicationRepo().createApplication(idTenant2); + tenant2.getApplicationRepo().createPutTransaction(idTenant2, 3l).commit(); handler = new ApplicationHandler(ApplicationHandler.testOnlyContext(), Zone.defaultZone(), 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 2c84e2d8ad4..bfd6a35cc2e 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 @@ -222,13 +222,13 @@ public class ApplicationHandlerTest { } private void deleteAndAssertOKResponseMocked(ApplicationId applicationId, boolean fullAppIdInUrl) throws IOException { - long sessionId = tenantRepository.getTenant(applicationId.tenant()).getApplicationRepo().getSessionIdForApplication(applicationId); + long sessionId = tenantRepository.getTenant(applicationId.tenant()).getApplicationRepo().requireActiveSessionOf(applicationId); deleteAndAssertResponse(applicationId, Zone.defaultZone(), Response.Status.OK, null, fullAppIdInUrl); assertNull(tenantRepository.getTenant(applicationId.tenant()).getLocalSessionRepo().getSession(sessionId)); } private void deleteAndAssertOKResponse(Tenant tenant, ApplicationId applicationId) throws IOException { - long sessionId = tenant.getApplicationRepo().getSessionIdForApplication(applicationId); + long sessionId = tenant.getApplicationRepo().requireActiveSessionOf(applicationId); deleteAndAssertResponse(applicationId, Zone.defaultZone(), Response.Status.OK, null, true); assertNull(tenant.getLocalSessionRepo().getSession(sessionId)); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java index 2d3dcc592f7..1db70956407 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java @@ -46,7 +46,8 @@ public class HostHandlerTest { private HostHandler hostHandler; static void addMockApplication(Tenant tenant, ApplicationId applicationId, long sessionId) { - tenant.getApplicationRepo().createPutApplicationTransaction(applicationId, sessionId).commit(); + tenant.getApplicationRepo().createApplication(applicationId); + tenant.getApplicationRepo().createPutTransaction(applicationId, sessionId).commit(); ApplicationPackage app = FilesApplicationPackage.fromFile(testApp); tenant.getLocalSessionRepo().addSession(new SessionHandlerTest.MockSession(sessionId, app, applicationId)); TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder() diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java index f57e7f09b39..f97bc443a38 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java @@ -51,17 +51,17 @@ public class ListApplicationsHandlerTest { final String url = "http://myhost:14000/application/v2/tenant/mytenant/application/"; assertResponse(url, Response.Status.OK, "[]"); - applicationRepo.createPutApplicationTransaction( - new ApplicationId.Builder().tenant("tenant").applicationName("foo").instanceName("quux").build(), - 1).commit(); + ApplicationId id1 = ApplicationId.from("mytenant", "foo", "quux"); + applicationRepo.createApplication(id1); + applicationRepo.createPutTransaction(id1, 1).commit(); assertResponse(url, Response.Status.OK, "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]"); - applicationRepo.createPutApplicationTransaction( - new ApplicationId.Builder().tenant("tenant").applicationName("bali").instanceName("quux").build(), - 1).commit(); + ApplicationId id2 = ApplicationId.from("mytenant", "bali", "quux"); + applicationRepo.createApplication(id2); + applicationRepo.createPutTransaction(id2, 1).commit(); assertResponse(url, Response.Status.OK, - "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"," + - "\"" + url + "bali/environment/dev/region/us-east/instance/quux\"]" + "[\"" + url + "bali/environment/dev/region/us-east/instance/quux\"," + + "\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]" ); } @@ -82,12 +82,12 @@ public class ListApplicationsHandlerTest { @Test public void require_that_listing_works_with_multiple_tenants() throws Exception { - applicationRepo.createPutApplicationTransaction(new ApplicationId.Builder() - .tenant("tenant") - .applicationName("foo").instanceName("quux").build(), 1).commit(); - applicationRepo2.createPutApplicationTransaction(new ApplicationId.Builder() - .tenant("tenant") - .applicationName("quux").instanceName("foo").build(), 1).commit(); + ApplicationId id1 = ApplicationId.from("mytenant", "foo", "quux"); + applicationRepo.createApplication(id1); + applicationRepo.createPutTransaction(id1, 1).commit(); + ApplicationId id2 = ApplicationId.from("foobar", "quux", "foo"); + applicationRepo2.createApplication(id2); + applicationRepo2.createPutTransaction(id2, 1).commit(); String url = "http://myhost:14000/application/v2/tenant/mytenant/application/"; assertResponse(url, Response.Status.OK, "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]"); 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 380b76c30af..858d1e0eaa7 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,6 +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); + 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/SessionCreateHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java index 94d3b126bd7..7f7f71a1fe8 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java @@ -203,7 +203,8 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { .applicationName("foo") .instanceName("quux") .build(); - applicationRepo.createPutApplicationTransaction(fooId, 2).commit(); + applicationRepo.createApplication(fooId); + applicationRepo.createPutTransaction(fooId, 2).commit(); assertFromParameter("3", "http://myhost:40555/application/v2/tenant/" + tenant + "/application/foo/environment/test/region/baz/instance/quux"); localSessionRepo.addSession(new SessionHandlerTest.MockSession(5l, FilesApplicationPackage.fromFile(testApp))); ApplicationId bioId = new ApplicationId.Builder() @@ -211,7 +212,8 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { .applicationName("foobio") .instanceName("quux") .build(); - applicationRepo.createPutApplicationTransaction(bioId, 5).commit(); + applicationRepo.createApplication(bioId); + applicationRepo.createPutTransaction(bioId, 5).commit(); assertFromParameter("6", "http://myhost:40555/application/v2/tenant/" + tenant + "/application/foobio/environment/staging/region/baz/instance/quux"); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java index e7db4dcf58f..a4432dcbfcd 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java @@ -196,12 +196,14 @@ public class LocalSessionTest { zkClient.write(Collections.singletonMap(new Version(0, 0, 0), new MockFileRegistry())); File sessionDir = new File(tenantFileSystemDirs.sessionsPath(), String.valueOf(sessionId)); sessionDir.createNewFile(); + TenantApplications applications = TenantApplications.create(curator, new MockReloadHandler(), tenant); + applications.createApplication(zkc.readApplicationId()); return new LocalSession(tenant, sessionId, preparer, new SessionContext( FilesApplicationPackage.fromFile(testApp), zkc, sessionDir, - TenantApplications.create(curator, new MockReloadHandler(), tenant), + applications, new HostRegistry<>(), superModelGenerationCounter, flagSource)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java index a7b74e69a21..d5d0fe72dbe 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/RemoteSessionTest.java @@ -3,23 +3,26 @@ package com.yahoo.vespa.config.server.session; import com.google.common.io.Files; import com.yahoo.cloud.config.ConfigserverConfig; +import com.yahoo.component.Version; import com.yahoo.config.application.api.ApplicationPackage; import com.yahoo.config.model.NullConfigModelRegistry; -import com.yahoo.config.model.api.*; +import com.yahoo.config.model.api.Model; +import com.yahoo.config.model.api.ModelContext; +import com.yahoo.config.model.api.ModelCreateResult; +import com.yahoo.config.model.api.ModelFactory; +import com.yahoo.config.model.api.ValidationParameters; import com.yahoo.config.model.deploy.DeployState; +import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; -import com.yahoo.config.model.test.MockApplicationPackage; -import com.yahoo.component.Version; -import com.yahoo.vespa.config.server.application.ApplicationSet; -import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; import com.yahoo.vespa.config.server.TestComponentRegistry; +import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.application.PermanentApplicationPackage; -import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.VespaModelFactory; - import org.junit.Before; import org.junit.Test; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java index 4046384005d..9bd5c5f1614 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java @@ -81,6 +81,8 @@ public class TenantRepositoryTest { @Test public void testListenersAdded() throws IOException, SAXException { + tenantRepository.getTenant(tenant1).getApplicationRepo().createApplication(ApplicationId.defaultId()); + tenantRepository.getTenant(tenant1).getApplicationRepo().createPutTransaction(ApplicationId.defaultId(), 4).commit(); tenantRepository.getTenant(tenant1).getReloadHandler().reloadConfig(ApplicationSet.fromSingle( new Application(new VespaModel(MockApplicationPackage.createEmpty()), new ServerCache(), diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java index 5f18046cb81..26a6f5e0c5f 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRequestHandlerTest.java @@ -1,6 +1,7 @@ // 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.tenant; +import com.yahoo.component.Version; import com.yahoo.config.ConfigInstance; import com.yahoo.config.SimpletypesConfig; import com.yahoo.config.application.api.ApplicationPackage; @@ -9,9 +10,10 @@ import com.yahoo.config.model.application.provider.BaseDeployLogger; import com.yahoo.config.model.application.provider.DeployData; import com.yahoo.config.model.application.provider.FilesApplicationPackage; import com.yahoo.config.model.application.provider.MockFileRegistry; -import com.yahoo.config.provision.ApplicationName; import com.yahoo.config.provision.AllocatedHosts; -import com.yahoo.component.Version; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationName; +import com.yahoo.config.provision.TenantName; import com.yahoo.io.IOUtils; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.ConfigPayload; @@ -19,27 +21,24 @@ import com.yahoo.vespa.config.GetConfigRequest; import com.yahoo.vespa.config.protocol.ConfigResponse; import com.yahoo.vespa.config.protocol.DefContent; import com.yahoo.vespa.config.protocol.VespaVersion; -import com.yahoo.vespa.config.server.application.ApplicationSet; -import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.ReloadListener; import com.yahoo.vespa.config.server.ServerCache; import com.yahoo.vespa.config.server.TestComponentRegistry; -import com.yahoo.vespa.config.server.rpc.UncompressedConfigResponseFactory; import com.yahoo.vespa.config.server.application.Application; -import com.yahoo.config.provision.ApplicationId; -import com.yahoo.config.provision.TenantName; +import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.deploy.ZooKeeperDeployer; +import com.yahoo.vespa.config.server.host.HostRegistries; import com.yahoo.vespa.config.server.model.TestModelFactory; import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; import com.yahoo.vespa.config.server.monitoring.MetricUpdater; import com.yahoo.vespa.config.server.monitoring.Metrics; +import com.yahoo.vespa.config.server.rpc.UncompressedConfigResponseFactory; import com.yahoo.vespa.config.server.session.RemoteSession; import com.yahoo.vespa.config.server.session.SessionZooKeeperClient; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.model.VespaModel; import com.yahoo.vespa.model.VespaModelFactory; - import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -48,11 +47,22 @@ import org.xml.sax.SAXException; import java.io.File; import java.io.IOException; -import java.util.*; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen @@ -83,7 +93,7 @@ public class TenantRequestHandlerTest { Metrics sh = Metrics.createTestMetrics(); List<ReloadListener> listeners = new ArrayList<>(); listeners.add(listener); - server = new TenantRequestHandler(sh, tenant, listeners, new UncompressedConfigResponseFactory(), new HostRegistries()); + server = new TenantRequestHandler(sh, tenant, listeners, new UncompressedConfigResponseFactory(), new HostRegistries(), curator); componentRegistry = new TestComponentRegistry.Builder() .curator(curator) .modelFactoryRegistry(createRegistry()) @@ -166,6 +176,8 @@ public class TenantRequestHandlerTest { public void testReloadConfig() throws IOException { ApplicationId applicationId = new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build(); + server.applications().createApplication(applicationId); + server.applications().createPutTransaction(applicationId, 1).commit(); server.reloadConfig(reloadConfig(1)); assertThat(listener.reloaded.get(), is(1)); // Using only payload list for this simple test @@ -185,6 +197,7 @@ public class TenantRequestHandlerTest { listener.reloaded.set(0); feedApp(app2, 2, defaultApp(), true); + server.applications().createPutTransaction(applicationId, 2).commit(); server.reloadConfig(reloadConfig(2L)); configResponse = getConfigResponse(SimpletypesConfig.class, server, defaultApp(), vespaVersion, ""); assertTrue(configResponse.isInternalRedeploy()); @@ -196,19 +209,34 @@ public class TenantRequestHandlerTest { @Test public void testRemoveApplication() { + ApplicationId appId = ApplicationId.from(tenant.value(), "default", "default"); + server.reloadConfig(reloadConfig(1)); + assertThat(listener.reloaded.get(), is(0)); + + server.applications().createApplication(appId); + server.applications().createPutTransaction(appId, 1).commit(); server.reloadConfig(reloadConfig(1)); + assertThat(listener.reloaded.get(), is(1)); + assertThat(listener.removed.get(), is(0)); - server.removeApplication(new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build()); + + server.removeApplication(appId); + assertThat(listener.removed.get(), is(0)); + + server.applications().createDeleteTransaction(appId).commit(); + server.removeApplication(appId); assertThat(listener.removed.get(), is(1)); } @Test public void testResolveForAppId() { long id = 1L; - SessionZooKeeperClient zkc = new SessionZooKeeperClient(curator, TenantRepository.getSessionsPath(tenant).append(String.valueOf(id))); ApplicationId appId = new ApplicationId.Builder() .tenant(tenant) .applicationName("myapp").instanceName("myinst").build(); + server.applications().createApplication(appId); + server.applications().createPutTransaction(appId, 1).commit(); + SessionZooKeeperClient zkc = new SessionZooKeeperClient(curator, TenantRepository.getSessionsPath(tenant).append(String.valueOf(id))); zkc.writeApplicationId(appId); RemoteSession session = new RemoteSession(appId.tenant(), id, componentRegistry, zkc); server.reloadConfig(session.ensureApplicationLoaded()); @@ -246,6 +274,8 @@ public class TenantRequestHandlerTest { } private void feedAndReloadApp(File appDir, long sessionId, ApplicationId appId) throws IOException { + server.applications().createApplication(appId); + server.applications().createPutTransaction(appId, sessionId).commit(); feedApp(appDir, sessionId, appId, false); SessionZooKeeperClient zkc = new SessionZooKeeperClient(curator, TenantRepository.getSessionsPath(tenant).append(String.valueOf(sessionId))); zkc.writeApplicationId(appId); @@ -281,9 +311,11 @@ public class TenantRequestHandlerTest { @Test public void testHasApplication() { assertdefaultAppNotFound(); + ApplicationId appId = ApplicationId.from(tenant.value(), "default", "default"); + server.applications().createApplication(appId); + server.applications().createPutTransaction(appId, 1).commit(); server.reloadConfig(reloadConfig(1)); - assertTrue(server.hasApplication(new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build(), - Optional.of(vespaVersion))); + assertTrue(server.hasApplication(appId, Optional.of(vespaVersion))); } private void assertdefaultAppNotFound() { @@ -292,10 +324,13 @@ public class TenantRequestHandlerTest { @Test public void testMultipleApplicationsReload() { + ApplicationId appId = ApplicationId.from(tenant.value(), "foo", "default"); assertdefaultAppNotFound(); + server.applications().createApplication(appId); + server.applications().createPutTransaction(appId, 1).commit(); server.reloadConfig(reloadConfig(1, "foo")); assertdefaultAppNotFound(); - assertTrue(server.hasApplication(new ApplicationId.Builder().tenant(tenant).applicationName("foo").build(), + assertTrue(server.hasApplication(appId, Optional.of(vespaVersion))); assertThat(server.resolveApplicationId("doesnotexist"), is(ApplicationId.defaultId())); assertThat(server.resolveApplicationId("mytesthost"), is(new ApplicationId.Builder() @@ -308,6 +343,8 @@ public class TenantRequestHandlerTest { assertdefaultAppNotFound(); VespaModel model = new VespaModel(FilesApplicationPackage.fromFile(new File("src/test/apps/app"))); + server.applications().createApplication(ApplicationId.defaultId()); + server.applications().createPutTransaction(ApplicationId.defaultId(), 1).commit(); server.reloadConfig(ApplicationSet.fromSingle(new Application(model, new ServerCache(), 1, 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 5e5b3546ced..6ac2466ee7b 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 2525dbecca2..268362052f5 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)); } diff --git a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java index 899737ec7fb..d1ce744de7f 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java @@ -26,6 +26,7 @@ public class CuratorCompletionWaiterTest { fail("Waiting failed due to timeout"); } }); + t1.start(); notifier.notifyCompletion(); t1.join(); } |