Merge pull request #14463 from vespa-engine/hmusum/configserver-refactoring-29

Create remote session in addLocalSession() only when it does not exist

2 files changed, 24 insertions, 22 deletions

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 4f577d8f62c..c198f1dd0fc 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
@@ -123,10 +123,11 @@ public class SessionRepository {
     // ---------------- Local sessions ----------------------------------------------------------------
 
     public synchronized void addLocalSession(LocalSession session) {
-        localSessionCache.put(session.getSessionId(), session);
         long sessionId = session.getSessionId();
-        RemoteSession remoteSession = createRemoteSession(sessionId);
-        addSessionStateWatcher(sessionId, remoteSession);
+        localSessionCache.put(sessionId, session);
+        if (remoteSessionCache.get(sessionId) == null) {
+            createRemoteSession(sessionId);
+        }
     }
 
     public LocalSession getLocalSession(long sessionId) {
@@ -347,13 +348,10 @@ public class SessionRepository {
         SessionZooKeeperClient sessionZKClient = createSessionZooKeeperClient(sessionId);
         if (sessionZKClient.readStatus().equals(Session.Status.DELETE)) return;
 
-        log.log(Level.FINE, () -> "Adding remote session to SessionRepository: " + sessionId);
-        RemoteSession remoteSession = createRemoteSession(sessionId);
-        loadSessionIfActive(remoteSession);
-        addRemoteSession(remoteSession);
+        log.log(Level.FINE, () -> "Adding remote session " + sessionId);
+        createRemoteSession(sessionId);
         if (distributeApplicationPackage())
             createLocalSessionUsingDistributedApplicationPackage(sessionId);
-        addSessionStateWatcher(sessionId, remoteSession);
     }
 
     void activate(RemoteSession session) {
@@ -463,9 +461,13 @@ public class SessionRepository {
         return create(applicationDirectory, applicationId, activeSessionId, false, timeoutBudget);
     }
 
-    public RemoteSession createRemoteSession(long sessionId) {
+    public synchronized RemoteSession createRemoteSession(long sessionId) {
         SessionZooKeeperClient sessionZKClient = createSessionZooKeeperClient(sessionId);
-        return new RemoteSession(tenantName, sessionId, componentRegistry, sessionZKClient);
+        RemoteSession session = new RemoteSession(tenantName, sessionId, componentRegistry, sessionZKClient);
+        remoteSessionCache.put(sessionId, session);
+        loadSessionIfActive(session);
+        addSessionStateWatcher(sessionId, session);
+        return session;
     }
 
     private void ensureSessionPathDoesNotExist(long sessionId) {
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 c6c08beea17..d6d08aaac6c 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
@@ -24,18 +24,18 @@ public class SessionStateWatcher {
     private static final Logger log = Logger.getLogger(SessionStateWatcher.class.getName());
 
     private final Curator.FileCache fileCache;
-    private final RemoteSession remoteSession;
+    private final RemoteSession session;
     private final MetricUpdater metrics;
     private final Executor zkWatcherExecutor;
     private final SessionRepository sessionRepository;
 
     SessionStateWatcher(Curator.FileCache fileCache,
-                        RemoteSession remoteSession,
+                        RemoteSession session,
                         MetricUpdater metrics,
                         Executor zkWatcherExecutor,
                         SessionRepository sessionRepository) {
         this.fileCache = fileCache;
-        this.remoteSession = remoteSession;
+        this.session = session;
         this.metrics = metrics;
         this.fileCache.addListener(this::nodeChanged);
         this.fileCache.start();
@@ -44,24 +44,24 @@ public class SessionStateWatcher {
     }
 
     private void sessionStatusChanged(Status newStatus) {
-        long sessionId = remoteSession.getSessionId();
+        long sessionId = session.getSessionId();
         switch (newStatus) {
             case NEW:
             case NONE:
                 break;
             case PREPARE:
                 createLocalSession(sessionId);
-                sessionRepository.prepare(remoteSession);
+                sessionRepository.prepare(session);
                 break;
             case ACTIVATE:
                 createLocalSession(sessionId);
-                sessionRepository.activate(remoteSession);
+                sessionRepository.activate(session);
                 break;
             case DEACTIVATE:
-                sessionRepository.deactivate(remoteSession);
+                sessionRepository.deactivate(session);
                 break;
             case DELETE:
-                sessionRepository.delete(remoteSession);
+                sessionRepository.delete(session);
                 break;
             default:
                 throw new IllegalStateException("Unknown status " + newStatus);
@@ -75,7 +75,7 @@ public class SessionStateWatcher {
     }
 
     public long getSessionId() {
-        return remoteSession.getSessionId();
+        return session.getSessionId();
     }
 
     public void close() {
@@ -93,12 +93,12 @@ public class SessionStateWatcher {
                 ChildData node = fileCache.getCurrentData();
                 if (node != null) {
                     newStatus = Status.parse(Utf8.toString(node.getData()));
-                    log.log(Level.FINE, remoteSession.logPre() + "Session change: Session "
-                                        + remoteSession.getSessionId() + " changed status to " + newStatus.name());
+                    log.log(Level.FINE, session.logPre() + "Session change: Session "
+                                        + session.getSessionId() + " changed status to " + newStatus.name());
                     sessionStatusChanged(newStatus);
                 }
             } catch (Exception e) {
-                log.log(Level.WARNING, remoteSession.logPre() + "Error handling session change to " +
+                log.log(Level.WARNING, session.logPre() + "Error handling session change to " +
                                        newStatus.name() + " for session " + getSessionId(), e);
                 metrics.incSessionChangeErrors();
             }