From 2b6998928e122982ba035b10093a0982a8d62191 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Sat, 14 Nov 2020 08:53:11 +0100 Subject: Avoid looking up active session when it is known When activating use the session id of the session we want to activate instead of looking up active session of the application (which requires application data in zookeeper pointing to the active session to have been written before we look it up, which is not guaranteed to have been done) --- .../config/server/application/TenantApplications.java | 4 ++-- .../vespa/config/server/session/SessionRepository.java | 14 +++++--------- .../server/application/TenantApplicationsTest.java | 15 ++++++++------- .../com/yahoo/vespa/config/server/rpc/RpcServerTest.java | 6 ++++-- .../vespa/config/server/tenant/TenantRepositoryTest.java | 16 ++++++++-------- 5 files changed, 27 insertions(+), 28 deletions(-) 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 7c54fd39a74..6e87b65dcb8 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 @@ -238,12 +238,12 @@ public class TenantApplications implements RequestHandler, HostValidator session.logPre() + "Getting session from repo: " + session); - ApplicationSet app = ensureApplicationLoaded(session); - log.log(Level.FINE, () -> session.logPre() + "Reloading config for " + sessionId); - applicationRepo.reloadConfig(app); + log.log(Level.FINE, () -> session.logPre() + "Activating " + sessionId); + applicationRepo.activateApplication(ensureApplicationLoaded(session), sessionId); log.log(Level.FINE, () -> session.logPre() + "Notifying " + waiter); notifyCompletion(waiter, session); log.log(Level.INFO, session.logPre() + "Session activated: " + sessionId); @@ -330,15 +328,13 @@ public class SessionRepository { public void delete(Session remoteSession) { long sessionId = remoteSession.getSessionId(); - // TODO: Change log level to FINE when debugging is finished - log.log(Level.INFO, () -> remoteSession.logPre() + "Deactivating and deleting remote session " + sessionId); + log.log(Level.FINE, () -> remoteSession.logPre() + "Deactivating and deleting remote session " + sessionId); createSetStatusTransaction(remoteSession, Session.Status.DELETE).commit(); deleteRemoteSessionFromZooKeeper(remoteSession); remoteSessionCache.remove(sessionId); LocalSession localSession = getLocalSession(sessionId); if (localSession != null) { - // TODO: Change log level to FINE when debugging is finished - log.log(Level.INFO, () -> localSession.logPre() + "Deleting local session " + sessionId); + log.log(Level.FINE, () -> localSession.logPre() + "Deleting local session " + sessionId); deleteLocalSession(localSession); } } @@ -354,7 +350,7 @@ public class SessionRepository { for (ApplicationId applicationId : applicationRepo.activeApplications()) { if (applicationRepo.requireActiveSessionOf(applicationId) == session.getSessionId()) { log.log(Level.FINE, () -> "Found active application for session " + session.getSessionId() + " , loading it"); - applicationRepo.reloadConfig(ensureApplicationLoaded(session)); + applicationRepo.activateApplication(ensureApplicationLoaded(session), session.getSessionId()); log.log(Level.INFO, session.logPre() + "Application activated successfully: " + applicationId + " (generation " + session.getSessionId() + ")"); return; } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java index a1249838324..0f6aa66b9c3 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java @@ -178,13 +178,14 @@ public class TenantApplicationsTest { ApplicationId applicationId = ApplicationId.defaultId(); applications.createApplication(applicationId); applications.createPutTransaction(applicationId, 1).commit(); - applications.reloadConfig(ApplicationSet.fromSingle(new Application(model, - new ServerCache(), - 1, - false, - vespaVersion, - MetricUpdater.createTestUpdater(), - applicationId))); + applications.activateApplication(ApplicationSet.fromSingle(new Application(model, + new ServerCache(), + 1, + false, + vespaVersion, + MetricUpdater.createTestUpdater(), + applicationId)), + 1); Set> configNames = applications.listConfigs(applicationId, Optional.of(vespaVersion), false); assertTrue(configNames.contains(new ConfigKey<>("sentinel", "hosts", "cloud.config"))); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java index 4ee8cf81e4a..57462c41db8 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcServerTest.java @@ -28,6 +28,7 @@ import com.yahoo.vespa.config.server.application.ApplicationSet; import com.yahoo.vespa.config.server.application.TenantApplications; import com.yahoo.vespa.config.server.monitoring.MetricUpdater; import com.yahoo.vespa.config.server.session.PrepareParams; +import com.yahoo.vespa.config.server.session.RemoteSession; import com.yahoo.vespa.model.VespaModel; import org.junit.Rule; import org.junit.Test; @@ -64,8 +65,9 @@ public class RpcServerTest { ApplicationRepository applicationRepository = tester.applicationRepository(); applicationRepository.deploy(testApp, new PrepareParams.Builder().applicationId(applicationId).build()); TenantApplications applicationRepo = tester.tenant().getApplicationRepo(); - ApplicationSet applicationSet = tester.tenant().getSessionRepository().ensureApplicationLoaded(applicationRepository.getActiveRemoteSession(applicationId)); - applicationRepo.reloadConfig(applicationSet); + RemoteSession activeSession = applicationRepository.getActiveRemoteSession(applicationId); + ApplicationSet applicationSet = tester.tenant().getSessionRepository().ensureApplicationLoaded(activeSession); + applicationRepo.activateApplication(applicationSet, activeSession.getSessionId()); testPrintStatistics(tester); testGetConfig(tester); testEnabled(tester); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java index 8678c42eab4..6104d95b5c2 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/tenant/TenantRepositoryTest.java @@ -89,14 +89,14 @@ public class TenantRepositoryTest { ApplicationId id = ApplicationId.from(tenant1, ApplicationName.defaultName(), InstanceName.defaultName()); applicationRepo.createApplication(id); applicationRepo.createPutTransaction(id, 4).commit(); - applicationRepo.reloadConfig(ApplicationSet.fromSingle( - new Application(new VespaModel(MockApplicationPackage.createEmpty()), - new ServerCache(), - 4L, - false, - new Version(1, 2, 3), - MetricUpdater.createTestUpdater(), - id))); + applicationRepo.activateApplication(ApplicationSet.fromSingle(new Application(new VespaModel(MockApplicationPackage.createEmpty()), + new ServerCache(), + 4L, + false, + new Version(1, 2, 3), + MetricUpdater.createTestUpdater(), + id)), + 4); assertEquals(1, listener.reloaded.get()); } -- cgit v1.2.3