From ff46131d4828e2848e36aed2e445a65919a46450 Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Wed, 27 Feb 2019 12:15:30 +0100 Subject: Minor cleanup --- .../server/application/TenantApplications.java | 74 +++++++++++----------- .../vespa/config/server/deploy/Deployment.java | 3 +- .../vespa/config/server/session/LocalSession.java | 12 ++-- .../config/server/session/SessionFactory.java | 8 +-- .../config/server/session/SessionFactoryImpl.java | 1 + 5 files changed, 45 insertions(+), 53 deletions(-) (limited to 'configserver') 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 7731e13eac2..3705a0ec145 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; @@ -17,12 +16,13 @@ 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.util.List; -import java.util.Optional; +import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.logging.Logger; +import java.util.stream.Collectors; +import java.util.stream.Stream; /** * The applications of a tenant, backed by ZooKeeper. @@ -70,40 +70,39 @@ public class TenantApplications { * @return a list of {@link ApplicationId}s that are active. */ public List listApplications() { - try { - List appNodes = curator.framework().getChildren().forPath(applicationsPath.getAbsolute()); - List applicationIds = new ArrayList<>(); - for (String appNode : appNodes) { - parseApplication(appNode).ifPresent(applicationIds::add); - } - return applicationIds; - } catch (Exception e) { - throw new RuntimeException(TenantRepository.logPre(tenant)+"Unable to list applications", e); - } + return curator.getChildren(applicationsPath).stream() + .flatMap(this::parseApplication) + .collect(Collectors.toUnmodifiableList()); } - private Optional parseApplication(String appNode) { + // TODO jvenstad: Remove after it has run once everywhere. + private Stream parseApplication(String appNode) { try { - ApplicationId id = ApplicationId.fromSerializedForm(appNode); - getSessionIdForApplication(id); - return Optional.of(id); - } catch (IllegalArgumentException e) { - log.log(LogLevel.INFO, TenantRepository.logPre(tenant)+"Unable to parse application with id '" + appNode + "', ignoring."); - return Optional.empty(); + return Stream.of(ApplicationId.fromSerializedForm(appNode)); + } 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)); + } + catch (Exception e) { + log.log(LogLevel.WARNING, TenantRepository.logPre(tenant) + "Failed to clean up stray node '" + appNode + "'!", e); + } + return Stream.empty(); } } /** - * 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))); + return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId))); } else { - return new CuratorTransaction(curator).add(CuratorOperations.create(applicationsPath.append(applicationId.serializedForm()).getAbsolute(), Utf8.toAsciiBytes(sessionId))); + return new CuratorTransaction(curator).add(CuratorOperations.create(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId))); } } @@ -115,7 +114,7 @@ public class TenantApplications { * @throws IllegalArgumentException if the application does not exist */ public long getSessionIdForApplication(ApplicationId applicationId) { - String path = applicationsPath.append(applicationId.serializedForm()).getAbsolute(); + String path = applicationPath(applicationId).getAbsolute(); try { return Long.parseLong(Utf8.toString(curator.framework().getData().forPath(path))); } catch (Exception e) { @@ -124,18 +123,22 @@ public class TenantApplications { } /** - * Returns a transaction which deletes this application - * - * @param applicationId an {@link ApplicationId} to delete. + * Returns a transaction which deletes this application. */ public CuratorTransaction deleteApplication(ApplicationId applicationId) { - Path path = applicationsPath.append(applicationId.serializedForm()); - return CuratorTransaction.from(CuratorOperations.delete(path.getAbsolute()), curator); + return CuratorTransaction.from(CuratorOperations.delete(applicationPath(applicationId).getAbsolute()), curator); } /** - * Closes the application repo. Once a repo has been closed, it should not be used again. - */ + * Removes all applications not known to this from the config server state. + */ + public void removeUnusedApplications() { + reloadHandler.removeApplicationsExcept(Set.copyOf(listApplications())); + } + + /** + * Closes the application repo. Once a repo has been closed, it should not be used again. + */ public void close() { directoryCache.close(); } @@ -169,13 +172,8 @@ public class TenantApplications { log.log(LogLevel.DEBUG, TenantRepository.logPre(applicationId) + "Application added: " + applicationId); } - /** - * Removes unused applications - * - */ - public void removeUnusedApplications() { - ImmutableSet activeApplications = ImmutableSet.copyOf(listApplications()); - reloadHandler.removeApplicationsExcept(activeApplications); + private Path applicationPath(ApplicationId id) { + return applicationsPath.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..082be2583c2 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 */ 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..0cdf5ebfe95 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 { 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 { 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.createPutApplicationTransaction(zooKeeperClient.readApplicationId(), getSessionId()).operations()); + return transaction; } public Transaction createDeactivateTransaction() { 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..90eeb89dc8e 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 @@ -194,4 +194,5 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { } return nonExistingActiveSession; } + } -- cgit v1.2.3