Revert "Revert "Don't consider number of working nodes in coverage""

3 files changed, 22 insertions, 24 deletions

diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java
index 1bcb640e3a5..1de274ce6cf 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/InvokerFactory.java
@@ -82,7 +82,7 @@ public abstract class InvokerFactory {
             if ( ! searchCluster.isPartialGroupCoverageSufficient(success) && !acceptIncompleteCoverage) {
                 return Optional.empty();
             }
-            if (invokers.size() == 0) {
+            if (invokers.isEmpty()) {
                 return Optional.of(createCoverageErrorInvoker(nodes, failed));
             }
         }
diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java
index ce834b108db..159a42676ec 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/searchcluster/SearchCluster.java
@@ -311,19 +311,17 @@ public class SearchCluster implements NodeManager<Node> {
         // With just one group sufficient coverage may not be the same as full coverage, as the
         // group will always be marked sufficient for use.
         updateSufficientCoverage(group, true);
-        boolean sufficientCoverage = isGroupCoverageSufficient(group.workingNodes(),
-                                                               group.getActiveDocuments(),
+        boolean sufficientCoverage = isGroupCoverageSufficient(group.getActiveDocuments(),
                                                                group.getActiveDocuments());
         trackGroupCoverageChanges(group, sufficientCoverage, group.getActiveDocuments());
     }
 
     private void pingIterationCompletedMultipleGroups() {
-        aggregateNodeValues();
+        orderedGroups().forEach(Group::aggregateNodeValues);
         long medianDocuments = medianDocumentsPerGroup();
         boolean anyGroupsSufficientCoverage = false;
         for (Group group : orderedGroups()) {
-            boolean sufficientCoverage = isGroupCoverageSufficient(group.workingNodes(),
-                                                                   group.getActiveDocuments(),
+            boolean sufficientCoverage = isGroupCoverageSufficient(group.getActiveDocuments(),
                                                                    medianDocuments);
             anyGroupsSufficientCoverage = anyGroupsSufficientCoverage || sufficientCoverage;
             updateSufficientCoverage(group, sufficientCoverage);
@@ -331,10 +329,6 @@ public class SearchCluster implements NodeManager<Node> {
         }
     }
 
-    private void aggregateNodeValues() {
-        orderedGroups().forEach(Group::aggregateNodeValues);
-    }
-
     private long medianDocumentsPerGroup() {
         if (orderedGroups().isEmpty()) return 0;
         var activeDocuments = orderedGroups().stream().map(Group::getActiveDocuments).collect(Collectors.toList());
@@ -356,23 +350,13 @@ public class SearchCluster implements NodeManager<Node> {
         }
     }
 
-    private boolean isGroupCoverageSufficient(int workingNodesInGroup, long activeDocuments, long medianDocuments) {
+    private boolean isGroupCoverageSufficient(long activeDocuments, long medianDocuments) {
         double documentCoverage = 100.0 * (double) activeDocuments / medianDocuments;
         if (medianDocuments > 0 && documentCoverage < dispatchConfig.minActivedocsPercentage())
             return false;
-
-        if ( ! isGroupNodeCoverageSufficient(workingNodesInGroup))
-            return false;
-
         return true;
     }
 
-    private boolean isGroupNodeCoverageSufficient(int workingNodesInGroup) {
-        int nodesAllowedDown = dispatchConfig.maxNodesDownPerGroup()
-                + (int) (((double) wantedGroupSize() * (100.0 - dispatchConfig.minGroupCoverage())) / 100.0);
-        return workingNodesInGroup + nodesAllowedDown >= wantedGroupSize();
-    }
-
     public boolean isGroupWellBalanced(OptionalInt groupId) {
         if (groupId.isEmpty()) return false;
         Group group = groups().get(groupId.getAsInt());
@@ -386,7 +370,7 @@ public class SearchCluster implements NodeManager<Node> {
         if (orderedGroups().size() == 1)
             return nodes.size() >= wantedGroupSize() - dispatchConfig.maxNodesDownPerGroup();
         long activeDocuments = nodes.stream().mapToLong(Node::getActiveDocuments).sum();
-        return isGroupCoverageSufficient(nodes.size(), activeDocuments, medianDocumentsPerGroup());
+        return isGroupCoverageSufficient(activeDocuments, medianDocumentsPerGroup());
     }
 
     private void trackGroupCoverageChanges(Group group, boolean fullCoverage, long medianDocuments) {
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterCoverageTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterCoverageTest.java
index 6338107d4b6..65e7173c4ee 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterCoverageTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/searchcluster/SearchClusterCoverageTest.java
@@ -74,10 +74,10 @@ public class SearchClusterCoverageTest {
 
     @Test
     public void three_groups_one_has_a_node_down() {
-        var tester =  new SearchClusterTester(3, 3);
+        var tester = new SearchClusterTester(3, 3);
 
         tester.setDocsPerNode(100, 0);
-        tester.setDocsPerNode(150, 1);
+        tester.setDocsPerNode(100, 1);
         tester.setDocsPerNode(100, 2);
         tester.setWorking(1, 1, false);
         tester.pingIterationCompleted();
@@ -86,4 +86,18 @@ public class SearchClusterCoverageTest {
         assertTrue(tester.group(2).hasSufficientCoverage());
     }
 
+    @Test
+    public void three_groups_one_has_a_node_down_but_remaining_has_enough_docs() {
+        var tester = new SearchClusterTester(3, 3);
+
+        tester.setDocsPerNode(100, 0);
+        tester.setDocsPerNode(150, 1);
+        tester.setDocsPerNode(100, 2);
+        tester.setWorking(1, 1, false);
+        tester.pingIterationCompleted();
+        assertTrue(tester.group(0).hasSufficientCoverage());
+        assertTrue("Sufficient documents on remaining two nodes", tester.group(1).hasSufficientCoverage());
+        assertTrue(tester.group(2).hasSufficientCoverage());
+    }
+
 }