diff options
author | Harald Musum <musum@verizonmedia.com> | 2020-11-14 08:53:11 +0100 |
---|---|---|
committer | Harald Musum <musum@verizonmedia.com> | 2020-11-14 08:53:11 +0100 |
commit | 2b6998928e122982ba035b10093a0982a8d62191 (patch) | |
tree | b5d882165e137ef1f9a247fbbc20b8662d9dc2d1 | |
parent | e9f719b1962f4105c10ebb66a4b8c336db02ac06 (diff) |
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)
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<Applica * * @param applicationSet the {@link ApplicationSet} to be reloaded */ - public void reloadConfig(ApplicationSet applicationSet) { + public void activateApplication(ApplicationSet applicationSet, long activeSessionId) { ApplicationId id = applicationSet.getId(); try (Lock lock = lock(id)) { if ( ! exists(id)) return; // Application was deleted before activation. - if (applicationSet.getApplicationGeneration() != requireActiveSessionOf(id)) + if (applicationSet.getApplicationGeneration() != activeSessionId) return; // Application activated a new session before we got here. setLiveApp(applicationSet); 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 40cf4f31cf8..8679d7d678a 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 @@ -319,10 +319,8 @@ public class SessionRepository { void activate(RemoteSession session) { long sessionId = session.getSessionId(); Curator.CompletionWaiter waiter = createSessionZooKeeperClient(sessionId).getActiveWaiter(); - log.log(Level.FINE, () -> 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<ConfigKey<?>> 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()); } |