Merge pull request #13759 from vespa-engine/hmusum/configserver-refactoring-19

Config server refactoring, part 19

10 files changed, 35 insertions, 40 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 0557fa6e552..e57fc104dd3 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
@@ -307,7 +307,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         if (activeSession == null) return Optional.empty();
         TimeoutBudget timeoutBudget = new TimeoutBudget(clock, timeout);
         LocalSession newSession = tenant.getSessionRepository().createSessionFromExisting(activeSession, logger, true, timeoutBudget);
-        tenant.getSessionRepository().addSession(newSession);
+        tenant.getSessionRepository().addLocalSession(newSession);
 
         return Optional.of(Deployment.unprepared(newSession, this, hostProvisioner, tenant, timeout, clock,
                                                  false /* don't validate as this is already deployed */, bootstrap));
@@ -646,7 +646,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         SessionRepository sessionRepository = tenant.getSessionRepository();
         RemoteSession fromSession = getExistingSession(tenant, applicationId);
         LocalSession session = sessionRepository.createSessionFromExisting(fromSession, logger, internalRedeploy, timeoutBudget);
-        sessionRepository.addSession(session);
+        sessionRepository.addLocalSession(session);
         return session.getSessionId();
     }
 
@@ -669,7 +669,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
                                                                            applicationId,
                                                                            timeoutBudget,
                                                                            activeSessionId);
-        tenant.getSessionRepository().addSession(session);
+        tenant.getSessionRepository().addLocalSession(session);
         return session.getSessionId();
     }
 
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java
index 2e101762fc4..fa1d90e0fb1 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/PrepareParams.java
@@ -22,7 +22,7 @@ import java.util.List;
 import java.util.Optional;
 
 /**
- * Parameters for prepare. Immutable.
+ * Parameters for preparing an application. Immutable.
  *
  * @author Ulf Lilleengen
  */
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java
index 1e832548342..3643b237d7e 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/Session.java
@@ -17,9 +17,10 @@ import java.util.Optional;
 /**
  * A session represents an instance of an application that can be edited, prepared and activated. This
  * class represents the common stuff between sessions working on the local file
- * system ({@link LocalSession}s) and sessions working on zookeeper {@link RemoteSession}s.
+ * system ({@link LocalSession}s) and sessions working on zookeeper ({@link RemoteSession}s).
  *
  * @author Ulf Lilleengen
+ * @author hmusum
  */
 public abstract class Session implements Comparable<Session>  {
 
@@ -32,10 +33,7 @@ public abstract class Session implements Comparable<Session>  {
         this.sessionId = sessionId;
         this.sessionZooKeeperClient = sessionZooKeeperClient;
     }
-    /**
-     * Retrieve the session id for this session.
-     * @return the session id.
-     */
+
     public final long getSessionId() {
         return sessionId;
     }
@@ -50,7 +48,7 @@ public abstract class Session implements Comparable<Session>  {
     }
 
     /**
-     * Represents the status of this session.
+     * The status of this session.
      */
     public enum Status {
         NEW, PREPARE, ACTIVATE, DEACTIVATE, DELETE, NONE;
@@ -72,11 +70,9 @@ public abstract class Session implements Comparable<Session>  {
      * @return log preamble
      */
     public String logPre() {
-        if (getApplicationId().equals(ApplicationId.defaultId())) {
-            return TenantRepository.logPre(getTenantName());
-        } else {
-            return TenantRepository.logPre(getApplicationId());
-        }
+        return getApplicationId().equals(ApplicationId.defaultId())
+                ? TenantRepository.logPre(getTenantName())
+                : TenantRepository.logPre(getApplicationId());
     }
 
     public Instant getCreateTime() {
@@ -89,7 +85,7 @@ public abstract class Session implements Comparable<Session>  {
 
     void setApplicationPackageReference(FileReference applicationPackageReference) {
         if (applicationPackageReference == null) throw new IllegalArgumentException(String.format(
-                "Null application package FileReference for tenant: %s, session: %d", tenant, sessionId));
+                "Null application package file reference for tenant %s, session id %d", tenant, sessionId));
         sessionZooKeeperClient.writeApplicationPackageReference(applicationPackageReference);
     }
 
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 2da93b7f243..b039aeda318 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
@@ -119,12 +119,12 @@ public class SessionRepository {
 
     // ---------------- Local sessions ----------------------------------------------------------------
 
-    public synchronized void addSession(LocalSession session) {
+    public synchronized void addLocalSession(LocalSession session) {
         localSessionCache.addSession(session);
         long sessionId = session.getSessionId();
         Curator.FileCache fileCache = curator.createFileCache(getSessionStatePath(sessionId).getAbsolute(), false);
-        RemoteSession remoteSession = new RemoteSession(tenantName, sessionId, componentRegistry, createSessionZooKeeperClient(sessionId));
-        addWatcher(sessionId, fileCache, remoteSession, Optional.of(session));
+        RemoteSession remoteSession = createRemoteSession(sessionId);
+        addSesssionStateWatcher(sessionId, fileCache, remoteSession, Optional.of(session));
     }
 
     public LocalSession getLocalSession(long sessionId) {
@@ -137,12 +137,11 @@ public class SessionRepository {
 
     private void loadLocalSessions() {
         File[] sessions = tenantFileSystemDirs.sessionsPath().listFiles(sessionApplicationsFilter);
-        if (sessions == null) {
-            return;
-        }
+        if (sessions == null) return;
+
         for (File session : sessions) {
             try {
-                addSession(createSessionFromId(Long.parseLong(session.getName())));
+                addLocalSession(createSessionFromId(Long.parseLong(session.getName())));
             } catch (IllegalArgumentException e) {
                 log.log(Level.WARNING, "Could not load session '" +
                         session.getAbsolutePath() + "':" + e.getMessage() + ", skipping it.");
@@ -327,7 +326,7 @@ public class SessionRepository {
         Optional<LocalSession> localSession = Optional.empty();
         if (distributeApplicationPackage())
             localSession = createLocalSessionUsingDistributedApplicationPackage(sessionId);
-        addWatcher(sessionId, fileCache, remoteSession, localSession);
+        addSesssionStateWatcher(sessionId, fileCache, remoteSession, localSession);
     }
 
     private boolean distributeApplicationPackage() {
@@ -553,7 +552,6 @@ public class SessionRepository {
             return Optional.of(createSessionFromId(sessionId));
         }
 
-        log.log(Level.INFO, "Creating local session for session id " + sessionId);
         SessionZooKeeperClient sessionZKClient = createSessionZooKeeperClient(sessionId);
         FileReference fileReference = sessionZKClient.readApplicationPackageReference();
         log.log(Level.FINE, "File reference for session id " + sessionId + ": " + fileReference);
@@ -570,8 +568,9 @@ public class SessionRepository {
                 return Optional.empty();
             }
             ApplicationId applicationId = sessionZKClient.readApplicationId();
+            log.log(Level.INFO, "Creating local session for session id " + sessionId);
             LocalSession localSession = createLocalSession(sessionDir, applicationId, sessionId);
-            addSession(localSession);
+            addLocalSession(localSession);
             return Optional.of(localSession);
         }
         return Optional.empty();
@@ -615,7 +614,7 @@ public class SessionRepository {
         return new TenantFileSystemDirs(componentRegistry.getConfigServerDB(), tenantName).getUserApplicationDir(sessionId);
     }
 
-    private void addWatcher(long sessionId, Curator.FileCache fileCache, RemoteSession remoteSession, Optional<LocalSession> localSession) {
+    private void addSesssionStateWatcher(long sessionId, Curator.FileCache fileCache, 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));
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
index 654d811a31f..23323d11f76 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ConfigServerBootstrapTest.java
@@ -199,8 +199,8 @@ public class ConfigServerBootstrapTest {
         throw new RuntimeException(messageIfWaitingFails);
     }
 
-    private MockRpc createRpcServer(ConfigserverConfig configserverConfig) throws IOException {
-        return new MockRpc(configserverConfig.rpcport(), temporaryFolder.newFolder());
+    private MockRpcServer createRpcServer(ConfigserverConfig configserverConfig) throws IOException {
+        return new MockRpcServer(configserverConfig.rpcport(), temporaryFolder.newFolder());
     }
 
     private StateMonitor createStateMonitor() {
@@ -241,11 +241,11 @@ public class ConfigServerBootstrapTest {
                              stateMonitor);
     }
 
-    public static class MockRpc extends com.yahoo.vespa.config.server.rpc.MockRpc {
+    public static class MockRpcServer extends com.yahoo.vespa.config.server.rpc.MockRpcServer {
 
         volatile boolean isRunning = false;
 
-        MockRpc(int port, File tempDir) {
+        MockRpcServer(int port, File tempDir) {
             super(port, tempDir);
         }
 
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 4cf81d22e3c..97085416073 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
@@ -54,13 +54,13 @@ public class ApplicationContentHandlerTest extends ContentHandlerTestBase {
 
         session2 = new MockLocalSession(2, FilesApplicationPackage.fromFile(new File("src/test/apps/content")));
         Tenant tenant1 = tenantRepository.getTenant(tenantName1);
-        tenant1.getSessionRepository().addSession(session2);
+        tenant1.getSessionRepository().addLocalSession(session2);
         tenant1.getApplicationRepo().createApplication(idTenant1);
         tenant1.getApplicationRepo().createPutTransaction(idTenant1, 2).commit();
 
         MockLocalSession session3 = new MockLocalSession(3, FilesApplicationPackage.fromFile(new File("src/test/apps/content2")));
         Tenant tenant2 = tenantRepository.getTenant(tenantName2);
-        tenant2.getSessionRepository().addSession(session3);
+        tenant2.getSessionRepository().addLocalSession(session3);
         tenant2.getApplicationRepo().createApplication(idTenant2);
         tenant2.getApplicationRepo().createPutTransaction(idTenant2, 3).commit();
 
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 d8724cb4db2..20e4ef56166 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
@@ -52,7 +52,7 @@ public class HostHandlerTest {
         tenant.getApplicationRepo().createApplication(applicationId);
         tenant.getApplicationRepo().createPutTransaction(applicationId, sessionId).commit();
         ApplicationPackage app = FilesApplicationPackage.fromFile(testApp);
-        tenant.getSessionRepository().addSession(new SessionHandlerTest.MockLocalSession(sessionId, app, applicationId));
+        tenant.getSessionRepository().addLocalSession(new SessionHandlerTest.MockLocalSession(sessionId, app, applicationId));
         TestComponentRegistry componentRegistry = new TestComponentRegistry.Builder()
                 .modelFactoryRegistry(new ModelFactoryRegistry(Collections.singletonList(new VespaModelFactory(new NullConfigModelRegistry()))))
                 .build();
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java
index f639843ac08..20d9be080e9 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionContentHandlerTest.java
@@ -43,7 +43,7 @@ public class SessionContentHandlerTest extends ContentHandlerTestBase {
     public void setupHandler() throws Exception {
         tenantRepository = new TenantRepository(componentRegistry, false);
         tenantRepository.addTenant(tenant);
-        tenantRepository.getTenant(tenant).getSessionRepository().addSession(new MockLocalSession(1L, FilesApplicationPackage.fromFile(createTestApp())));
+        tenantRepository.getTenant(tenant).getSessionRepository().addLocalSession(new MockLocalSession(1L, FilesApplicationPackage.fromFile(createTestApp())));
         handler = createHandler();
         pathPrefix = "/application/v2/tenant/" + tenant + "/session/";
         baseUrl = "http://foo:1337/application/v2/tenant/" + tenant + "/session/1/content/";
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java
index 87459228d0d..8c62fe77557 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/DelayedConfigResponseTest.java
@@ -39,7 +39,7 @@ public class DelayedConfigResponseTest {
     @Test
     public void testDelayedConfigResponses() throws IOException {
 
-        MockRpc rpc = new MockRpc(13337, temporaryFolder.newFolder());
+        MockRpcServer rpc = new MockRpcServer(13337, temporaryFolder.newFolder());
         DelayedConfigResponses responses = new DelayedConfigResponses(rpc, 1, false);
         assertThat(responses.size(), is(0));
         JRTServerConfigRequest req = createRequest("foo", "md5", "myid", "mymd5", 3, 1000000, "bar");
@@ -60,7 +60,7 @@ public class DelayedConfigResponseTest {
     @Test
     public void testDelayResponseRemove() throws IOException {
         GetConfigContext context = GetConfigContext.testContext(ApplicationId.defaultId());
-        MockRpc rpc = new MockRpc(13337, temporaryFolder.newFolder());
+        MockRpcServer rpc = new MockRpcServer(13337, temporaryFolder.newFolder());
         DelayedConfigResponses responses = new DelayedConfigResponses(rpc, 1, false);
         responses.delayResponse(createRequest("foolio", "md5", "myid", "mymd5", 3, 100000, "bar"), context);
         assertThat(responses.size(), is(1));
@@ -70,7 +70,7 @@ public class DelayedConfigResponseTest {
 
     @Test
     public void testDelayedConfigResponse() throws IOException {
-        MockRpc rpc = new MockRpc(13337, temporaryFolder.newFolder());
+        MockRpcServer rpc = new MockRpcServer(13337, temporaryFolder.newFolder());
         DelayedConfigResponses responses = new DelayedConfigResponses(rpc, 1, false);
         assertThat(responses.size(), is(0));
         assertThat(responses.toString(), is("DelayedConfigResponses. Average Size=0"));
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java
index 9f514d9996f..96518d535bf 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpc.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/MockRpcServer.java
@@ -23,7 +23,7 @@ import java.util.concurrent.CompletionService;
  *
  * @author Ulf Lilleengen
  */
-public class MockRpc extends RpcServer {
+public class MockRpcServer extends RpcServer {
 
     public boolean forced = false;
     public RuntimeException exception = null;
@@ -37,7 +37,7 @@ public class MockRpc extends RpcServer {
     public volatile JRTServerConfigRequest latestRequest = null;
 
 
-    public MockRpc(int port, File tempDir) {
+    public MockRpcServer(int port, File tempDir) {
         super(createConfig(port),
               null,
               Metrics.createTestMetrics(),