diff options
author | Harald Musum <musum@verizonmedia.com> | 2020-10-02 13:57:16 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-10-02 13:57:16 +0200 |
commit | 183f2cd98d89863091d07c4402ed1e4e3b8a2105 (patch) | |
tree | 64f7959ecc74a5b2b7c6a31f53516fd51d6d5341 | |
parent | 1a980f57d890cc67a3eb81aa5da92c42751a5201 (diff) |
Revert "Revert "Cleanup some config server code ""
8 files changed, 45 insertions, 55 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 902553e9610..5bbe9c967cf 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 @@ -285,13 +285,14 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye bootstrapping.set(false); } - public PrepareResult prepare(Tenant tenant, long sessionId, PrepareParams prepareParams) { + public PrepareResult prepare(long sessionId, PrepareParams prepareParams) { DeployHandlerLogger logger = DeployHandlerLogger.forPrepareParams(prepareParams); - Deployment deployment = prepare(tenant, sessionId, prepareParams, logger); + Deployment deployment = prepare(sessionId, prepareParams, logger); return new PrepareResult(sessionId, deployment.configChangeActions(), logger); } - private Deployment prepare(Tenant tenant, long sessionId, PrepareParams prepareParams, DeployLogger logger) { + private Deployment prepare(long sessionId, PrepareParams prepareParams, DeployHandlerLogger logger) { + Tenant tenant = getTenant(prepareParams.getApplicationId()); validateThatLocalSessionIsNotActive(tenant, sessionId); LocalSession session = getLocalSession(tenant, sessionId); ApplicationId applicationId = prepareParams.getApplicationId(); @@ -325,8 +326,7 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye public PrepareResult deploy(File applicationPackage, PrepareParams prepareParams, DeployHandlerLogger logger) { ApplicationId applicationId = prepareParams.getApplicationId(); long sessionId = createSession(applicationId, prepareParams.getTimeoutBudget(), applicationPackage); - Tenant tenant = getTenant(applicationId); - Deployment deployment = prepare(tenant, sessionId, prepareParams, logger); + Deployment deployment = prepare(sessionId, prepareParams, logger); deployment.activate(); return new PrepareResult(sessionId, deployment.configChangeActions(), logger); @@ -653,11 +653,10 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } public List<Version> getAllVersions(ApplicationId applicationId) { - Optional<ApplicationSet> applicationSet = getCurrentActiveApplicationSet(getTenant(applicationId), applicationId); - if (applicationSet.isEmpty()) - return List.of(); - else - return applicationSet.get().getAllVersions(applicationId); + Optional<ApplicationSet> applicationSet = getActiveApplicationSet(applicationId); + return applicationSet.isEmpty() + ? List.of() + : applicationSet.get().getAllVersions(applicationId); } // ---------------- Convergence ---------------------------------------------------------------- @@ -795,14 +794,9 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } public long createSession(ApplicationId applicationId, TimeoutBudget timeoutBudget, File applicationDirectory) { - Tenant tenant = getTenant(applicationId); - tenant.getApplicationRepo().createApplication(applicationId); - Optional<Long> activeSessionId = tenant.getApplicationRepo().activeSessionOf(applicationId); - LocalSession session = tenant.getSessionRepository().createSession(applicationDirectory, - applicationId, - timeoutBudget, - activeSessionId); - tenant.getSessionRepository().addLocalSession(session); + SessionRepository sessionRepository = getTenant(applicationId).getSessionRepository(); + LocalSession session = sessionRepository.createSession(applicationDirectory, applicationId, timeoutBudget); + sessionRepository.addLocalSession(session); return session.getSessionId(); } @@ -930,17 +924,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye return session; } - public Optional<ApplicationSet> getCurrentActiveApplicationSet(Tenant tenant, ApplicationId appId) { - Optional<ApplicationSet> currentActiveApplicationSet = Optional.empty(); - TenantApplications applicationRepo = tenant.getApplicationRepo(); - try { - long currentActiveSessionId = applicationRepo.requireActiveSessionOf(appId); - RemoteSession currentActiveSession = getRemoteSession(tenant, currentActiveSessionId); - currentActiveApplicationSet = Optional.ofNullable(currentActiveSession.ensureApplicationLoaded()); - } catch (IllegalArgumentException e) { - // Do nothing if we have no currently active session - } - return currentActiveApplicationSet; + public Optional<ApplicationSet> getActiveApplicationSet(ApplicationId appId) { + return getTenant(appId).getSessionRepository().getActiveApplicationSet(appId); } private File decompressApplication(InputStream in, String contentType, File tempDir) { 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 fc7bd70679a..ed5f1b2d056 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 @@ -11,7 +11,6 @@ import com.yahoo.config.provision.Provisioner; import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.config.server.ApplicationRepository.ActionTimer; import com.yahoo.vespa.config.server.TimeoutBudget; -import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.configchange.ConfigChangeActions; import com.yahoo.vespa.config.server.configchange.RestartActions; import com.yahoo.vespa.config.server.http.InternalServerException; @@ -100,9 +99,9 @@ public class Deployment implements com.yahoo.config.provision.Deployment { ApplicationId applicationId = params.getApplicationId(); try (ActionTimer timer = applicationRepository.timerFor(applicationId, "deployment.prepareMillis")) { - Optional<ApplicationSet> activeApplicationSet = applicationRepository.getCurrentActiveApplicationSet(tenant, applicationId); this.configChangeActions = tenant.getSessionRepository().prepareLocalSession( - session, deployLogger, params, activeApplicationSet, tenant.getPath(), clock.instant()); + session, deployLogger, params, tenant.getPath(), clock.instant()); + this.prepared = true; } } 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 258af35be6f..6fa2075807f 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 @@ -34,13 +34,12 @@ public class SessionPrepareHandler extends SessionHandler { this.zookeeperBarrierTimeout = Duration.ofSeconds(configserverConfig.zookeeper().barrierTimeout()); } - @Override + @Override protected HttpResponse handlePUT(HttpRequest request) { - Tenant tenant = getExistingTenant(request); - TenantName tenantName = tenant.getName(); long sessionId = getSessionIdV2(request); + TenantName tenantName = getExistingTenant(request).getName(); PrepareParams prepareParams = PrepareParams.fromHttpRequest(request, tenantName, zookeeperBarrierTimeout); - PrepareResult result = applicationRepository.prepare(tenant, sessionId, prepareParams); + PrepareResult result = applicationRepository.prepare(sessionId, prepareParams); return new SessionPrepareResponse(result, tenantName, request); } 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 7e410503907..9417a798d1f 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 @@ -13,7 +13,6 @@ import com.yahoo.vespa.curator.Curator; import org.apache.zookeeper.KeeperException; import java.time.Clock; -import java.util.Optional; import java.util.Set; import java.util.logging.Level; import java.util.logging.Logger; @@ -58,13 +57,13 @@ public class RemoteSession extends Session { ApplicationPackage applicationPackage = sessionZooKeeperClient.loadApplicationPackage(); // Read hosts allocated on the config server instance which created this - Optional<AllocatedHosts> allocatedHosts = applicationPackage.getAllocatedHosts(); + SettableOptional<AllocatedHosts> allocatedHosts = new SettableOptional<>(applicationPackage.getAllocatedHosts()); return ApplicationSet.fromList(applicationLoader.buildModels(getApplicationId(), sessionZooKeeperClient.readDockerImageRepository(), sessionZooKeeperClient.readVespaVersion(), applicationPackage, - new SettableOptional<>(allocatedHosts), + allocatedHosts, clock.instant())); } 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 5e10b7dcc61..53230eec831 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 @@ -153,21 +153,20 @@ public class SessionRepository { public ConfigChangeActions prepareLocalSession(LocalSession session, DeployLogger logger, PrepareParams params, - Optional<ApplicationSet> currentActiveApplicationSet, Path tenantPath, Instant now) { applicationRepo.createApplication(params.getApplicationId()); // TODO jvenstad: This is wrong, but it has to be done now, since preparation can change the application ID of a session :( logger.log(Level.FINE, "Created application " + params.getApplicationId()); long sessionId = session.getSessionId(); SessionZooKeeperClient sessionZooKeeperClient = createSessionZooKeeperClient(sessionId); - Curator.CompletionWaiter waiter = sessionZooKeeperClient.createPrepareWaiter(); + Optional<ApplicationSet> activeApplicationSet = getActiveApplicationSet(params.getApplicationId()); ConfigChangeActions actions = sessionPreparer.prepare(applicationRepo.getHostValidator(), logger, params, - currentActiveApplicationSet, tenantPath, now, + activeApplicationSet, tenantPath, now, getSessionAppDir(sessionId), session.getApplicationPackage(), sessionZooKeeperClient) .getConfigChangeActions(); setPrepared(session); - waiter.awaitCompletion(params.getTimeoutBudget().timeLeft()); + sessionZooKeeperClient.createPrepareWaiter().awaitCompletion(params.getTimeoutBudget().timeLeft()); return actions; } @@ -436,8 +435,9 @@ public class SessionRepository { * @param timeoutBudget Timeout for creating session and waiting for other servers. * @return a new session */ - public LocalSession createSession(File applicationDirectory, ApplicationId applicationId, - TimeoutBudget timeoutBudget, Optional<Long> activeSessionId) { + public LocalSession createSession(File applicationDirectory, ApplicationId applicationId, TimeoutBudget timeoutBudget) { + applicationRepo.createApplication(applicationId); + Optional<Long> activeSessionId = applicationRepo.activeSessionOf(applicationId); return create(applicationDirectory, applicationId, activeSessionId, false, timeoutBudget); } @@ -560,6 +560,18 @@ public class SessionRepository { return applicationPackage; } + public Optional<ApplicationSet> getActiveApplicationSet(ApplicationId appId) { + Optional<ApplicationSet> currentActiveApplicationSet = Optional.empty(); + try { + long currentActiveSessionId = applicationRepo.requireActiveSessionOf(appId); + RemoteSession currentActiveSession = getRemoteSession(currentActiveSessionId); + currentActiveApplicationSet = Optional.ofNullable(currentActiveSession.ensureApplicationLoaded()); + } catch (IllegalArgumentException e) { + // Do nothing if we have no currently active session + } + return currentActiveApplicationSet; + } + private void copyApp(File sourceDir, File destinationDir) throws IOException { if (destinationDir.exists()) throw new RuntimeException("Destination dir " + destinationDir + " already exists"); 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 b2deaa22b2e..7dc19491654 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 @@ -537,7 +537,7 @@ public class ApplicationRepositoryTest { long firstSession = result.sessionId(); long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, testAppJdiscOnly); - applicationRepository.prepare(applicationRepository.getTenant(applicationId()), sessionId, prepareParams()); + applicationRepository.prepare(sessionId, prepareParams()); exceptionRule.expect(RuntimeException.class); exceptionRule.expectMessage(containsString("Timeout exceeded when trying to activate 'test1.testapp'")); applicationRepository.activate(applicationRepository.getTenant(applicationId()), sessionId, new TimeoutBudget(clock, Duration.ofSeconds(0)), false); @@ -562,7 +562,7 @@ public class ApplicationRepositoryTest { PrepareResult result2 = deployApp(testAppJdiscOnly); result2.sessionId(); - applicationRepository.prepare(applicationRepository.getTenant(applicationId()), sessionId2, prepareParams()); + applicationRepository.prepare(sessionId2, prepareParams()); exceptionRule.expect(ActivationConflictException.class); exceptionRule.expectMessage(containsString("tenant:test1 app:testapp:default Cannot activate session 3 because the currently active session (4) has changed since session 3 was created (was 2 at creation time)")); applicationRepository.activate(applicationRepository.getTenant(applicationId()), sessionId2, timeoutBudget, false); @@ -575,7 +575,7 @@ public class ApplicationRepositoryTest { exceptionRule.expect(IllegalStateException.class); exceptionRule.expectMessage(containsString("Session is active: 2")); - applicationRepository.prepare(applicationRepository.getTenant(applicationId()), sessionId, prepareParams()); + applicationRepository.prepare(sessionId, prepareParams()); exceptionRule.expect(IllegalStateException.class); exceptionRule.expectMessage(containsString("tenant:test1 app:testapp:default Session 2 is already active")); 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 6b9abf5d7ba..ddc880c344e 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 @@ -17,7 +17,6 @@ import com.yahoo.vespa.config.server.http.HandlerTest; import com.yahoo.vespa.config.server.http.HttpErrorResponse; import com.yahoo.vespa.config.server.http.SessionHandlerTest; import com.yahoo.vespa.config.server.session.PrepareParams; -import com.yahoo.vespa.config.server.tenant.Tenant; import com.yahoo.vespa.config.server.tenant.TenantRepository; import org.junit.Before; import org.junit.Rule; @@ -72,8 +71,7 @@ public class HostHandlerTest { public void require_correct_tenant_and_application_for_hostname() throws Exception { ApplicationId applicationId = applicationId(); applicationRepository.deploy(testApp, new PrepareParams.Builder().applicationId(applicationId).build()); - Tenant tenant = applicationRepository.getTenant(applicationId); - String hostname = applicationRepository.getCurrentActiveApplicationSet(tenant, applicationId).get().getAllHosts().iterator().next(); + String hostname = applicationRepository.getActiveApplicationSet(applicationId).get().getAllHosts().iterator().next(); assertApplicationForHost(hostname, applicationId); } 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 511717acfc0..732b815156f 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 @@ -135,14 +135,12 @@ public class SessionActiveHandlerTest { ApplicationMetaData getMetaData() { return metaData; } void invoke() { - Tenant tenant = applicationRepository.getTenant(applicationId()); long sessionId = applicationRepository.createSession(applicationId(), new TimeoutBudget(componentRegistry.getClock(), Duration.ofSeconds(10)), testApp); - applicationRepository.prepare(tenant, - sessionId, - new PrepareParams.Builder().applicationId(applicationId()).build()); + applicationRepository.prepare(sessionId, new PrepareParams.Builder().applicationId(applicationId()).build()); actResponse = handler.handle(createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.ACTIVE, sessionId, subPath)); + Tenant tenant = applicationRepository.getTenant(applicationId()); LocalSession session = applicationRepository.getActiveLocalSession(tenant, applicationId()); metaData = session.getMetaData(); this.sessionId = sessionId; |