Bring default state of ClusterStateBundle deferred activation flag in line with C++ impl

I.e. disabled by default. Also reduce log level for logging
used during development.

4 files changed, 21 insertions, 25 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundle.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundle.java
index 7e9a4ed6fca..fc06fef5b30 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundle.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundle.java
@@ -35,7 +35,7 @@ public class ClusterStateBundle {
         private Map<String, AnnotatedClusterState> explicitDerivedStates;
         private ClusterStateDeriver stateDeriver;
         private Set<String> bucketSpaces;
-        private boolean deferredActivation = true;
+        private boolean deferredActivation = false;
 
         public Builder(AnnotatedClusterState baselineState) {
             this.baselineState = baselineState;
@@ -91,7 +91,7 @@ public class ClusterStateBundle {
     }
 
     private ClusterStateBundle(AnnotatedClusterState baselineState, Map<String, AnnotatedClusterState> derivedBucketSpaceStates) {
-        this(baselineState, derivedBucketSpaceStates, true);
+        this(baselineState, derivedBucketSpaceStates, false);
     }
 
     private ClusterStateBundle(AnnotatedClusterState baselineState, Map<String,
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java
index 69d1fb64f48..791315bb3d1 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java
@@ -440,10 +440,11 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd
     /** Called when all distributors have acked newest cluster state version. */
     public void handleAllDistributorsInSync(DatabaseHandler database, DatabaseHandler.Context context) throws InterruptedException {
         Set<ConfiguredNode> nodes = new HashSet<>(cluster.clusterInfo().getConfiguredNodes().values());
-        ClusterState currentState = stateVersionTracker.getVersionedClusterState();
-        log.fine(() -> String.format("All distributors have ACKed cluster state version %d", currentState.getVersion()));
-        stateChangeHandler.handleAllDistributorsInSync(currentState, nodes, database, context);
-        convergedStates.add(stateVersionTracker.getVersionedClusterStateBundle()); // FIXME ugh, going via version tracker?
+        // TODO wouldn't it be better to always get bundle information from the state broadcaster?
+        var currentBundle = stateVersionTracker.getVersionedClusterStateBundle();
+        log.fine(() -> String.format("All distributors have ACKed cluster state version %d", currentBundle.getVersion()));
+        stateChangeHandler.handleAllDistributorsInSync(currentBundle.getBaselineClusterState(), nodes, database, context);
+        convergedStates.add(currentBundle);
     }
 
     private boolean changesConfiguredNodeSet(Collection<ConfiguredNode> newNodes) {
@@ -672,7 +673,6 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd
                 // Reset timer to only see warning once.
                 firstAllowedStateBroadcast = currentTime;
             }
-            // FIXME bad interaction with activation..!
             sentAny = systemStateBroadcaster.broadcastNewStateBundleIfRequired(databaseContext, communicator);
             if (sentAny) {
                 // FIXME won't this inhibit resending to unresponsive nodes?
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java
index c4e72202893..8f306036301 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcaster.java
@@ -91,12 +91,12 @@ public class SystemStateBroadcaster {
                 // NO_SUCH_METHOD implies node is on a version that does not understand explicit activations
                 // and it has already merrily started using the state version. Treat as if it had been ACKed.
                 if (req.getReply().getReturnCode() != ErrorCode.NO_SUCH_METHOD) {
-                    log.log(LogLevel.INFO, () -> String.format("Activation NACK for node %s with version %d, message %s",
-                            info, version, req.getReply().getReturnMessage())); // TODO log level
+                    log.log(LogLevel.DEBUG, () -> String.format("Activation NACK for node %s with version %d, message %s",
+                            info, version, req.getReply().getReturnMessage()));
                     success = false;
                 } else {
-                    log.log(LogLevel.INFO, () -> String.format("Node %s did not understand state activation RPC; " +
-                            "implicitly treating state %d as activated on node", info, version)); // TODO log level
+                    log.log(LogLevel.DEBUG, () -> String.format("Node %s did not understand state activation RPC; " +
+                            "implicitly treating state %d as activated on node", info, version));
                 }
             }
             info.setSystemStateVersionActivationAcked(version, success);
@@ -199,7 +199,7 @@ public class SystemStateBroadcaster {
         if (!anyDistributorsNeedStateBundle && (currentStateVersion > lastStateVersionBundleAcked)) {
             markCurrentClusterStateBundleAsReceivedByAllDistributors();
             if (clusterStateBundle.deferredActivation()) {
-                log.log(LogLevel.INFO, () -> String.format("All distributors have ACKed cluster state " + // TODO log level
+                log.log(LogLevel.DEBUG, () -> String.format("All distributors have ACKed cluster state " +
                         "version %d, sending activation", currentStateVersion));
             } else {
                 markCurrentClusterStateAsConverged(database, dbContext, fleetController);
@@ -218,7 +218,7 @@ public class SystemStateBroadcaster {
         if (!anyDistributorsNeedActivation && (currentStateVersion > lastClusterStateVersionConverged)) {
             markCurrentClusterStateAsConverged(database, dbContext, fleetController);
         } else {
-            log.log(LogLevel.INFO, () -> String.format("distributors still need activation in state %d (last converged: %d)", // TODO log level
+            log.log(LogLevel.DEBUG, () -> String.format("distributors still need activation in state %d (last converged: %d)",
                     currentStateVersion, lastClusterStateVersionConverged));
         }
     }
@@ -228,8 +228,7 @@ public class SystemStateBroadcaster {
     }
 
     private void markCurrentClusterStateAsConverged(DatabaseHandler database, DatabaseHandler.Context dbContext, FleetController fleetController) throws InterruptedException {
-        // TODO log level
-        log.log(LogLevel.INFO, "All distributors have newest clusterstate, updating start timestamps in zookeeper and clearing them from cluster state");
+        log.log(LogLevel.DEBUG, "All distributors have newest clusterstate, updating start timestamps in zookeeper and clearing them from cluster state");
         lastClusterStateVersionConverged = clusterStateBundle.getVersion();
         lastClusterStateBundleConverged = clusterStateBundle;
         fleetController.handleAllDistributorsInSync(database, dbContext);
@@ -256,13 +255,11 @@ public class SystemStateBroadcaster {
             if (nodeNeedsToObserveStartupTimestamps(node)) {
                 // TODO this is the same for all nodes, compute only once
                 ClusterStateBundle modifiedBundle = clusterStateBundle.cloneWithMapper(state -> buildModifiedClusterState(state, dbContext));
-                // TODO log level
-                log.log(LogLevel.INFO, () -> String.format("Sending modified cluster state version %d" +
+                log.log(LogLevel.DEBUG, () -> String.format("Sending modified cluster state version %d" +
                         " to node %s: %s", baselineState.getVersion(), node, modifiedBundle));
                 communicator.setSystemState(modifiedBundle, node, setClusterStateWaiter);
             } else {
-                // TODO log level
-                log.log(LogLevel.INFO, () -> String.format("Sending system state version %d to node %s. " +
+                log.log(LogLevel.DEBUG, () -> String.format("Sending system state version %d to node %s. " +
                         "(went down time %d, node start time %d)", baselineState.getVersion(), node,
                         node.getWentDownWithStartTime(), node.getStartTimestamp()));
                 communicator.setSystemState(clusterStateBundle, node, setClusterStateWaiter);
@@ -284,8 +281,7 @@ public class SystemStateBroadcaster {
 
         var recipients = resolveStateActivationSendSet(dbContext);
         for (NodeInfo node : recipients) {
-            // TODO log level
-            log.log(LogLevel.INFO, () -> String.format("Sending cluster state activation to node %s for version %d",
+            log.log(LogLevel.DEBUG, () -> String.format("Sending cluster state activation to node %s for version %d",
                     node, clusterStateBundle.getVersion()));
             communicator.activateClusterStateVersion(clusterStateBundle.getVersion(), node, activateClusterStateVersionWaiter);
         }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundleTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundleTest.java
index db9c8b4dd73..127211fc911 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundleTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStateBundleTest.java
@@ -90,7 +90,7 @@ public class ClusterStateBundleTest {
     public void toString_without_bucket_space_states_prints_only_baseline_state() {
         ClusterStateBundle bundle = ClusterStateBundle.ofBaselineOnly(
                 annotatedStateOf("distributor:2 storage:2"));
-        assertThat(bundle.toString(), equalTo("ClusterStateBundle('distributor:2 storage:2' (deferred activation))"));
+        assertThat(bundle.toString(), equalTo("ClusterStateBundle('distributor:2 storage:2')"));
     }
 
     @Test
@@ -99,7 +99,7 @@ public class ClusterStateBundleTest {
         assertThat(bundle.toString(), equalTo("ClusterStateBundle('distributor:2 storage:2', " +
                 "default 'distributor:2 storage:2 .0.s:d', " +
                 "global 'distributor:2 storage:2', " +
-                "narnia 'distributor:2 .0.s:d storage:2' (deferred activation))"));
+                "narnia 'distributor:2 .0.s:d storage:2')"));
     }
 
     @Test
@@ -133,9 +133,9 @@ public class ClusterStateBundleTest {
     }
 
     @Test
-    public void deferred_activation_is_enabled_by_default() {
+    public void deferred_activation_is_disabled_by_default() {
         ClusterStateBundle bundle = createTestBundle();
-        assertTrue(bundle.deferredActivation());
+        assertFalse(bundle.deferredActivation());
     }
 
     @Test