From 843ef14f5a7939e356424ac9d39570605cd6c798 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Tue, 11 May 2021 09:15:44 +0200 Subject: Revert "Revert "Don't consider number of working nodes in coverage"" --- .../com/yahoo/search/dispatch/InvokerFactory.java | 2 +- .../dispatch/searchcluster/SearchCluster.java | 26 +++++----------------- .../searchcluster/SearchClusterCoverageTest.java | 18 +++++++++++++-- 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 { // 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 { } } - 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 { } } - 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 { 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()); + } + } -- cgit v1.2.3