Merge pull request #13817 from vespa-engine/hmusum/configserver-refactoring-22

Config server refactoring, part 22

10 files changed, 69 insertions, 63 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 e57fc104dd3..8ba4581e158 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
@@ -306,8 +306,9 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         LocalSession activeSession = getActiveLocalSession(tenant, application);
         if (activeSession == null) return Optional.empty();
         TimeoutBudget timeoutBudget = new TimeoutBudget(clock, timeout);
-        LocalSession newSession = tenant.getSessionRepository().createSessionFromExisting(activeSession, logger, true, timeoutBudget);
-        tenant.getSessionRepository().addLocalSession(newSession);
+        SessionRepository sessionRepository = tenant.getSessionRepository();
+        LocalSession newSession = sessionRepository.createSessionFromExisting(activeSession, logger, true, timeoutBudget);
+        sessionRepository.addLocalSession(newSession);
 
         return Optional.of(Deployment.unprepared(newSession, this, hostProvisioner, tenant, timeout, clock,
                                                  false /* don't validate as this is already deployed */, bootstrap));
@@ -493,11 +494,10 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
 
     private Application getApplication(ApplicationId applicationId, Optional<Version> version) {
         try {
-            Tenant tenant = tenantRepository.getTenant(applicationId.tenant());
+            Tenant tenant = getTenant(applicationId);
             if (tenant == null) throw new NotFoundException("Tenant '" + applicationId.tenant() + "' not found");
             long sessionId = getSessionIdForApplication(tenant, applicationId);
-            RemoteSession session = tenant.getSessionRepository().getRemoteSession(sessionId);
-            if (session == null) throw new NotFoundException("Remote session " + sessionId + " not found");
+            RemoteSession session = getRemoteSession(tenant, sessionId);
             return session.ensureApplicationLoaded().getForVersionOrLatest(version, clock.instant());
         } catch (NotFoundException e) {
             log.log(Level.WARNING, "Failed getting application for '" + applicationId + "': " + e.getMessage());
@@ -625,15 +625,15 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return applicationRepo.requireActiveSessionOf(applicationId);
     }
 
-    public void validateThatRemoteSessionIsNotActive(Tenant tenant, long sessionId) {
-        RemoteSession session = getRemoteSession(tenant, sessionId);
+    public void validateThatSessionIsNotActive(Tenant tenant, long sessionId) {
+        Session session = getRemoteSession(tenant, sessionId);
         if (Session.Status.ACTIVATE.equals(session.getStatus())) {
             throw new IllegalStateException("Session is active: " + sessionId);
         }
     }
 
-    public void validateThatRemoteSessionIsPrepared(Tenant tenant, long sessionId) {
-        RemoteSession session = getRemoteSession(tenant, sessionId);
+    public void validateThatSessionIsPrepared(Tenant tenant, long sessionId) {
+        Session session = getRemoteSession(tenant, sessionId);
         if ( ! Session.Status.PREPARE.equals(session.getStatus()))
             throw new IllegalStateException("Session not prepared: " + sessionId);
     }
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 2136906f205..7337dc446f5 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
@@ -234,7 +234,6 @@ public class TenantApplications implements RequestHandler, HostValidator<Applica
         return locksPath.append(id.serializedForm());
     }
 
-
     /**
      * Gets a config for the given app, or null if not found
      */
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistry.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistry.java
index a10b8d9c6fb..ce582a8a1a8 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistry.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDBRegistry.java
@@ -20,7 +20,7 @@ import java.util.Optional;
 public class FileDBRegistry implements FileRegistry {
 
     private final AddFileInterface manager;
-    private List<Entry> entries = new ArrayList<>();
+    private final List<Entry> entries = new ArrayList<>();
     private final Map<String, FileReference> fileReferenceCache = new HashMap<>();
 
     public FileDBRegistry(AddFileInterface manager) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandler.java
index bade20f45ad..c0789a9c828 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandler.java
@@ -49,8 +49,8 @@ public class SessionPrepareHandler extends SessionHandler {
     protected HttpResponse handleGET(HttpRequest request) {
         Tenant tenant = getExistingTenant(request);
         long sessionId = getSessionIdV2(request);
-        applicationRepository.validateThatRemoteSessionIsNotActive(tenant, sessionId);
-        applicationRepository.validateThatRemoteSessionIsPrepared(tenant, sessionId);
+        applicationRepository.validateThatSessionIsNotActive(tenant, sessionId);
+        applicationRepository.validateThatSessionIsPrepared(tenant, sessionId);
         return new SessionPrepareResponse(applicationRepository.createDeployLog(), tenant.getName(), request, sessionId);
     }
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java
index 3dbb3fd11e3..54da80a0299 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizer.java
@@ -183,7 +183,7 @@ public class MultiTenantRpcAuthorizer implements RpcAuthorizer {
             log.log(Level.FINE, () -> String.format("Client '%s' identified as %s", request.target().toString(), identity.toString()));
             return Optional.of(identity);
         } catch (NodeIdentifierException e) {
-            throw new AuthorizationException("Failed to identity peer: " + e.getMessage(), e);
+            throw new AuthorizationException("Failed to identify peer: " + e.getMessage(), e);
         }
     }
 
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 66ed721a3e1..c520094e294 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
@@ -9,7 +9,6 @@ import com.yahoo.lang.SettableOptional;
 import com.yahoo.transaction.Transaction;
 import com.yahoo.vespa.config.server.GlobalComponentRegistry;
 import com.yahoo.vespa.config.server.application.ApplicationSet;
-import com.yahoo.vespa.config.server.application.TenantApplications;
 import com.yahoo.vespa.config.server.modelfactory.ActivatedModelsBuilder;
 import com.yahoo.vespa.curator.Curator;
 import org.apache.zookeeper.KeeperException;
@@ -49,7 +48,7 @@ public class RemoteSession extends Session {
         this.clock = componentRegistry.getClock();
     }
 
-    void loadPrepared() {
+    void prepare() {
         Curator.CompletionWaiter waiter = sessionZooKeeperClient.getPrepareWaiter();
         ensureApplicationLoaded();
         notifyCompletion(waiter);
@@ -80,17 +79,6 @@ public class RemoteSession extends Session {
     public Transaction createDeleteTransaction() {
         return sessionZooKeeperClient.createWriteStatusTransaction(Status.DELETE);
     }
-
-    void makeActive(TenantApplications tenantApplications) {
-        Curator.CompletionWaiter waiter = sessionZooKeeperClient.getActiveWaiter();
-        log.log(Level.FINE, () -> logPre() + "Getting session from repo: " + getSessionId());
-        ApplicationSet app = ensureApplicationLoaded();
-        log.log(Level.FINE, () -> logPre() + "Reloading config for " + getSessionId());
-        tenantApplications.reloadConfig(app);
-        log.log(Level.FINE, () -> logPre() + "Notifying " + waiter);
-        notifyCompletion(waiter);
-        log.log(Level.INFO, logPre() + "Session activated: " + getSessionId());
-    }
     
     void confirmUpload() {
         Curator.CompletionWaiter waiter = sessionZooKeeperClient.getUploadWaiter();
@@ -99,7 +87,7 @@ public class RemoteSession extends Session {
         log.log(Level.FINE, "Done notifying upload for session " + getSessionId());
     }
 
-    private void notifyCompletion(Curator.CompletionWaiter completionWaiter) {
+    void notifyCompletion(Curator.CompletionWaiter completionWaiter) {
         try {
             completionWaiter.notifyCompletion();
         } catch (RuntimeException e) {
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 798dcfcd8e7..b6b0ac45bb5 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
@@ -128,9 +128,7 @@ public class SessionPreparer {
                                                   tenantPath, serverDbSessionDir, applicationPackage, sessionZooKeeperClient);
 
         preparation.preprocess();
-
         var distributedApplicationPackage = preparation.distributeApplicationPackage();
-
         try {
             AllocatedHosts allocatedHosts = preparation.buildModels(now);
             preparation.makeResult(allocatedHosts);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
index 8a7be7ef176..8c3ab795f02 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java
@@ -70,7 +70,7 @@ public class SessionRepository {
 
     private static final Logger log = Logger.getLogger(SessionRepository.class.getName());
     private static final FilenameFilter sessionApplicationsFilter = (dir, name) -> name.matches("\\d+");
-    private static final long nonExistingActiveSession = 0;
+    private static final long nonExistingActiveSessionId = 0;
 
     private final SessionCache<LocalSession> localSessionCache = new SessionCache<>();
     private final SessionCache<RemoteSession> remoteSessionCache = new SessionCache<>();
@@ -108,22 +108,24 @@ public class SessionRepository {
         this.distributeApplicationPackage = Flags.CONFIGSERVER_DISTRIBUTE_APPLICATION_PACKAGE.bindTo(flagSource);
         this.metrics = componentRegistry.getMetrics().getOrCreateMetricUpdater(Metrics.createDimensions(tenantName));
         this.locksPath = TenantRepository.getLocksPath(tenantName);
-
-        loadLocalSessions();
-        initializeRemoteSessions();
+        loadSessions(); // Needs to be done before creating cache below
         this.directoryCache = curator.createDirectoryCache(sessionsPath.getAbsolute(), false, false, componentRegistry.getZkCacheExecutor());
         this.directoryCache.addListener(this::childEvent);
         this.directoryCache.start();
     }
 
+    private void loadSessions() {
+        loadLocalSessions();
+        initializeRemoteSessions();
+    }
+
     // ---------------- Local sessions ----------------------------------------------------------------
 
     public synchronized void addLocalSession(LocalSession session) {
         localSessionCache.addSession(session);
         long sessionId = session.getSessionId();
-        Curator.FileCache fileCache = curator.createFileCache(getSessionStatePath(sessionId).getAbsolute(), false);
         RemoteSession remoteSession = createRemoteSession(sessionId);
-        addSesssionStateWatcher(sessionId, fileCache, remoteSession, Optional.of(session));
+        addSessionStateWatcher(sessionId, remoteSession, Optional.of(session));
     }
 
     public LocalSession getLocalSession(long sessionId) {
@@ -211,18 +213,21 @@ public class SessionRepository {
             SessionStateWatcher watcher = sessionStateWatchers.remove(sessionId);
             if (watcher != null) watcher.close();
             localSessionCache.removeSession(sessionId);
-            NestedTransaction transaction = new NestedTransaction();
-            deleteLocalSession(session, transaction);
-            transaction.commit();
+            deletePersistentData(sessionId);
         }
     }
 
-    /** Add transactions to delete this session to the given nested transaction */
-    public void deleteLocalSession(LocalSession session, NestedTransaction transaction) {
-        long sessionId = session.getSessionId();
+    private void deletePersistentData(long sessionId) {
+        NestedTransaction transaction = new NestedTransaction();
         SessionZooKeeperClient sessionZooKeeperClient = createSessionZooKeeperClient(sessionId);
+
+        // We will try to delete data from zookeeper from several servers, but since we take a lock
+        // and the transaction will either delete everything or nothing (which will happen if it has been done
+        // on another server) this works fine
         transaction.add(sessionZooKeeperClient.deleteTransaction(), FileTransaction.class);
+
         transaction.add(FileTransaction.from(FileOperations.delete(getSessionAppDir(sessionId).getAbsolutePath())));
+        transaction.commit();
     }
 
     public void close() {
@@ -320,14 +325,32 @@ public class SessionRepository {
     public void sessionAdded(long sessionId) {
         log.log(Level.FINE, () -> "Adding remote session to SessionRepository: " + sessionId);
         RemoteSession remoteSession = createRemoteSession(sessionId);
-        Curator.FileCache fileCache = curator.createFileCache(getSessionStatePath(sessionId).getAbsolute(), false);
-        fileCache.addListener(this::nodeChanged);
         loadSessionIfActive(remoteSession);
         addRemoteSession(remoteSession);
         Optional<LocalSession> localSession = Optional.empty();
         if (distributeApplicationPackage())
             localSession = createLocalSessionUsingDistributedApplicationPackage(sessionId);
-        addSesssionStateWatcher(sessionId, fileCache, remoteSession, localSession);
+        addSessionStateWatcher(sessionId, remoteSession, localSession);
+    }
+
+    void activate(RemoteSession session) {
+        long sessionId = session.getSessionId();
+        Curator.CompletionWaiter waiter = createSessionZooKeeperClient(sessionId).getActiveWaiter();
+        log.log(Level.FINE, () -> session.logPre() + "Getting session from repo: " + sessionId);
+        ApplicationSet app = session.ensureApplicationLoaded();
+        log.log(Level.FINE, () -> session.logPre() + "Reloading config for " + sessionId);
+        applicationRepo.reloadConfig(app);
+        log.log(Level.FINE, () -> session.logPre() + "Notifying " + waiter);
+        session.notifyCompletion(waiter);
+        log.log(Level.INFO, session.logPre() + "Session activated: " + sessionId);
+    }
+
+    public void deactivate(RemoteSession remoteSession) {
+        remoteSession.deactivate();
+    }
+
+    void prepare(RemoteSession session) {
+        session.prepare();
     }
 
     boolean distributeApplicationPackage() {
@@ -428,7 +451,7 @@ public class SessionRepository {
             user = "unknown";
         }
         DeployData deployData = new DeployData(user, userDir.getAbsolutePath(), applicationId, deployTimestamp,
-                                               internalRedeploy, sessionId, currentlyActiveSessionId.orElse(nonExistingActiveSession));
+                                               internalRedeploy, sessionId, currentlyActiveSessionId.orElse(nonExistingActiveSessionId));
         return FilesApplicationPackage.fromFileWithDeployData(configApplicationDir, deployData);
     }
 
@@ -615,13 +638,15 @@ public class SessionRepository {
         return new TenantFileSystemDirs(componentRegistry.getConfigServerDB(), tenantName).getUserApplicationDir(sessionId);
     }
 
-    private void addSesssionStateWatcher(long sessionId, Curator.FileCache fileCache, RemoteSession remoteSession, Optional<LocalSession> localSession) {
+    private void addSessionStateWatcher(long sessionId, RemoteSession remoteSession, Optional<LocalSession> localSession) {
         // Remote session will always be present in an existing state watcher, but local session might not
         if (sessionStateWatchers.containsKey(sessionId)) {
             localSession.ifPresent(session -> sessionStateWatchers.get(sessionId).addLocalSession(session));
         } else {
-            sessionStateWatchers.put(sessionId, new SessionStateWatcher(fileCache, applicationRepo, remoteSession,
-                                                                        localSession, metrics, zkWatcherExecutor, this));
+            Curator.FileCache fileCache = curator.createFileCache(getSessionStatePath(sessionId).getAbsolute(), false);
+            fileCache.addListener(this::nodeChanged);
+            sessionStateWatchers.put(sessionId, new SessionStateWatcher(fileCache, remoteSession, localSession,
+                                                                        metrics, zkWatcherExecutor, this));
         }
     }
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java
index 65c62a392b7..d7c8928a6eb 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionStateWatcher.java
@@ -2,7 +2,6 @@
 package com.yahoo.vespa.config.server.session;
 
 import com.yahoo.text.Utf8;
-import com.yahoo.vespa.config.server.application.TenantApplications;
 import com.yahoo.vespa.config.server.monitoring.MetricUpdater;
 import com.yahoo.vespa.curator.Curator;
 import org.apache.curator.framework.recipes.cache.ChildData;
@@ -26,7 +25,6 @@ public class SessionStateWatcher {
     private static final Logger log = Logger.getLogger(SessionStateWatcher.class.getName());
 
     private final Curator.FileCache fileCache;
-    private final TenantApplications tenantApplications;
     private final RemoteSession remoteSession;
     private final MetricUpdater metrics;
     private final Executor zkWatcherExecutor;
@@ -34,14 +32,12 @@ public class SessionStateWatcher {
     private Optional<LocalSession> localSession;
 
     SessionStateWatcher(Curator.FileCache fileCache,
-                        TenantApplications tenantApplications,
                         RemoteSession remoteSession,
                         Optional<LocalSession> localSession,
                         MetricUpdater metrics,
                         Executor zkWatcherExecutor,
                         SessionRepository sessionRepository) {
         this.fileCache = fileCache;
-        this.tenantApplications = tenantApplications;
         this.remoteSession = remoteSession;
         this.localSession = localSession;
         this.metrics = metrics;
@@ -56,15 +52,15 @@ public class SessionStateWatcher {
 
         if (newStatus.equals(Status.PREPARE)) {
             createLocalSession(sessionId);
-            log.log(Level.FINE, remoteSession.logPre() + "Loading prepared session: " + sessionId);
-            remoteSession.loadPrepared();
+            log.log(Level.FINE, remoteSession.logPre() + "Preparing session: " + sessionId);
+            sessionRepository.prepare(remoteSession);
         } else if (newStatus.equals(Status.ACTIVATE)) {
             createLocalSession(sessionId);
-            remoteSession.makeActive(tenantApplications);
+            sessionRepository.activate(remoteSession);
         } else if (newStatus.equals(Status.DEACTIVATE)) {
-            remoteSession.deactivate();
+            sessionRepository.deactivate(remoteSession);
         } else if (newStatus.equals(Status.DELETE)) {
-            remoteSession.deactivate();
+            sessionRepository.deactivate(remoteSession);
             localSession.ifPresent(session -> {
                 log.log(Level.FINE, session.logPre() + "Deleting session " + sessionId);
                 sessionRepository.deleteLocalSession(session);
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 e90f01e98c4..490e1a4075c 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
@@ -75,7 +75,7 @@ public class RemoteSessionTest {
     @Test
     public void require_that_applications_are_loaded() {
         RemoteSession session = createSession(3, Arrays.asList(new MockModelFactory(), new VespaModelFactory(new NullConfigModelRegistry())));
-        session.loadPrepared();
+        session.prepare();
         ApplicationSet applicationSet = session.ensureApplicationLoaded();
         assertNotNull(applicationSet);
         assertThat(applicationSet.getApplicationGeneration(), is(3L));
@@ -101,7 +101,7 @@ public class RemoteSessionTest {
         okFactory.throwOnLoad = false;
 
         RemoteSession session = createSession(3, Arrays.asList(okFactory, failingFactory));
-        session.loadPrepared();
+        session.prepare();
     }
 
     @Test
@@ -116,7 +116,7 @@ public class RemoteSessionTest {
         okFactory.throwOnLoad = false;
 
         RemoteSession session = createSession(3, Arrays.asList(okFactory, failingFactory), failingFactory.clock());
-        session.loadPrepared();
+        session.prepare();
     }
 
     @Test
@@ -138,7 +138,7 @@ public class RemoteSessionTest {
 
         SessionZooKeeperClient zkc = new MockSessionZKClient(curator, tenantName, 3, application);
         RemoteSession session = createSession(3, zkc, Arrays.asList(okFactory, failingFactory));
-        session.loadPrepared();
+        session.prepare();
 
         // Does not cause an error because model version 3 is skipped
     }
@@ -162,7 +162,7 @@ public class RemoteSessionTest {
 
         SessionZooKeeperClient zkc = new MockSessionZKClient(curator, tenantName, 3, application);
         RemoteSession session = createSession(4, zkc, Arrays.asList(okFactory, failingFactory));
-        session.loadPrepared();
+        session.prepare();
 
         // Does not cause an error because model version 4 is skipped
     }