Add config to clustercontroller for the min merge completion ratio of buckets in a bucket space before it is considered complete.

13 files changed, 100 insertions, 38 deletions

diff --git a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java
index 2a9c076a060..98e13f64ee8 100644
--- a/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java
+++ b/clustercontroller-apps/src/main/java/com/yahoo/vespa/clustercontroller/apps/clustercontroller/ClusterControllerClusterConfigurer.java
@@ -74,6 +74,7 @@ public class ClusterControllerClusterConfigurer {
         options.minNodeRatioPerGroup = config.min_node_ratio_per_group();
         options.setMaxDeferredTaskVersionWaitTime(Duration.ofMillis((int)(config.max_deferred_task_version_wait_time_sec() * 1000)));
         options.enableMultipleBucketSpaces = config.enable_multiple_bucket_spaces();
+        options.minMergeCompletionRatio = config.min_merge_completion_ratio();
     }
 
     private void configure(SlobroksConfig config) {
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingChecker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingChecker.java
index 07586569bf5..e6ce1df2b48 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingChecker.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingChecker.java
@@ -1,10 +1,6 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.clustercontroller.core;
 
-import com.yahoo.document.FixedBucketSpaces;
-
-import java.util.Iterator;
-
 /**
  * Class checking whether a particular bucket space on a content node might have buckets pending.
  *
@@ -13,9 +9,12 @@ import java.util.Iterator;
 public class AggregatedStatsMergePendingChecker implements MergePendingChecker {
 
     private final AggregatedClusterStats stats;
+    private final double minMergeCompletionRatio;
 
-    public AggregatedStatsMergePendingChecker(AggregatedClusterStats stats) {
+    public AggregatedStatsMergePendingChecker(AggregatedClusterStats stats,
+                                              double minMergeCompletionRatio) {
         this.stats = stats;
+        this.minMergeCompletionRatio = minMergeCompletionRatio;
     }
 
     @Override
@@ -26,7 +25,8 @@ public class AggregatedStatsMergePendingChecker implements MergePendingChecker {
         ContentNodeStats nodeStats = stats.getStats().getContentNode(contentNodeIndex);
         if (nodeStats != null) {
             ContentNodeStats.BucketSpaceStats bucketSpaceStats = nodeStats.getBucketSpace(bucketSpace);
-            return (bucketSpaceStats != null && bucketSpaceStats.mayHaveBucketsPending());
+            return (bucketSpaceStats != null &&
+                    bucketSpaceStats.mayHaveBucketsPending(minMergeCompletionRatio));
         }
         return true;
     }
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java
index 9db4ce4e44a..2a9ca13b3ee 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregator.java
@@ -73,8 +73,8 @@ public class ClusterStatsAggregator {
         return result;
     }
 
-    MergePendingChecker createMergePendingChecker() {
-        return new AggregatedStatsMergePendingChecker(getAggregatedStats());
+    MergePendingChecker createMergePendingChecker(double minMergeCompletionRatio) {
+        return new AggregatedStatsMergePendingChecker(getAggregatedStats(), minMergeCompletionRatio);
     }
 
     /**
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 75e8db40f0e..68f19c721d9 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
@@ -19,13 +19,15 @@ public class ClusterStatsChangeTracker {
     private AggregatedStatsMergePendingChecker checker;
     private Map<Integer, Boolean> prevMayHaveMergesPending = null;
 
-    public ClusterStatsChangeTracker(AggregatedClusterStats aggregatedStats) {
-        setAggregatedStats(aggregatedStats);
+    public ClusterStatsChangeTracker(AggregatedClusterStats aggregatedStats,
+                                     double minMergeCompletionRatio) {
+        setAggregatedStats(aggregatedStats, minMergeCompletionRatio);
     }
 
-    private void setAggregatedStats(AggregatedClusterStats aggregatedStats) {
+    private void setAggregatedStats(AggregatedClusterStats aggregatedStats,
+                                    double minMergeCompletionRatio) {
         this.aggregatedStats = aggregatedStats;
-        checker = new AggregatedStatsMergePendingChecker(this.aggregatedStats);
+        checker = new AggregatedStatsMergePendingChecker(this.aggregatedStats, minMergeCompletionRatio);
     }
 
     public void syncAggregatedStats() {
@@ -36,9 +38,10 @@ public class ClusterStatsChangeTracker {
         }
     }
 
-    public void updateAggregatedStats(AggregatedClusterStats newAggregatedStats) {
+    public void updateAggregatedStats(AggregatedClusterStats newAggregatedStats,
+                                      double minMergeCompletionRatio) {
         syncAggregatedStats();
-        setAggregatedStats(newAggregatedStats);
+        setAggregatedStats(newAggregatedStats, minMergeCompletionRatio);
     }
 
     public boolean statsHaveChanged() {
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java
index 72a7b2e4bcf..8bbbb20bef4 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java
@@ -54,8 +54,18 @@ public class ContentNodeStats {
             return bucketsPending;
         }
 
-        public boolean mayHaveBucketsPending() {
-            return (bucketsPending > 0) || (invalidCount > 0);
+        public boolean mayHaveBucketsPending(double minMergeCompletionRatio) {
+            if (invalidCount > 0) {
+                return true;
+            }
+            if (bucketsTotal == 0) {
+                return bucketsPending > 0;
+            }
+            return calcMergeCompletionRatio() < minMergeCompletionRatio;
+        }
+
+        private double calcMergeCompletionRatio() {
+            return ((double) Math.max(0, (bucketsTotal - bucketsPending)) / (double) bucketsTotal);
         }
 
         public boolean valid() {
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 2e2534916a7..54c6c11f81e 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
@@ -121,7 +121,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd
         this.stateGatherer = nodeStateGatherer;
         this.stateChangeHandler = stateChangeHandler;
         this.systemStateBroadcaster = systemStateBroadcaster;
-        this.stateVersionTracker = new StateVersionTracker();
+        this.stateVersionTracker = new StateVersionTracker(options.minMergeCompletionRatio);
         this.metricUpdater = metricUpdater;
 
         this.statusPageServer = statusPage;
@@ -452,6 +452,7 @@ public class FleetController implements NodeStateOrHostInfoChangeHandler, NodeAd
         } else {
             configuredBucketSpaces = Collections.emptySet();
         }
+        stateVersionTracker.setMinMergeCompletionRatio(options.minMergeCompletionRatio);
 
         communicator.propagateOptions(options);
 
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java
index 3bf8edafbe3..860d38b3438 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetControllerOptions.java
@@ -122,6 +122,9 @@ public class FleetControllerOptions implements Cloneable {
     // TODO replace this flag with a set of bucket spaces instead
     public boolean enableMultipleBucketSpaces = false;
 
+    // TODO: Choose a default value
+    public double minMergeCompletionRatio = 1.0;
+
     // TODO: Replace usage of this by usage where the nodes are explicitly passed (below)
     public FleetControllerOptions(String clusterName) {
         this.clusterName = clusterName;
diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java
index d4cd8f902c6..0673a10c282 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/StateVersionTracker.java
@@ -36,10 +36,13 @@ public class StateVersionTracker {
 
     private final LinkedList<ClusterStateHistoryEntry> clusterStateHistory = new LinkedList<>();
     private int maxHistoryEntryCount = 50;
+    private double minMergeCompletionRatio;
 
-    StateVersionTracker() {
+    StateVersionTracker(double minMergeCompletionRatio) {
         clusterStateView = ClusterStateView.create(currentUnversionedState.getBaselineClusterState());
-        clusterStatsChangeTracker = new ClusterStatsChangeTracker(clusterStateView.getStatsAggregator().getAggregatedStats());
+        clusterStatsChangeTracker = new ClusterStatsChangeTracker(clusterStateView.getStatsAggregator().getAggregatedStats(),
+                minMergeCompletionRatio);
+        this.minMergeCompletionRatio = minMergeCompletionRatio;
     }
 
     void setVersionRetrievedFromZooKeeper(final int version) {
@@ -58,6 +61,10 @@ public class StateVersionTracker {
         this.maxHistoryEntryCount = maxHistoryEntryCount;
     }
 
+    void setMinMergeCompletionRatio(double minMergeCompletionRatio) {
+        this.minMergeCompletionRatio = minMergeCompletionRatio;
+    }
+
     int getCurrentVersion() {
         return this.currentVersion;
     }
@@ -128,7 +135,8 @@ public class StateVersionTracker {
                 newStateBundle.getBaselineClusterState().getDistributionBitCount());
         // TODO should this take place in updateLatestCandidateStateBundle instead? I.e. does it require a consolidated state?
         clusterStateView = ClusterStateView.create(currentClusterState.getBaselineClusterState());
-        clusterStatsChangeTracker.updateAggregatedStats(clusterStateView.getStatsAggregator().getAggregatedStats());
+        clusterStatsChangeTracker.updateAggregatedStats(clusterStateView.getStatsAggregator().getAggregatedStats(),
+                minMergeCompletionRatio);
     }
 
     private void recordCurrentStateInHistoryAtTime(final long currentTimeMs) {
@@ -149,7 +157,7 @@ public class StateVersionTracker {
     }
 
     MergePendingChecker createMergePendingChecker() {
-        return clusterStateView.getStatsAggregator().createMergePendingChecker();
+        return clusterStateView.getStatsAggregator().createMergePendingChecker(minMergeCompletionRatio);
     }
 
     /*
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingCheckerTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingCheckerTest.java
index 8ae6501d16b..304579cf75a 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingCheckerTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingCheckerTest.java
@@ -15,24 +15,35 @@ public class AggregatedStatsMergePendingCheckerTest {
         private final AggregatedClusterStats mockAggregatedStats = mock(AggregatedClusterStats.class);
         private final AggregatedStatsMergePendingChecker checker;
 
-        public Fixture(ContentClusterStatsBuilder builder, boolean hasUpdatesFromAllDistributors) {
+        public Fixture(ContentClusterStatsBuilder builder,
+                       boolean hasUpdatesFromAllDistributors,
+                       double minMergeCompletionRatio) {
             when(mockAggregatedStats.getStats()).thenReturn(builder.build());
             when(mockAggregatedStats.hasUpdatesFromAllDistributors()).thenReturn(hasUpdatesFromAllDistributors);
-            this.checker = new AggregatedStatsMergePendingChecker(mockAggregatedStats);
+            this.checker = new AggregatedStatsMergePendingChecker(mockAggregatedStats, minMergeCompletionRatio);
         }
 
         public static Fixture fromBucketsPending(long bucketsPending) {
             return new Fixture(new ContentClusterStatsBuilder()
-                    .add(1, "default", 5, bucketsPending), true);
+                    .add(1, "default", 5, bucketsPending),
+                    true, 1.0);
+        }
+
+        public static Fixture fromBucketsPending(long bucketsPending,
+                                                 double minMergeCompletionRatio) {
+            return new Fixture(new ContentClusterStatsBuilder()
+                    .add(1, "default", 5, bucketsPending),
+                    true, minMergeCompletionRatio);
         }
 
         public static Fixture fromInvalidBucketStats() {
             return new Fixture(new ContentClusterStatsBuilder()
-                    .add(1, "default"), true);
+                    .add(1, "default"),
+                    true, 1.0);
         }
 
         public static Fixture fromIncompleteStats() {
-            return new Fixture(new ContentClusterStatsBuilder(), false);
+            return new Fixture(new ContentClusterStatsBuilder(), false, 1.0);
         }
 
         public boolean mayHaveMergesPending(String bucketSpace, int contentNodeIndex) {
@@ -77,4 +88,12 @@ public class AggregatedStatsMergePendingCheckerTest {
         assertTrue(f.mayHaveMergesPending("default", 1));
     }
 
+    @Test
+    public void min_merge_completion_ratio_is_used_when_calculating_may_have_merges_pending() {
+        // Completion ratio is (5-3)/5 = 0.4
+        assertTrue(Fixture.fromBucketsPending(3, 0.6).mayHaveMergesPending("default", 1));
+        // Completion ratio is (5-2)/5 = 0.6
+        assertFalse(Fixture.fromBucketsPending(2, 0.6).mayHaveMergesPending("default", 1));
+    }
+
 }
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 4e5de68756d..6261469beca 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
@@ -37,19 +37,22 @@ public class ClusterStatsChangeTrackerTest {
         private final Set<Integer> contentNodeIndices;
         private ClusterStatsAggregator aggregator;
         private final ClusterStatsChangeTracker tracker;
+        private final double minMergeCompletionRatio;
 
-        private Fixture(Integer... contentNodeIndices) {
-            this.contentNodeIndices = Sets.newHashSet(contentNodeIndices);
+        private Fixture(Set<Integer> contentNodeIndices,
+                        double minMergeCompletionRatio) {
+            this.contentNodeIndices = contentNodeIndices;
+            this.minMergeCompletionRatio = minMergeCompletionRatio;
             aggregator = new ClusterStatsAggregator(Sets.newHashSet(1), this.contentNodeIndices);
-            tracker = new ClusterStatsChangeTracker(aggregator.getAggregatedStats());
+            tracker = new ClusterStatsChangeTracker(aggregator.getAggregatedStats(), minMergeCompletionRatio);
         }
 
         public static Fixture empty() {
-            return new Fixture(0, 1);
+            return new Fixture(Sets.newHashSet(0, 1), 1.0);
         }
 
         public static Fixture fromStats(StatsBuilder builder) {
-            Fixture result = new Fixture(0, 1);
+            Fixture result = new Fixture(Sets.newHashSet(0, 1), 1.0);
             result.updateStats(builder);
             return result;
         }
@@ -57,7 +60,7 @@ public class ClusterStatsChangeTrackerTest {
         public void newAggregatedStats(StatsBuilder builder) {
             aggregator = new ClusterStatsAggregator(Sets.newHashSet(1), contentNodeIndices);
             updateStats(builder);
-            tracker.updateAggregatedStats(aggregator.getAggregatedStats());
+            tracker.updateAggregatedStats(aggregator.getAggregatedStats(), minMergeCompletionRatio);
         }
 
         private void updateStats(StatsBuilder builder) {
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsTest.java
index c142913a061..f1cc9af4488 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsTest.java
@@ -53,17 +53,25 @@ public class ContentNodeStatsTest {
 
     @Test
     public void invalid_bucket_space_stats_may_have_pending_buckets() {
-        assertTrue(BucketSpaceStats.invalid().mayHaveBucketsPending());
+        assertTrue(BucketSpaceStats.invalid().mayHaveBucketsPending(1.0));
     }
 
     @Test
-    public void valid_bucket_space_stats_may_have_pending_buckets() {
-        assertTrue(BucketSpaceStats.of(5, 1).mayHaveBucketsPending());
+    public void bucket_space_stats_without_buckets_total_use_buckets_pending_to_calculate_may_have_buckets_pending() {
+        assertTrue(BucketSpaceStats.of(0, 2).mayHaveBucketsPending(0.6));
+        assertTrue(BucketSpaceStats.of(0, 1).mayHaveBucketsPending(0.6));
+        assertFalse(BucketSpaceStats.of(0, 0).mayHaveBucketsPending(0.6));
     }
 
     @Test
-    public void valid_bucket_space_stats_may_have_no_pending_buckets() {
-        assertFalse(BucketSpaceStats.of(5, 0).mayHaveBucketsPending());
+    public void min_merge_completion_ratio_is_used_to_calculate_bucket_space_stats_may_have_buckets_pending() {
+        assertTrue(BucketSpaceStats.of(5, 6).mayHaveBucketsPending(0.6));
+        assertTrue(BucketSpaceStats.of(5, 5).mayHaveBucketsPending(0.6));
+        assertTrue(BucketSpaceStats.of(5, 4).mayHaveBucketsPending(0.6));
+        assertTrue(BucketSpaceStats.of(5, 3).mayHaveBucketsPending(0.6));
+        assertFalse(BucketSpaceStats.of(5, 2).mayHaveBucketsPending(0.6));
+        assertFalse(BucketSpaceStats.of(5, 1).mayHaveBucketsPending(0.6));
+        assertFalse(BucketSpaceStats.of(5, 0).mayHaveBucketsPending(0.6));
     }
 
 }
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 02017972e59..1cb09fcf2d1 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
@@ -33,7 +33,7 @@ public class StateVersionTrackerTest {
     }
 
     private static StateVersionTracker createWithMockedMetrics() {
-        return new StateVersionTracker();
+        return new StateVersionTracker(1.0);
     }
 
     private static void updateAndPromote(final StateVersionTracker versionTracker,
diff --git a/configdefinitions/src/vespa/fleetcontroller.def b/configdefinitions/src/vespa/fleetcontroller.def
index c9d6a958eda..30620cd2a31 100644
--- a/configdefinitions/src/vespa/fleetcontroller.def
+++ b/configdefinitions/src/vespa/fleetcontroller.def
@@ -157,3 +157,9 @@ max_deferred_task_version_wait_time_sec double default=30.0
 
 ## Switch to enable multiple bucket spaces in cluster controller.
 enable_multiple_bucket_spaces bool default=false
+
+## The minimum merge completion ratio of buckets in a bucket space before it is considered complete.
+##
+## Bucket merges are considered complete when:
+## ((buckets_total - buckets_pending) / buckets_total)) >= min_merge_completion_ratio
+min_merge_completion_ratio double default=1.0