Bind deferred activation decision to concrete bundle instance, not global config

Ensure that deferred activation flags are propagated during building and cloning.

5 files changed, 38 insertions, 9 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 35fe32f21c9..ca50701b763 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
@@ -75,7 +75,7 @@ public class ClusterStateBundle {
 
         public ClusterStateBundle deriveAndBuild() {
             if ((stateDeriver == null || bucketSpaces == null || bucketSpaces.isEmpty()) && explicitDerivedStates == null) {
-                return ClusterStateBundle.ofBaselineOnly(baselineState);
+                return ClusterStateBundle.ofBaselineOnly(baselineState, deferredActivation);
             }
             Map<String, AnnotatedClusterState> derived;
             if (explicitDerivedStates != null) {
@@ -148,7 +148,7 @@ public class ClusterStateBundle {
         Map<String, AnnotatedClusterState> clonedDerived = derivedBucketSpaceStates.entrySet().stream()
                 .collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue().cloneWithClusterState(
                         mapper.apply(e.getValue().getClusterState().clone()))));
-        return new ClusterStateBundle(clonedBaseline, clonedDerived);
+        return new ClusterStateBundle(clonedBaseline, clonedDerived, deferredActivation);
     }
 
     public ClusterStateBundle clonedWithVersionSet(int version) {
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 a653ace265b..69d1fb64f48 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
@@ -841,7 +841,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd
             final ClusterStateBundle candidateBundle = ClusterStateBundle.builder(candidate)
                     .bucketSpaces(configuredBucketSpaces)
                     .stateDeriver(createBucketSpaceStateDeriver())
-                    .deferredActivation(true)
+                    .deferredActivation(options.enableTwoPhaseClusterStateActivation)
                     .deriveAndBuild();
             stateVersionTracker.updateLatestCandidateStateBundle(candidateBundle);
             invokeCandidateStateListeners(candidateBundle);
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 e4a9f62b054..c4e72202893 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
@@ -180,10 +180,6 @@ public class SystemStateBroadcaster {
         return (node.getClusterStateVersionActivationSent() == clusterStateBundle.getVersion());
     }
 
-    private static boolean twoPhaseTransitionEnabled(FleetController controller) {
-        return controller.getOptions().enableTwoPhaseClusterStateActivation;
-    }
-
     /**
      * Checks if all distributor nodes have ACKed (and activated) the most recent cluster state.
      * Iff this is the case, triggers handleAllDistributorsInSync() on the provided FleetController
@@ -202,7 +198,7 @@ public class SystemStateBroadcaster {
 
         if (!anyDistributorsNeedStateBundle && (currentStateVersion > lastStateVersionBundleAcked)) {
             markCurrentClusterStateBundleAsReceivedByAllDistributors();
-            if (twoPhaseTransitionEnabled(fleetController)) {
+            if (clusterStateBundle.deferredActivation()) {
                 log.log(LogLevel.INFO, () -> String.format("All distributors have ACKed cluster state " + // TODO log level
                         "version %d, sending activation", currentStateVersion));
             } else {
@@ -211,7 +207,7 @@ public class SystemStateBroadcaster {
             return; // Either converged (no two-phase) or activations must be sent before we can continue.
         }
 
-        if (anyDistributorsNeedStateBundle || !twoPhaseTransitionEnabled(fleetController)) {
+        if (anyDistributorsNeedStateBundle || !clusterStateBundle.deferredActivation()) {
             return;
         }
 
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 fa802f7fd71..e10694d553f 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
@@ -4,8 +4,11 @@ package com.yahoo.vespa.clustercontroller.core;
 import com.yahoo.vdslib.state.*;
 import org.junit.Test;
 
+import java.util.function.Function;
+
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -117,4 +120,27 @@ public class ClusterStateBundleTest {
         assertFalse(bundle.deferredActivation());
     }
 
+    @Test
+    public void simple_bundle_without_derived_states_propagates_deferred_activation_flag() {
+        var bundle = ClusterStateBundle
+                .builder(annotatedStateOf("distributor:2 storage:2"))
+                .deferredActivation(false) // defaults to true
+                .deriveAndBuild();
+        assertFalse(bundle.deferredActivation());
+    }
+
+    @Test
+    public void cloning_preserves_false_deferred_activation_flag() {
+        var bundle = createTestBundleBuilder(true).deferredActivation(false).deriveAndBuild();
+        var derived = bundle.cloneWithMapper(Function.identity());
+        assertEquals(bundle, derived);
+    }
+
+    @Test
+    public void cloning_preserves_true_deferred_activation_flag() {
+        var bundle = createTestBundleBuilder(true).deferredActivation(true).deriveAndBuild();
+        var derived = bundle.cloneWithMapper(Function.identity());
+        assertEquals(bundle, derived);
+    }
+
 }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java
index 75b65f5d85f..40175cd7bc9 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/SystemStateBroadcasterTest.java
@@ -133,4 +133,11 @@ public class SystemStateBroadcasterTest {
 
         // TODO
     }
+
+    /*
+    TODO test
+      - activation not sent before distributors have acked
+      - activation not sent if two phase activation is disabled
+     */
+
 }