Throw if we cannot find a sub-group with available distributors

This matches the existing behavior of the C++ Distribution implementation
and avoids triggering an NPE if a distributor is attempted resolved in a
hierarchic cluster when the cluster state does not have any available
distributors (such as when it is down).

2 files changed, 46 insertions, 5 deletions

diff --git a/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java b/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java
index 455a58ed140..1c387fc2a93 100644
--- a/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java
+++ b/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java
@@ -173,7 +173,9 @@ public class Distribution {
         return true;
     }
     private Group getIdealDistributorGroup(BucketId bucket, ClusterState clusterState, Group parent, int redundancy) {
-        if (parent.isLeafGroup()) return parent;
+        if (parent.isLeafGroup()) {
+            return parent;
+        }
         int[] redundancyArray = parent.getDistribution().getRedundancyArray(redundancy);
         TreeSet<ScoredGroup> results = new TreeSet<>();
         int seed = getGroupSeed(bucket, clusterState, parent);
@@ -192,7 +194,9 @@ public class Distribution {
                 results.remove(results.first());
             }
         }
-        if (results.isEmpty()) return null;
+        if (results.isEmpty()) {
+            return null;
+        }
         return getIdealDistributorGroup(bucket, clusterState, results.first().group, redundancyArray[0]);
     }
     private class ResultGroup implements Comparable<ResultGroup> {
@@ -411,6 +415,9 @@ public class Distribution {
         }
 
         Group idealGroup = getIdealDistributorGroup(bucket, state, nodeGraph, redundancy);
+        if (idealGroup == null) {
+            throw new NoDistributorsAvailableException("No distributors available in cluster state version " + state.getVersion());
+        }
         int seed = getDistributorSeed(bucket, state);
         RandomGen random = new RandomGen(seed);
         int randomIndex = 0;
diff --git a/vdslib/src/test/java/com/yahoo/vdslib/distribution/DistributionTestCase.java b/vdslib/src/test/java/com/yahoo/vdslib/distribution/DistributionTestCase.java
index 6f4e4f4410e..8c877704169 100644
--- a/vdslib/src/test/java/com/yahoo/vdslib/distribution/DistributionTestCase.java
+++ b/vdslib/src/test/java/com/yahoo/vdslib/distribution/DistributionTestCase.java
@@ -5,6 +5,9 @@ import com.yahoo.vespa.config.content.StorDistributionConfig;
 import com.yahoo.vdslib.state.ClusterState;
 import com.yahoo.document.BucketId;
 import com.yahoo.vdslib.state.NodeType;
+import org.junit.After;
+import org.junit.Test;
+
 import java.io.BufferedWriter;
 import java.io.IOException;
 import java.io.StringWriter;
@@ -23,7 +26,10 @@ import java.util.Random;
 import java.util.Set;
 import java.util.Stack;
 
-public class DistributionTestCase extends junit.framework.TestCase {
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class DistributionTestCase {
     private DistributionTestFactory test;
     /** Build a set of buckets to test that should represent the entire bucket space well. */
     private static List<BucketId> getTestBuckets() { return getTestBuckets(16); }
@@ -48,13 +54,16 @@ public class DistributionTestCase extends junit.framework.TestCase {
         }
         return Collections.unmodifiableList(buckets);
     }
-    @Override
+
+    @After
     public void tearDown() throws Exception {
         if (test != null) {
             System.err.println("Verified " + test.getVerifiedTests() + " test results for test " + test.getName());
             test.recordTestResults();
         }
     }
+
+    @Test
     public void testSimple() {
         test = new DistributionTestFactory("simple");
         List<BucketId> buckets = getTestBuckets();
@@ -65,6 +74,8 @@ public class DistributionTestCase extends junit.framework.TestCase {
             if (i < nodes.length) t.assertNodeUsed(nodes[i]);
         }
     }
+
+    @Test
     public void testDown() throws Exception {
         test = new DistributionTestFactory("down")
                 .setUpStates("u")
@@ -122,7 +133,7 @@ public class DistributionTestCase extends junit.framework.TestCase {
         Files.move(tempFilePath, filePath, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
     }
 
-
+    @Test
     public void testWriteDistribution() throws IOException, ParseException, Distribution.TooFewBucketBitsInUseException, Distribution.NoDistributorsAvailableException {
         String clusterState = "distributor:9";
         String distributionConfig =
@@ -215,6 +226,7 @@ public class DistributionTestCase extends junit.framework.TestCase {
         writeDistributionTest("retired", clusterState, complexDistributionConfig);
     }
 
+    @Test
     public void testSplitBeyondSplitBitDoesntAffectDistribution() throws Exception {
         Random randomized = new Random(7123161);
         long val = randomized.nextLong();
@@ -223,6 +235,8 @@ public class DistributionTestCase extends junit.framework.TestCase {
             test.recordResult(new BucketId(i, val)).assertNodeUsed(2);
         }
     }
+
+    @Test
     public void testMinimalMovement() throws Exception {
         test = new DistributionTestFactory("minimal-movement")
                 .setClusterState(new ClusterState("distributor:4 .2.s:d"));
@@ -244,6 +258,8 @@ public class DistributionTestCase extends junit.framework.TestCase {
         assertEquals(63, moved);
         assertEquals(81, staying);
     }
+
+    @Test
     public void testAllDistributionBits() throws Exception {
         for (int distbits=0; distbits<=32; ++distbits) {
             test = new DistributionTestFactory("distbit" + distbits)
@@ -305,6 +321,7 @@ public class DistributionTestCase extends junit.framework.TestCase {
         }
     }
 
+    @Test
     public void testHierarchicalDistribution() throws Exception {
         test = new DistributionTestFactory("hierarchical-grouping")
                 .setDistribution(buildHierarchicalConfig(6, 3, 1, "1|2|*", 3));
@@ -312,6 +329,8 @@ public class DistributionTestCase extends junit.framework.TestCase {
             test.recordResult(bucket).assertNodeCount(1);
         }
     }
+
+    @Test
     public void testDistributorGroupTakeover() throws Exception {
         test = new DistributionTestFactory("hierarchical-grouping-distributor-takeover")
                 .setDistribution(buildHierarchicalConfig(6, 3, 1, "1|2|*", 3).distributor_auto_ownership_transfer_on_whole_group_down(true))
@@ -321,6 +340,8 @@ public class DistributionTestCase extends junit.framework.TestCase {
             test.recordResult(bucket).assertNodeCount(1);
         }
     }
+
+    @Test
     public void testDistributorNoGroupTakeover() throws Exception {
         test = new DistributionTestFactory("hierarchical-grouping-distributor-notakeover")
                 .setDistribution(buildHierarchicalConfig(6, 3, 1, "1|2|*", 3).distributor_auto_ownership_transfer_on_whole_group_down(false))
@@ -349,6 +370,8 @@ public class DistributionTestCase extends junit.framework.TestCase {
         }
         assertEquals(15, noneExisting);
     }
+
+    @Test
     public void testHierarchicalDistributionDeep() throws Exception {
         System.out.println(new StorDistributionConfig(buildHierarchicalConfig(8, 5, 3, "*|*", 3)));
         test = new DistributionTestFactory("hierarchical-grouping-deep")
@@ -362,6 +385,8 @@ public class DistributionTestCase extends junit.framework.TestCase {
         // itself only has 375 actual leaf nodes.
         assertEquals(375, nodes.size());
     }
+
+    @Test
     public void testHierarchicalDistributionCapacity() throws Exception {
         StorDistributionConfig.Builder config = buildHierarchicalConfig(6, 3, 1, "1|*", 3);
         config.group.get(1).capacity(3);
@@ -384,4 +409,13 @@ public class DistributionTestCase extends junit.framework.TestCase {
         assertTrue(Arrays.toString(counts) + ": Too large diff" + diff, diff < 3.1);
         assertTrue(Arrays.toString(counts) + ": Too small diff" + diff, diff > 2.9);
     }
+
+    @Test(expected = Distribution.NoDistributorsAvailableException.class)
+    public void clusterDownInHierarchicSetupThrowsNoDistributorsAvailableException() throws Exception {
+        ClusterState clusterState = new ClusterState("cluster:d");
+
+        StorDistributionConfig.Builder config = buildHierarchicalConfig(4, 4, 1, "1|1|1|*", 1);
+        Distribution distr = new Distribution(new StorDistributionConfig(config));
+        distr.getIdealDistributorNode(clusterState, new BucketId(16, 0), "uim");
+    }
 }