Merge pull request #5383 from vespa-engine/geirst/fix-cluster-stats-change-tracker-in-clustercontroller

Fix ClusterStatsChangeTracker to handle case where current node is no…

3 files changed, 15 insertions, 6 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTracker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTracker.java
index adfae7e36ea..75e8db40f0e 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTracker.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTracker.java
@@ -53,7 +53,7 @@ public class ClusterStatsChangeTracker {
                 if (prevValue != currValue) {
                     return true;
                 }
-            } else if (currValue) {
+            } else {
                 return true;
             }
         }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTrackerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTrackerTest.java
index c3dcf1e1cfc..4e5de68756d 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTrackerTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsChangeTrackerTest.java
@@ -77,14 +77,14 @@ public class ClusterStatsChangeTrackerTest {
     }
 
     @Test
-    public void stats_have_not_changed_if_all_nodes_in_sync_and_nothing_previous() {
-        Fixture f = Fixture.fromStats(stats().inSync(0).inSync(1));
-        assertFalse(f.statsHaveChanged());
+    public void stats_have_changed_if_in_sync_node_not_found_in_previous_stats() {
+        Fixture f = Fixture.fromStats(stats().inSync(0));
+        assertTrue(f.statsHaveChanged());
     }
 
     @Test
-    public void stats_have_changed_if_one_node_with_buckets_pending_and_nothing_previous() {
-        Fixture f = Fixture.fromStats(stats().inSync(0).bucketsPending(1));
+    public void stats_have_changed_if_buckets_pending_node_not_found_in_previous_stats() {
+        Fixture f = Fixture.fromStats(stats().bucketsPending(0));
         assertTrue(f.statsHaveChanged());
     }
 
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateVersionTrackerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateVersionTrackerTest.java
index c92f1badae8..02017972e59 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateVersionTrackerTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/StateVersionTrackerTest.java
@@ -277,11 +277,20 @@ public class StateVersionTrackerTest {
         when(distributorNode.isDistributor()).thenReturn(true);
         assertFalse(tracker.bucketSpaceMergeCompletionStateHasChanged());
 
+        // Set baseline state
         tracker.updateLatestCandidateStateBundle(ClusterStateBundle
                 .ofBaselineOnly(stateWithoutAnnotations("distributor:1 storage:1")));
         tracker.promoteCandidateToVersionedState(1234);
         assertFalse(tracker.bucketSpaceMergeCompletionStateHasChanged());
 
+        // Current node not in previous stats
+        tracker.handleUpdatedHostInfo(distributorNode, createHostInfo(0));
+        assertTrue(tracker.bucketSpaceMergeCompletionStateHasChanged());
+
+        // Sync aggregated stats
+        tracker.updateLatestCandidateStateBundle(ClusterStateBundle
+                .ofBaselineOnly(stateWithoutAnnotations("distributor:1 storage:1")));
+
         // Give 'global' bucket space no buckets pending, which is the same as previous stats
         tracker.handleUpdatedHostInfo(distributorNode, createHostInfo(0));
         assertFalse(tracker.bucketSpaceMergeCompletionStateHasChanged());