Add class checking whether a bucket space on a content node might have buckets pending.

5 files changed, 161 insertions, 51 deletions

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
new file mode 100644
index 00000000000..6eb4d17a63d
--- /dev/null
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingChecker.java
@@ -0,0 +1,28 @@
+// 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;
+
+/**
+ * Class checking whether a particular bucket space on a content node might have buckets pending.
+ *
+ * Aggregated stats over the entire content cluster is used to check this.
+ */
+public class AggregatedStatsMergePendingChecker implements MergePendingChecker {
+
+    private final ContentClusterStats clusterStats;
+
+    public AggregatedStatsMergePendingChecker(ContentClusterStats clusterStats) {
+        this.clusterStats = clusterStats;
+    }
+
+    @Override
+    public boolean hasMergesPending(String bucketSpace, int contentNodeIndex) {
+        ContentNodeStats nodeStats = clusterStats.getContentNode(contentNodeIndex);
+        if (nodeStats != null) {
+            ContentNodeStats.BucketSpaceStats bucketSpaceStats = nodeStats.getBucketSpace(bucketSpace);
+            if (bucketSpaceStats != null && bucketSpaceStats.mayHaveBucketsPending()) {
+                return true;
+            }
+        }
+        return false;
+    }
+}
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 d47ce45216e..b579b406890 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
@@ -125,6 +125,10 @@ public class ContentNodeStats {
         }
     }
 
+    public BucketSpaceStats getBucketSpace(String bucketSpace) {
+        return bucketSpaces.get(bucketSpace);
+    }
+
     public Map<String, BucketSpaceStats> getBucketSpaces() {
         return bucketSpaces;
     }
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
new file mode 100644
index 00000000000..fee3fe4a00b
--- /dev/null
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/AggregatedStatsMergePendingCheckerTest.java
@@ -0,0 +1,65 @@
+// 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 org.junit.Test;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class AggregatedStatsMergePendingCheckerTest {
+
+    private static class Fixture {
+
+        private final AggregatedStatsMergePendingChecker checker;
+
+        public Fixture(ContentClusterStatsBuilder builder) {
+            this.checker = new AggregatedStatsMergePendingChecker(builder.build());
+        }
+
+        public static Fixture fromBucketStats(long bucketsPending) {
+            return new Fixture(new ContentClusterStatsBuilder()
+                    .add(1, "default", 5, bucketsPending));
+        }
+
+        public static Fixture fromInvalidBucketStats() {
+            return new Fixture(new ContentClusterStatsBuilder()
+                    .add(1, "default"));
+        }
+
+        public boolean hasMergesPending(String bucketSpace, int contentNodeIndex) {
+            return checker.hasMergesPending(bucketSpace, contentNodeIndex);
+        }
+
+    }
+
+    @Test
+    public void unknown_content_node_has_no_merges_pending() {
+        Fixture f = Fixture.fromBucketStats(1);
+        assertFalse(f.hasMergesPending("default", 2));
+    }
+
+    @Test
+    public void unknown_bucket_space_has_no_merges_pending() {
+        Fixture f = Fixture.fromBucketStats(1);
+        assertFalse(f.hasMergesPending("global", 1));
+    }
+
+    @Test
+    public void valid_bucket_space_stats_can_have_no_merges_pending() {
+        Fixture f = Fixture.fromBucketStats(0);
+        assertFalse(f.hasMergesPending("default", 1));
+    }
+
+    @Test
+    public void valid_bucket_space_stats_can_have_merges_pending() {
+        Fixture f = Fixture.fromBucketStats(1);
+        assertTrue(f.hasMergesPending("default", 1));
+    }
+
+    @Test
+    public void invalid_bucket_space_stats_has_merges_pending() {
+        Fixture f = Fixture.fromInvalidBucketStats();
+        assertTrue(f.hasMergesPending("default", 1));
+    }
+
+}
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java
index e132e4d5750..3ad18a0c64d 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java
@@ -14,46 +14,16 @@ import static org.junit.Assert.assertEquals;
  */
 public class ClusterStatsAggregatorTest {
 
-    private static class StatsBuilder {
-        private final Map<Integer, Map<String, ContentNodeStats.BucketSpaceStats> > stats = new HashMap<>();
-
-        public StatsBuilder add(int nodeIndex, String bucketSpace, long bucketsTotal, long bucketsPending) {
-            return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.of(bucketsTotal, bucketsPending));
-        }
-        public StatsBuilder add(int nodeIndex, String bucketSpace) {
-            return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.invalid());
-        }
-        public StatsBuilder add(int nodeIndex, String bucketSpace, ContentNodeStats.BucketSpaceStats bucketSpaceStats) {
-            Map<String, ContentNodeStats.BucketSpaceStats> contentNodeStats = stats.get(nodeIndex);
-            if (contentNodeStats == null) {
-                contentNodeStats = new HashMap<>();
-                stats.put(nodeIndex, contentNodeStats);
-            }
-            contentNodeStats.put(bucketSpace, bucketSpaceStats);
-            return this;
-        }
-        public StatsBuilder add(int nodeIndex) {
-            stats.put(nodeIndex, new HashMap<>());
-            return this;
-        }
-        public ContentClusterStats build() {
-            Map<Integer, ContentNodeStats> nodeToStatsMap = new HashMap<>();
-            stats.forEach((nodeIndex, bucketSpaces) ->
-                    nodeToStatsMap.put(nodeIndex, new ContentNodeStats(nodeIndex, bucketSpaces)));
-            return new ContentClusterStats(nodeToStatsMap);
-        }
-    }
-
     private static class Fixture {
         private ClusterStatsAggregator aggregator;
         public Fixture(Set<Integer> distributorNodes,
                 Set<Integer> contentNodes) {
             aggregator = new ClusterStatsAggregator(distributorNodes, contentNodes);
         }
-        public void update(int distributorIndex, StatsBuilder clusterStats) {
+        public void update(int distributorIndex, ContentClusterStatsBuilder clusterStats) {
             aggregator.updateForDistributor(distributorIndex, clusterStats.build());
         }
-        public void verify(StatsBuilder expectedStats) {
+        public void verify(ContentClusterStatsBuilder expectedStats) {
             assertEquals(expectedStats.build(), aggregator.getAggregatedStats());
         }
     }
@@ -69,7 +39,7 @@ public class ClusterStatsAggregatorTest {
     @Test
     public void aggregator_handles_updates_to_single_distributor_and_content_node() {
         Fixture f = new Fixture(distributorNodes(1), contentNodes(3));
-        StatsBuilder stats = new StatsBuilder()
+        ContentClusterStatsBuilder stats = new ContentClusterStatsBuilder()
                 .add(3, "default", 10, 1)
                 .add(3, "global", 11, 2);
         f.update(1, stats);
@@ -80,17 +50,17 @@ public class ClusterStatsAggregatorTest {
     public void aggregator_handles_updates_to_multiple_distributors_and_content_nodes() {
         Fixture f = new Fixture(distributorNodes(1, 2), contentNodes(3, 4));
 
-        f.update(1, new StatsBuilder()
+        f.update(1, new ContentClusterStatsBuilder()
                 .add(3, "default", 10, 1)
                 .add(3, "global", 11, 2)
                 .add(4, "default", 12, 3)
                 .add(4, "global", 13, 4));
-        f.update(2, new StatsBuilder()
+        f.update(2, new ContentClusterStatsBuilder()
                 .add(3, "default", 14, 5)
                 .add(3, "global", 15, 6)
                 .add(4, "default", 16, 7)
                 .add(4, "global", 17, 8));
-        f.verify(new StatsBuilder()
+        f.verify(new ContentClusterStatsBuilder()
                 .add(3, "default", 10 + 14, 1 + 5)
                 .add(3, "global", 11 + 15, 2 + 6)
                 .add(4, "default", 12 + 16, 3 + 7)
@@ -101,29 +71,29 @@ public class ClusterStatsAggregatorTest {
     public void aggregator_handles_multiple_updates_from_same_distributor() {
         Fixture f = new Fixture(distributorNodes(1, 2), contentNodes(3));
 
-        f.update(1, new StatsBuilder().add(3, "default"));
-        f.verify(new StatsBuilder().add(3, "default"));
+        f.update(1, new ContentClusterStatsBuilder().add(3, "default"));
+        f.verify(new ContentClusterStatsBuilder().add(3, "default"));
 
-        f.update(2, new StatsBuilder().add(3, "default", 10, 1));
-        f.verify(new StatsBuilder().add(3, "default", 10, 1));
+        f.update(2, new ContentClusterStatsBuilder().add(3, "default", 10, 1));
+        f.verify(new ContentClusterStatsBuilder().add(3, "default", 10, 1));
 
-        f.update(1, new StatsBuilder().add(3, "default", 11, 2));
-        f.verify(new StatsBuilder().add(3, "default", 10 + 11, 1 + 2));
+        f.update(1, new ContentClusterStatsBuilder().add(3, "default", 11, 2));
+        f.verify(new ContentClusterStatsBuilder().add(3, "default", 10 + 11, 1 + 2));
 
-        f.update(2, new StatsBuilder().add(3, "default", 15, 6));
-        f.verify(new StatsBuilder().add(3, "default", 11 + 15, 2 + 6));
+        f.update(2, new ContentClusterStatsBuilder().add(3, "default", 15, 6));
+        f.verify(new ContentClusterStatsBuilder().add(3, "default", 11 + 15, 2 + 6));
 
-        f.update(1, new StatsBuilder().add(3, "default", 16, 7));
-        f.verify(new StatsBuilder().add(3, "default", 15 + 16, 6 + 7));
+        f.update(1, new ContentClusterStatsBuilder().add(3, "default", 16, 7));
+        f.verify(new ContentClusterStatsBuilder().add(3, "default", 15 + 16, 6 + 7));
 
-        f.update(2, new StatsBuilder().add(3, "default", 12, 3));
-        f.verify(new StatsBuilder().add(3, "default", 16 + 12, 7 + 3));
+        f.update(2, new ContentClusterStatsBuilder().add(3, "default", 12, 3));
+        f.verify(new ContentClusterStatsBuilder().add(3, "default", 16 + 12, 7 + 3));
     }
 
     @Test
     public void aggregator_handles_more_content_nodes_that_distributors() {
         Fixture f = new Fixture(distributorNodes(1), contentNodes(3, 4));
-        StatsBuilder stats = new StatsBuilder()
+        ContentClusterStatsBuilder stats = new ContentClusterStatsBuilder()
                 .add(3, "default", 10, 1)
                 .add(4, "default", 11, 2);
         f.update(1, stats);
@@ -134,9 +104,9 @@ public class ClusterStatsAggregatorTest {
     public void aggregator_ignores_updates_to_unknown_distributor() {
         Fixture f = new Fixture(distributorNodes(1), contentNodes(3));
         final int downDistributorIndex = 2;
-        f.update(downDistributorIndex, new StatsBuilder()
+        f.update(downDistributorIndex, new ContentClusterStatsBuilder()
                 .add(3, "default", 7, 3));
-        f.verify(new StatsBuilder().add(3));
+        f.verify(new ContentClusterStatsBuilder().add(3));
     }
 
 }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java
new file mode 100644
index 00000000000..ec0d5903aeb
--- /dev/null
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java
@@ -0,0 +1,43 @@
+// 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 java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Builder used for testing only.
+ */
+public class ContentClusterStatsBuilder {
+
+    private final Map<Integer, Map<String, ContentNodeStats.BucketSpaceStats>> stats = new HashMap<>();
+
+    public ContentClusterStatsBuilder add(int nodeIndex, String bucketSpace, long bucketsTotal, long bucketsPending) {
+        return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.of(bucketsTotal, bucketsPending));
+    }
+
+    public ContentClusterStatsBuilder add(int nodeIndex, String bucketSpace) {
+        return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.invalid());
+    }
+
+    public ContentClusterStatsBuilder add(int nodeIndex, String bucketSpace, ContentNodeStats.BucketSpaceStats bucketSpaceStats) {
+        Map<String, ContentNodeStats.BucketSpaceStats> contentNodeStats = stats.get(nodeIndex);
+        if (contentNodeStats == null) {
+            contentNodeStats = new HashMap<>();
+            stats.put(nodeIndex, contentNodeStats);
+        }
+        contentNodeStats.put(bucketSpace, bucketSpaceStats);
+        return this;
+    }
+
+    public ContentClusterStatsBuilder add(int nodeIndex) {
+        stats.put(nodeIndex, new HashMap<>());
+        return this;
+    }
+
+    public ContentClusterStats build() {
+        Map<Integer, ContentNodeStats> nodeToStatsMap = new HashMap<>();
+        stats.forEach((nodeIndex, bucketSpaces) ->
+                nodeToStatsMap.put(nodeIndex, new ContentNodeStats(nodeIndex, bucketSpaces)));
+        return new ContentClusterStats(nodeToStatsMap);
+    }
+}