From c05117e4ff37ede9550a7ddc1bfc330795268194 Mon Sep 17 00:00:00 2001 From: Jon Bratseth Date: Mon, 10 May 2021 17:05:20 +0200 Subject: 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, 24 insertions(+), 22 deletions(-) (limited to 'container-search') 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 1de274ce6cf..1bcb640e3a5 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.isEmpty()) { + if (invokers.size() == 0) { 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 159a42676ec..ce834b108db 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,17 +311,19 @@ 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.getActiveDocuments(), + boolean sufficientCoverage = isGroupCoverageSufficient(group.workingNodes(), + group.getActiveDocuments(), group.getActiveDocuments()); trackGroupCoverageChanges(group, sufficientCoverage, group.getActiveDocuments()); } private void pingIterationCompletedMultipleGroups() { - orderedGroups().forEach(Group::aggregateNodeValues); + aggregateNodeValues(); long medianDocuments = medianDocumentsPerGroup(); boolean anyGroupsSufficientCoverage = false; for (Group group : orderedGroups()) { - boolean sufficientCoverage = isGroupCoverageSufficient(group.getActiveDocuments(), + boolean sufficientCoverage = isGroupCoverageSufficient(group.workingNodes(), + group.getActiveDocuments(), medianDocuments); anyGroupsSufficientCoverage = anyGroupsSufficientCoverage || sufficientCoverage; updateSufficientCoverage(group, sufficientCoverage); @@ -329,6 +331,10 @@ 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()); @@ -350,13 +356,23 @@ public class SearchCluster implements NodeManager { } } - private boolean isGroupCoverageSufficient(long activeDocuments, long medianDocuments) { + private boolean isGroupCoverageSufficient(int workingNodesInGroup, 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()); @@ -370,7 +386,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(activeDocuments, medianDocumentsPerGroup()); + return isGroupCoverageSufficient(nodes.size(), 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 65e7173c4ee..6338107d4b6 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,21 +74,7 @@ public class SearchClusterCoverageTest { @Test public void three_groups_one_has_a_node_down() { - var tester = new SearchClusterTester(3, 3); - - tester.setDocsPerNode(100, 0); - tester.setDocsPerNode(100, 1); - tester.setDocsPerNode(100, 2); - tester.setWorking(1, 1, false); - tester.pingIterationCompleted(); - assertTrue(tester.group(0).hasSufficientCoverage()); - assertFalse(tester.group(1).hasSufficientCoverage()); - 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); + var tester = new SearchClusterTester(3, 3); tester.setDocsPerNode(100, 0); tester.setDocsPerNode(150, 1); @@ -96,7 +82,7 @@ public class SearchClusterCoverageTest { tester.setWorking(1, 1, false); tester.pingIterationCompleted(); assertTrue(tester.group(0).hasSufficientCoverage()); - assertTrue("Sufficient documents on remaining two nodes", tester.group(1).hasSufficientCoverage()); + assertFalse(tester.group(1).hasSufficientCoverage()); assertTrue(tester.group(2).hasSufficientCoverage()); } -- cgit v1.2.3