glebashnik/add-deferred-reconfiguration-for-remote-session (#35941)

glebashnik · web-flow · commit a4fe0bb12498 · 2026-02-17T09:40:38.000+01:00
This fixies a bug when only local session in Deployment.java marks clusters to defer reconfiguration, while remote sessions retrieved by config servers from zookeeper in SessionRepository don't do it. This results in different applyOnRestart response from different config servers.
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
@@ -10,6 +10,7 @@
 import com.yahoo.config.application.api.DeployLogger;
 import com.yahoo.config.model.api.ConfigDefinitionRepo;
 import com.yahoo.config.model.api.EndpointCertificateSecretStore;
+import com.yahoo.config.model.api.Model;
 import com.yahoo.config.model.api.OnnxModelCost;
 import com.yahoo.config.model.application.provider.DeployData;
 import com.yahoo.config.model.application.provider.FilesApplicationPackage;
@@ -24,10 +25,12 @@
 import com.yahoo.vespa.config.server.ConfigServerDB;
 import com.yahoo.vespa.config.server.TimeoutBudget;
 import com.yahoo.vespa.config.server.application.InheritableApplications;
+import com.yahoo.vespa.config.server.application.Application;
 import com.yahoo.vespa.config.server.application.ApplicationVersions;
 import com.yahoo.vespa.config.server.application.TenantApplications;
 import com.yahoo.vespa.config.server.configchange.ConfigChangeActions;
 import com.yahoo.vespa.config.server.deploy.TenantFileSystemDirs;
+import com.yahoo.text.Text;
 import com.yahoo.vespa.config.server.filedistribution.FileDistributionFactory;
 import com.yahoo.vespa.config.server.http.InvalidApplicationException;
 import com.yahoo.vespa.config.server.http.UnknownVespaVersionException;
@@ -76,6 +79,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.stream.Collectors;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
 import java.util.concurrent.ExecutorService;
@@ -86,6 +90,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import static com.yahoo.vespa.config.server.session.ActivationTriggers.DeferredReconfiguration;
 import static com.yahoo.vespa.config.server.session.Session.Status.ACTIVATE;
 import static com.yahoo.vespa.config.server.session.Session.Status.DEACTIVATE;
 import static com.yahoo.vespa.config.server.session.Session.Status.NEW;
@@ -464,18 +469,51 @@ void activate(long sessionId) {
 
         CompletionWaiter waiter = createSessionZooKeeperClient(sessionId).getActiveWaiter();
         log.log(Level.FINE, () -> session.logPre() + "Activating " + sessionId);
-        tenantApplications.activateApplication(ensureApplicationLoaded(session), sessionId);
+
+        ApplicationVersions applicationVersions = ensureApplicationLoaded(session);
+        applyDeferredReconfigurationOfClusters(session, applicationVersions);
+
+        tenantApplications.activateApplication(applicationVersions, sessionId);
         log.log(Level.FINE, () -> session.logPre() + "Notifying " + waiter);
         notifyCompletion(waiter);
         log.log(Level.INFO, session.logPre() + "Session activated: " + sessionId);
     }
 
+    /**
+     * Marks clusters for deferred reconfiguration in the model, i.e. wait until restart to apply new config.
+     * This is similar to {@link com.yahoo.vespa.config.server.deploy.Deployment::applyDeferredReconfigurationOfClusters}
+     * but for {@link ActivationTriggers} from {@link RemoteSession} in ZooKeeper 
+     * rather than from {@link Session} created locally on the config server.
+     */
+    private void applyDeferredReconfigurationOfClusters(RemoteSession session, ApplicationVersions applicationVersions) {
+        Set<String> clustersWithDeferredReconfiguration = session.getActivationTriggers().deferredReconfigurations().stream()
+                .map(DeferredReconfiguration::clusterId)
+                .collect(Collectors.toSet());
+        
+        if (clustersWithDeferredReconfiguration.isEmpty()) {
+            return;
+        }
+
+        // Get the model and mark clusters for deferred reconfiguration
+        Model model = applicationVersions.get(session.getVespaVersion())
+                .map(Application::getModel)
+                .orElseThrow(() -> new IllegalStateException(
+                        "Cannot apply deferred reconfiguration: no model available for session " + session.getSessionId()));
+        model.markClustersForDeferredReconfiguration(clustersWithDeferredReconfiguration);
+
+        clustersWithDeferredReconfiguration.forEach(clusterName ->
+                log.log(Level.INFO, session.logPre() +
+                        Text.format("Deferring reconfiguration of cluster '%s' until restart is completed", clusterName)));
+    }
+    
     private void loadSessionIfActive(RemoteSession session) {
         for (ApplicationId applicationId : tenantApplications.activeApplications()) {
             Optional<Long> activeSession = tenantApplications.activeSessionOf(applicationId);
             if (activeSession.isPresent() && activeSession.get() == session.getSessionId()) {
                 log.log(Level.FINE, () -> "Found active application for session " + session.getSessionId() + " , loading it");
-                tenantApplications.activateApplication(ensureApplicationLoaded(session), session.getSessionId());
+                ApplicationVersions applicationVersions = ensureApplicationLoaded(session);
+                applyDeferredReconfigurationOfClusters(session, applicationVersions);
+                tenantApplications.activateApplication(applicationVersions, 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/session/SessionRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
@@ -384,4 +384,75 @@ public ModelCreateResult createAndValidateModel(ModelContext modelContext, Valid
         }
     }
 
+    @Test
+    public void activate_applies_deferred_reconfiguration_from_zookeeper() throws Exception {
+        // Tests that SessionRepository.activate() reads ActivationTriggers from ZooKeeper
+        // and applies deferred reconfigurations to the model (non-primary server path)
+        setup();
+        long sessionId = deploy();
+
+        // Write ActivationTriggers to ZooKeeper to simulate what primary server writes
+        var zkClient = new SessionZooKeeperClient(curator, tenantName, sessionId,
+                applicationRepository.configserverConfig());
+        var triggers = new ActivationTriggers(
+                List.of(), // nodeRestarts
+                List.of(), // reindexings
+                List.of(new ActivationTriggers.DeferredReconfiguration("container")) // deferredReconfigurations
+        );
+        zkClient.writeActivationTriggers(triggers);
+
+        // Deactivate the session to clear applicationVersions
+        sessionRepository.deactivateSession(sessionId);
+
+        // Now activate it - this should read ActivationTriggers and apply deferred reconfigurations
+        sessionRepository.activate(sessionId);
+
+        // Verify that the model has deferChangesUntilRestart set
+        var session = sessionRepository.getRemoteSession(sessionId);
+        var appVersions = session.applicationVersions();
+        assertTrue("Session should be activated", appVersions.isPresent());
+
+        var model = (com.yahoo.vespa.model.VespaModel) appVersions.get().applications().get(0).getModel();
+        var cluster = model.getContainerClusters().get("container");
+        assertNotNull("Container cluster should exist", cluster);
+        assertTrue("Cluster should have deferChangesUntilRestart set after activation",
+                cluster.getDeferChangesUntilRestart());
+    }
+
+    @Test
+    public void loadSessionIfActive_applies_deferred_reconfiguration_from_zookeeper() throws Exception {
+        // Tests that loadSessionIfActive() applies deferred reconfigurations
+        // when discovering an already-active session (bootstrap/discovery path)
+        setup();
+        long sessionId = deploy();
+
+        // Write ActivationTriggers to ZooKeeper
+        SessionZooKeeperClient zkClient = new SessionZooKeeperClient(curator, tenantName, sessionId,
+                applicationRepository.configserverConfig());
+        ActivationTriggers triggers = new ActivationTriggers(
+                List.of(), // nodeRestarts
+                List.of(), // reindexings
+                List.of(new ActivationTriggers.DeferredReconfiguration("container")) // deferredReconfigurations
+        );
+        zkClient.writeActivationTriggers(triggers);
+
+        // Deactivate and then re-discover the session
+        sessionRepository.deactivateSession(sessionId);
+
+        // createRemoteSessionAndActivate calls loadSessionIfActive internally
+        RemoteSession session = sessionRepository.createRemoteSessionAndActivate(sessionId);
+
+        // Verify deferred reconfigurations were applied
+        var appVersions = session.applicationVersions();
+        if (appVersions.isPresent()) {
+            var model = (com.yahoo.vespa.model.VespaModel) appVersions.get().applications().get(0).getModel();
+            var cluster = model.getContainerClusters().get("container");
+            assertNotNull("Container cluster should exist", cluster);
+            assertTrue("Cluster should have deferChangesUntilRestart set when loading active session",
+                    cluster.getDeferChangesUntilRestart());
+        }
+        // If no appVersions, the session wasn't marked as active in tenantApplications,
+        // so loadSessionIfActive didn't load it - this is expected behavior
+    }
+
 }
