diff options
author | Harald Musum <musum@verizonmedia.com> | 2019-03-01 14:00:18 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-03-01 14:00:18 +0100 |
commit | a6f36b27934a220f80b30c65b47739c745b51697 (patch) | |
tree | a9aabcccb8dc7b9096d315f4d1e3b72a0760135f | |
parent | 47e68ff16cc3bca6fc04b0efbe8ae3b96bb61a93 (diff) |
Revert "Jvenstad/fix config model inconsitency"
31 files changed, 293 insertions, 374 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 481f071a32e..ea7b6a88a9c 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,7 +53,6 @@ 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; @@ -70,7 +69,6 @@ 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; @@ -264,10 +262,8 @@ 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 || ! 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 (tenant == null) return Optional.empty(); + LocalSession activeSession = getActiveSession(tenant, application); if (activeSession == null) return Optional.empty(); return Optional.of(Instant.ofEpochSecond(activeSession.getCreateTime())); } @@ -300,35 +296,34 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye if (tenant == null) return false; TenantApplications tenantApplications = tenant.getApplicationRepo(); - 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; - } + 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; + } - 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)); + 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)); - 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; } @@ -424,7 +419,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye Set<ApplicationId> listApplications() { return tenantRepository.getAllTenants().stream() - .flatMap(tenant -> tenant.getApplicationRepo().activeApplications().stream()) + .flatMap(tenant -> tenant.getApplicationRepo().listApplications().stream()) .collect(Collectors.toSet()); } @@ -483,7 +478,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.requireActiveSessionOf(applicationId); + return applicationRepo.getSessionIdForApplication(applicationId); } public void validateThatRemoteSessionIsNotActive(Tenant tenant, long sessionId) { @@ -525,7 +520,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye public long createSession(ApplicationId applicationId, TimeoutBudget timeoutBudget, File applicationDirectory) { Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); - tenant.getApplicationRepo().createApplication(applicationId); LocalSessionRepo localSessionRepo = tenant.getLocalSessionRepo(); SessionFactory sessionFactory = tenant.getSessionFactory(); LocalSession session = sessionFactory.createSession(applicationDirectory, applicationId, timeoutBudget); @@ -566,7 +560,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } private List<ApplicationId> activeApplications(TenantName tenantName) { - return tenantRepository.getTenant(tenantName).getApplicationRepo().activeApplications(); + return tenantRepository.getTenant(tenantName).getApplicationRepo().listApplications(); } // ---------------- Misc operations ---------------------------------------------------------------- @@ -617,7 +611,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye Optional<ApplicationSet> currentActiveApplicationSet = Optional.empty(); TenantApplications applicationRepo = tenant.getApplicationRepo(); try { - long currentActiveSessionId = applicationRepo.requireActiveSessionOf(appId); + long currentActiveSessionId = applicationRepo.getSessionIdForApplication(appId); RemoteSession currentActiveSession = getRemoteSession(tenant, currentActiveSessionId); if (currentActiveSession != null) { currentActiveApplicationSet = Optional.ofNullable(currentActiveSession.ensureApplicationLoaded()); @@ -647,7 +641,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye private List<ApplicationId> listApplicationIds(Tenant tenant) { TenantApplications applicationRepo = tenant.getApplicationRepo(); - return applicationRepo.activeApplications(); + return applicationRepo.listApplications(); } private void cleanupTempDirectory(File tempDir) { @@ -659,13 +653,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.requireActiveSessionOf(applicationId)); + return getLocalSession(tenant, applicationRepo.getSessionIdForApplication(applicationId)); } private LocalSession getActiveSession(Tenant tenant, ApplicationId applicationId) { TenantApplications applicationRepo = tenant.getApplicationRepo(); - if (applicationRepo.activeApplications().contains(applicationId)) { - return tenant.getLocalSessionRepo().getSession(applicationRepo.requireActiveSessionOf(applicationId)); + if (applicationRepo.listApplications().contains(applicationId)) { + return tenant.getLocalSessionRepo().getSession(applicationRepo.getSessionIdForApplication(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 81eb4107089..2c4d3d99408 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,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.application; +import com.google.common.collect.ImmutableSet; import com.yahoo.concurrent.ThreadFactoryFactory; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; @@ -11,32 +12,25 @@ 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.time.Duration; +import java.util.ArrayList; import java.util.List; -import java.util.Map; -import java.util.OptionalLong; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; +import java.util.Optional; 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 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>. + * 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. * * @author Ulf Lilleengen - * @author jonmv */ public class TenantApplications { @@ -44,20 +38,17 @@ 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, ReloadHandler reloadHandler, TenantName tenant) { + private TenantApplications(Curator curator, Path applicationsPath, ReloadHandler reloadHandler, TenantName tenant) { this.curator = curator; - this.applicationsPath = TenantRepository.getApplicationsPath(tenant); - this.locksPath = TenantRepository.getLocksPath(tenant); - this.locks = new ConcurrentHashMap<>(2); + this.applicationsPath = applicationsPath; + curator.create(applicationsPath); this.reloadHandler = reloadHandler; this.tenant = tenant; this.directoryCache = curator.createDirectoryCache(applicationsPath.getAbsolute(), false, false, pathChildrenExecutor); @@ -66,7 +57,11 @@ public class TenantApplications { } public static TenantApplications create(Curator curator, ReloadHandler reloadHandler, TenantName tenant) { - return new TenantApplications(curator, reloadHandler, 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); + } } /** @@ -74,103 +69,75 @@ public class TenantApplications { * * @return a list of {@link ApplicationId}s that are active. */ - 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. + public List<ApplicationId> listApplications() { try { - 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)); + List<String> appNodes = curator.framework().getChildren().forPath(applicationsPath.getAbsolute()); + List<ApplicationId> applicationIds = new ArrayList<>(); + for (String appNode : appNodes) { + parseApplication(appNode).ifPresent(applicationIds::add); } - catch (RuntimeException e) { - log.log(LogLevel.WARNING, TenantRepository.logPre(tenant) + "Failed to clean up stray node '" + appNode + "'!", e); - } - return false; + return applicationIds; + } catch (Exception e) { + throw new RuntimeException(TenantRepository.logPre(tenant)+"Unable to list applications", e); } } - 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)); + 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(); + } } /** - * Returns a transaction which writes the given session id as the currently active for the given application. + * Register active application and adds it to the repo. If it already exists it is overwritten. * * @param applicationId An {@link ApplicationId} that represents an active application. * @param sessionId Id of the session containing the application package for this id. */ - public Transaction createPutTransaction(ApplicationId applicationId, long sessionId) { - return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId))); - } - - /** - * Creates a node for the given application, marking its existence. - */ - public void createApplication(ApplicationId id) { - try (Lock lock = lock(id)) { - curator.create(applicationPath(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))); } } /** - * Return the active session id for a given application. + * Return the stored session id for a given application. * * @param applicationId an {@link ApplicationId} * @return session id of given application id. - * @throws IllegalArgumentException if the application has no active session - */ - public long requireActiveSessionOf(ApplicationId applicationId) { - return activeSessionOf(applicationId) - .orElseThrow(() -> new IllegalArgumentException("Application '" + applicationId + "' has no active session.")); - } - - /** - * Returns a transaction which deletes this application. + * @throws IllegalArgumentException if the application does not exist */ - public CuratorTransaction createDeleteTransaction(ApplicationId applicationId) { - return CuratorTransaction.from(CuratorOperations.deleteAll(applicationPath(applicationId).getAbsolute(), curator), curator); + 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); + } } /** - * Removes all applications not known to this from the config server state. + * Returns a transaction which deletes this application + * + * @param applicationId an {@link ApplicationId} to delete. */ - public void removeUnusedApplications() { - reloadHandler.removeApplicationsExcept(Set.copyOf(activeApplications())); + public CuratorTransaction deleteApplication(ApplicationId applicationId) { + Path path = applicationsPath.append(applicationId.serializedForm()); + return CuratorTransaction.from(CuratorOperations.delete(path.getAbsolute()), curator); } /** - * 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: @@ -200,12 +167,13 @@ public class TenantApplications { log.log(LogLevel.DEBUG, TenantRepository.logPre(applicationId) + "Application added: " + applicationId); } - private Path applicationPath(ApplicationId id) { - return applicationsPath.append(id.serializedForm()); - } - - private Path lockPath(ApplicationId id) { - return locksPath.append(id.serializedForm()); + /** + * Removes unused applications + * + */ + public void removeUnusedApplications() { + ImmutableSet<ApplicationId> activeApplications = ImmutableSet.copyOf(listApplications()); + reloadHandler.removeApplicationsExcept(activeApplications); } } 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 6d875c529a3..21716730825 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,8 +89,9 @@ public class Deployment implements com.yahoo.config.provision.Deployment { timeout, clock, true, true, session.getVespaVersion(), isBootstrap); } - public void setIgnoreSessionStaleFailure(boolean ignoreSessionStaleFailure) { + public Deployment setIgnoreSessionStaleFailure(boolean ignoreSessionStaleFailure) { this.ignoreSessionStaleFailure = ignoreSessionStaleFailure; + return this; } /** Prepares this. This does nothing if this is already prepared */ @@ -115,16 +116,13 @@ 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); - - try (Lock lock = tenant.getApplicationRepo().lock(session.getApplicationId())) { - if ( ! tenant.getApplicationRepo().exists(session.getApplicationId())) - return; // Application was deleted. - - validateSessionStatus(session); + long sessionId = session.getSessionId(); + validateSessionStatus(session); + try (Lock lock = tenant.getSessionLock(timeout)) { NestedTransaction transaction = new NestedTransaction(); transaction.add(deactivateCurrentActivateNew(applicationRepository.getActiveSession(session.getApplicationId()), session, ignoreSessionStaleFailure)); @@ -132,15 +130,13 @@ 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); } - - session.waitUntilActivated(timeoutBudget); - - log.log(LogLevel.INFO, session.logPre() + "Session " + session.getSessionId() + + log.log(LogLevel.INFO, session.logPre() + "Session " + sessionId + " activated successfully using " + ( hostProvisioner.isPresent() ? hostProvisioner.get() : "no host provisioner" ) + ". Config generation " + session.getMetaData().getGeneration()); @@ -158,7 +154,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 a7f8b8164a5..77572856ff5 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,7 +36,6 @@ 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 cdf995b80bb..6345532d4ff 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.activeApplications(); + return applicationRepo.listApplications(); } 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 af8956803ab..0f9f8b72de1 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,6 +83,12 @@ 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); } @@ -93,10 +99,8 @@ public class LocalSession extends Session implements Comparable<LocalSession> { public Transaction createActivateTransaction() { zooKeeperClient.createActiveWaiter(); - 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; + superModelGenerationCounter.increment(); + return setActive(); } 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 3f1619882cd..172d9c025d5 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.log.LogLevel; import com.yahoo.transaction.Transaction; +import com.yahoo.log.LogLevel; 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 ApplicationSet applicationSet = null; + private volatile ApplicationSet applicationSet = null; private final ActivatedModelsBuilder applicationLoader; private final Clock clock; @@ -69,15 +69,18 @@ public class RemoteSession extends Session { clock.instant())); } - public synchronized ApplicationSet ensureApplicationLoaded() { - return applicationSet == null ? applicationSet = loadApplication() : applicationSet; + public ApplicationSet ensureApplicationLoaded() { + if (applicationSet == null) { + applicationSet = loadApplication(); + } + return applicationSet; } public Session.Status getStatus() { return zooKeeperClient.readStatus(); } - public synchronized void deactivate() { + public 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 bbb06515bef..15182813a22 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,36 +1,33 @@ // 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.ReloadHandler; +import com.yahoo.vespa.config.server.application.ApplicationSet; 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 org.apache.curator.framework.CuratorFramework; -import org.apache.curator.framework.recipes.cache.ChildData; -import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; +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 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; +import org.apache.curator.framework.CuratorFramework; +import org.apache.curator.framework.recipes.cache.*; /** * Will watch/prepare sessions (applications) based on watched nodes in ZooKeeper, set for example @@ -118,6 +115,19 @@ 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()) @@ -180,16 +190,15 @@ public class RemoteSessionRepo extends SessionRepo<RemoteSession> { } private void loadSessionIfActive(RemoteSession session) { - for (ApplicationId applicationId : applicationRepo.activeApplications()) { + for (ApplicationId applicationId : applicationRepo.listApplications()) { try { - if (applicationRepo.requireActiveSessionOf(applicationId) == session.getSessionId()) { + if (applicationRepo.getSessionIdForApplication(applicationId) == session.getSessionId()) { log.log(LogLevel.DEBUG, "Found active application for session " + session.getSessionId() + " , loading it"); - reloadHandler.reloadConfig(session.ensureApplicationLoaded()); - log.log(LogLevel.INFO, session.logPre() + "Application activated successfully: " + applicationId); - return; + loadActiveSession(session); + break; } } catch (Exception e) { - log.log(LogLevel.WARNING, session.logPre() + "Skipping loading of application '" + applicationId + "': " + Exceptions.toMessageString(e)); + log.log(LogLevel.WARNING, session.logPre() + " error reading session id for " + applicationId, 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 5527d3060f7..a3dea83d50c 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,6 +17,8 @@ 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. @@ -27,10 +29,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 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. + * @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. * @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 d8ba5890545..b79ea720aea 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,11 +188,10 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { } private long getActiveSessionId(ApplicationId applicationId) { - List<ApplicationId> applicationIds = applicationRepo.activeApplications(); + List<ApplicationId> applicationIds = applicationRepo.listApplications(); if (applicationIds.contains(applicationId)) { - return applicationRepo.requireActiveSessionOf(applicationId); + return applicationRepo.getSessionIdForApplication(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 f0ceeb186fe..43a5ff6d0c2 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,8 +113,9 @@ public class SessionPreparer { log.log(LogLevel.DEBUG, () -> "time used " + params.getTimeoutBudget().timesUsed() + " : " + params.getApplicationId()); return preparation.result(); - } - catch (IllegalArgumentException e) { + } catch (OutOfCapacityException e) { + throw 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 88e71d7ddd1..a68f4a396cd 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,8 +11,10 @@ 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; @@ -28,7 +30,7 @@ public class Tenant implements TenantHandlerProvider { static final String SESSIONS = "sessions"; static final String APPLICATIONS = "applications"; - static final String LOCKS = "locks"; + static final String SESSION_LOCK_PATH = "activateLock"; private final TenantName name; private final RemoteSessionRepo remoteSessionRepo; @@ -36,6 +38,7 @@ 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; @@ -58,6 +61,7 @@ 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; @@ -106,6 +110,14 @@ 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(); @@ -153,4 +165,10 @@ 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 943ae6248fc..078b6e861a9 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 TenantRequestHandler reloadHandler; + private ReloadHandler reloadHandler; private RequestHandler requestHandler; private RemoteSessionFactory remoteSessionFactory; private TenantFileSystemDirs tenantFileSystemDirs; @@ -120,7 +120,7 @@ public class TenantBuilder { private void createApplicationRepo() { if (applicationRepo == null) { - applicationRepo = reloadHandler.applications(); + applicationRepo = TenantApplications.create(componentRegistry.getCurator(), reloadHandler, tenant); } } @@ -130,8 +130,7 @@ public class TenantBuilder { tenant, Collections.singletonList(componentRegistry.getReloadListener()), ConfigResponseFactory.create(componentRegistry.getConfigserverConfig()), - componentRegistry.getHostRegistries(), - componentRegistry.getCurator()); + componentRegistry.getHostRegistries()); if (hostValidator == null) { this.hostValidator = impl; } @@ -165,5 +164,9 @@ 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 c37db0e0df5..9c74c9c1e67 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,10 +262,7 @@ public class TenantRepository { */ private synchronized void writeTenantPath(TenantName name) { Path tenantPath = getTenantPath(name); - curator.createAtomically(tenantPath, - tenantPath.append(Tenant.SESSIONS), - tenantPath.append(Tenant.APPLICATIONS), - tenantPath.append(Tenant.LOCKS)); + curator.createAtomically(tenantPath, tenantPath.append(Tenant.SESSIONS), tenantPath.append(Tenant.APPLICATIONS)); } /** @@ -409,11 +406,4 @@ 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 3f2b8c76e44..a341061bd9a 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,7 +6,6 @@ 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; @@ -17,7 +16,6 @@ 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; @@ -31,8 +29,6 @@ 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 @@ -48,25 +44,23 @@ 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, - Curator curator) { // TODO jvenstad: Merge this class with TenantApplications, and straighten this out. + HostRegistries hostRegistries) { this.metrics = metrics; this.tenant = tenant; - this.reloadListeners = List.copyOf(reloadListeners); + this.reloadListeners = reloadListeners; this.responseFactory = responseFactory; - this.tenantMetricUpdater = metrics.getOrCreateMetricUpdater(Metrics.createDimensions(tenant)); - this.hostRegistry = hostRegistries.createApplicationHostRegistry(tenant); - this.applications = TenantApplications.create(curator, this, tenant); + tenantMetricUpdater = metrics.getOrCreateMetricUpdater(Metrics.createDimensions(tenant)); + hostRegistry = hostRegistries.createApplicationHostRegistry(tenant); } /** @@ -99,40 +93,26 @@ public class TenantRequestHandler implements RequestHandler, ReloadHandler, Host * * @param applicationSet the {@link ApplicationSet} to be reloaded */ - @Override public void reloadConfig(ApplicationSet 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); - } + setLiveApp(applicationSet); + notifyReloadListeners(applicationSet); } @Override public void removeApplication(ApplicationId 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)); - } + 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); } @@ -257,6 +237,4 @@ 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 7a4998215e3..3d94dedf651 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().requireActiveSessionOf(tester.applicationId()); + long activeSessionId = tester.tenant().getApplicationRepo().getSessionIdForApplication(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().requireActiveSessionOf(tester.applicationId())); + assertEquals(activeSessionId, tester.tenant().getApplicationRepo().getSessionIdForApplication(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 69c88dc0275..a708e4d8ace 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.activeApplications(); + List<ApplicationId> applications = repo.listApplications(); assertThat(applications.size(), is(2)); - 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)); + 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)); } @Test @@ -52,7 +52,7 @@ public class TenantApplicationsTest { writeApplicationData(createApplicationId("foo"), 3L); writeApplicationData("invalid", 3L); TenantApplications repo = createZKAppRepo(); - List<ApplicationId> applications = repo.activeApplications(); + List<ApplicationId> applications = repo.listApplications(); 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.requireActiveSessionOf(createApplicationId("nonexistent")); + repo.getSessionIdForApplication(createApplicationId("nonexistent")); } @Test(expected = IllegalArgumentException.class) @@ -70,19 +70,18 @@ public class TenantApplicationsTest { curatorFramework.create().creatingParentsIfNeeded() .forPath(TenantRepository.getApplicationsPath(tenantName).append(baz.serializedForm()).getAbsolute()); TenantApplications repo = createZKAppRepo(); - repo.requireActiveSessionOf(baz); + repo.getSessionIdForApplication(baz); } @Test public void require_that_application_ids_can_be_written() throws Exception { TenantApplications repo = createZKAppRepo(); ApplicationId myapp = createApplicationId("myapp"); - repo.createApplication(myapp); - repo.createPutTransaction(myapp, 3l).commit(); + repo.createPutApplicationTransaction(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.createPutTransaction(myapp, 5l).commit(); + repo.createPutApplicationTransaction(myapp, 5l).commit(); assertTrue(curatorFramework.checkExists().forPath(path) != null); assertThat(Utf8.toString(curatorFramework.getData().forPath(path)), is("5")); } @@ -92,15 +91,13 @@ public class TenantApplicationsTest { TenantApplications repo = createZKAppRepo(); ApplicationId id1 = createApplicationId("myapp"); ApplicationId id2 = createApplicationId("myapp2"); - 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)); + 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)); } @Test @@ -111,7 +108,7 @@ public class TenantApplicationsTest { MockReloadHandler reloadHandler = new MockReloadHandler(); TenantApplications repo = createZKAppRepo(reloadHandler); assertNull(reloadHandler.lastRemoved); - repo.createDeleteTransaction(foo).commit(); + repo.deleteApplication(foo).commit(); long endTime = System.currentTimeMillis() + 60_000; while (System.currentTimeMillis() < endTime && reloadHandler.lastRemoved == null) { Thread.sleep(100); @@ -129,7 +126,11 @@ public class TenantApplicationsTest { } private static ApplicationId createApplicationId(String name) { - return ApplicationId.from(tenantName.value(), name, "myinst"); + return new ApplicationId.Builder() + .tenant(tenantName.value()) + .applicationName(name) + .instanceName("myinst") + .build(); } 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 6b67dcc4e9a..7ffb9552cf8 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() - .requireActiveSessionOf(applicationId)); + .getSessionIdForApplication(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 c6a8e1f2f9d..b0bb3bf244f 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,14 +56,12 @@ 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().createApplication(idTenant1); - tenant1.getApplicationRepo().createPutTransaction(idTenant1, 2l).commit(); + tenant1.getApplicationRepo().createPutApplicationTransaction(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().createApplication(idTenant2); - tenant2.getApplicationRepo().createPutTransaction(idTenant2, 3l).commit(); + tenant2.getApplicationRepo().createPutApplicationTransaction(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 bfd6a35cc2e..2c84e2d8ad4 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().requireActiveSessionOf(applicationId); + long sessionId = tenantRepository.getTenant(applicationId.tenant()).getApplicationRepo().getSessionIdForApplication(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().requireActiveSessionOf(applicationId); + long sessionId = tenant.getApplicationRepo().getSessionIdForApplication(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 1db70956407..2d3dcc592f7 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,8 +46,7 @@ public class HostHandlerTest { private HostHandler hostHandler; static void addMockApplication(Tenant tenant, ApplicationId applicationId, long sessionId) { - tenant.getApplicationRepo().createApplication(applicationId); - tenant.getApplicationRepo().createPutTransaction(applicationId, sessionId).commit(); + tenant.getApplicationRepo().createPutApplicationTransaction(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 f97bc443a38..f57e7f09b39 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, "[]"); - ApplicationId id1 = ApplicationId.from("mytenant", "foo", "quux"); - applicationRepo.createApplication(id1); - applicationRepo.createPutTransaction(id1, 1).commit(); + applicationRepo.createPutApplicationTransaction( + new ApplicationId.Builder().tenant("tenant").applicationName("foo").instanceName("quux").build(), + 1).commit(); assertResponse(url, Response.Status.OK, "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]"); - ApplicationId id2 = ApplicationId.from("mytenant", "bali", "quux"); - applicationRepo.createApplication(id2); - applicationRepo.createPutTransaction(id2, 1).commit(); + applicationRepo.createPutApplicationTransaction( + new ApplicationId.Builder().tenant("tenant").applicationName("bali").instanceName("quux").build(), + 1).commit(); assertResponse(url, Response.Status.OK, - "[\"" + url + "bali/environment/dev/region/us-east/instance/quux\"," + - "\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]" + "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"," + + "\"" + url + "bali/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 { - 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(); + 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(); 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 858d1e0eaa7..380b76c30af 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java @@ -323,9 +323,6 @@ 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 7f7f71a1fe8..94d3b126bd7 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,8 +203,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { .applicationName("foo") .instanceName("quux") .build(); - applicationRepo.createApplication(fooId); - applicationRepo.createPutTransaction(fooId, 2).commit(); + applicationRepo.createPutApplicationTransaction(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() @@ -212,8 +211,7 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { .applicationName("foobio") .instanceName("quux") .build(); - applicationRepo.createApplication(bioId); - applicationRepo.createPutTransaction(bioId, 5).commit(); + applicationRepo.createPutApplicationTransaction(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 a4432dcbfcd..e7db4dcf58f 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,14 +196,12 @@ 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, - applications, + TenantApplications.create(curator, new MockReloadHandler(), tenant), 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 d5d0fe72dbe..a7b74e69a21 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,26 +3,23 @@ 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.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.api.*; 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.vespa.config.server.TestComponentRegistry; +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.application.PermanentApplicationPackage; import com.yahoo.vespa.config.server.modelfactory.ModelFactoryRegistry; -import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.config.server.TestComponentRegistry; +import com.yahoo.vespa.config.server.application.PermanentApplicationPackage; import com.yahoo.vespa.curator.mock.MockCurator; +import com.yahoo.vespa.curator.Curator; 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 9bd5c5f1614..4046384005d 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,8 +81,6 @@ 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 26a6f5e0c5f..5f18046cb81 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,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.tenant; -import com.yahoo.component.Version; import com.yahoo.config.ConfigInstance; import com.yahoo.config.SimpletypesConfig; import com.yahoo.config.application.api.ApplicationPackage; @@ -10,10 +9,9 @@ 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.AllocatedHosts; -import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.ApplicationName; -import com.yahoo.config.provision.TenantName; +import com.yahoo.config.provision.AllocatedHosts; +import com.yahoo.component.Version; import com.yahoo.io.IOUtils; import com.yahoo.vespa.config.ConfigKey; import com.yahoo.vespa.config.ConfigPayload; @@ -21,24 +19,27 @@ 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.vespa.config.server.application.ApplicationSet; +import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.TenantName; 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; @@ -47,22 +48,11 @@ import org.xml.sax.SAXException; import java.io.File; import java.io.IOException; -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.*; import java.util.concurrent.atomic.AtomicInteger; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertThat; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; /** * @author Ulf Lilleengen @@ -93,7 +83,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(), curator); + server = new TenantRequestHandler(sh, tenant, listeners, new UncompressedConfigResponseFactory(), new HostRegistries()); componentRegistry = new TestComponentRegistry.Builder() .curator(curator) .modelFactoryRegistry(createRegistry()) @@ -176,8 +166,6 @@ 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 @@ -197,7 +185,6 @@ 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()); @@ -209,34 +196,19 @@ 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(appId); - assertThat(listener.removed.get(), is(0)); - - server.applications().createDeleteTransaction(appId).commit(); - server.removeApplication(appId); + server.removeApplication(new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build()); 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()); @@ -274,8 +246,6 @@ 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); @@ -311,11 +281,9 @@ 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(appId, Optional.of(vespaVersion))); + assertTrue(server.hasApplication(new ApplicationId.Builder().applicationName(ApplicationName.defaultName()).tenant(tenant).build(), + Optional.of(vespaVersion))); } private void assertdefaultAppNotFound() { @@ -324,13 +292,10 @@ 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(appId, + assertTrue(server.hasApplication(new ApplicationId.Builder().tenant(tenant).applicationName("foo").build(), Optional.of(vespaVersion))); assertThat(server.resolveApplicationId("doesnotexist"), is(ApplicationId.defaultId())); assertThat(server.resolveApplicationId("mytesthost"), is(new ApplicationId.Builder() @@ -343,8 +308,6 @@ 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 6ac2466ee7b..5e5b3546ced 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,6 +51,11 @@ 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. * @@ -70,18 +75,19 @@ public abstract class ApplicationMaintainer extends Maintainer { /** Returns the applications that should be maintained by this now. */ protected abstract Set<ApplicationId> applicationsNeedingMaintenance(); - /** Redeploy this application. */ + /** Redeploy this application. A lock will be taken for the duration of the deployment activation */ 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. - 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(); - }); + // + // 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(); } 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 268362052f5..2525dbecca2 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,7 +38,8 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer { this.start = clock.instant(); } - private boolean canDeployNow(ApplicationId application) { + @Override + protected 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 d1ce744de7f..899737ec7fb 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorCompletionWaiterTest.java @@ -26,7 +26,6 @@ public class CuratorCompletionWaiterTest { fail("Waiting failed due to timeout"); } }); - t1.start(); notifier.notifyCompletion(); t1.join(); } |