Merge pull request #18278 from vespa-engine/bratseth/cleanup-min-group-coverage

Gc unused config options: min-group-coverage and max nodes per group

16 files changed, 20 insertions, 56 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java
index 51949e78838..efb47e97ccb 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/ContentSearchCluster.java
@@ -316,12 +316,8 @@ public class ContentSearchCluster extends AbstractConfigProducer<SearchCluster>
     }
 
     public void handleRedundancy(Redundancy redundancy) {
-        if (hasIndexedCluster()) {
-            if (usesHierarchicDistribution()) {
-                indexedCluster.setMaxNodesDownPerFixedRow((redundancy.effectiveFinalRedundancy() / groupToSpecMap.size()) - 1);
-            }
+        if (hasIndexedCluster())
             indexedCluster.setSearchableCopies(redundancy.readyCopies());
-        }
         this.redundancy = redundancy;
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/DispatchTuning.java b/config-model/src/main/java/com/yahoo/vespa/model/content/DispatchTuning.java
index 3b694f8986c..786d032578f 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/DispatchTuning.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/DispatchTuning.java
@@ -15,7 +15,6 @@ public class DispatchTuning {
 
     private final Integer maxHitsPerPartition;
     private DispatchPolicy dispatchPolicy;
-    private final Double minGroupCoverage;
     private final Double minActiveDocsCoverage;
 
     public Double getTopkProbability() {
@@ -27,7 +26,6 @@ public class DispatchTuning {
     private DispatchTuning(Builder builder) {
         maxHitsPerPartition = builder.maxHitsPerPartition;
         dispatchPolicy = builder.dispatchPolicy;
-        minGroupCoverage = builder.minGroupCoverage;
         minActiveDocsCoverage = builder.minActiveDocsCoverage;
         topkProbability = builder.topKProbability;
     }
@@ -41,9 +39,6 @@ public class DispatchTuning {
     @SuppressWarnings("unused")
     public void setDispatchPolicy(DispatchPolicy dispatchPolicy) { this.dispatchPolicy = dispatchPolicy; }
 
-    /** Returns the percentage of nodes in a group which must be up for that group to receive queries */
-    public Double getMinGroupCoverage() { return minGroupCoverage; }
-
     /** Returns the percentage of documents which must be available in a group for that group to receive queries */
     public Double getMinActiveDocsCoverage() { return minActiveDocsCoverage; }
 
@@ -51,7 +46,6 @@ public class DispatchTuning {
 
         private Integer maxHitsPerPartition;
         private DispatchPolicy dispatchPolicy;
-        private Double minGroupCoverage;
         private Double minActiveDocsCoverage;
         private Double topKProbability;
 
@@ -81,11 +75,6 @@ public class DispatchTuning {
             }
         }
 
-        public Builder setMinGroupCoverage(Double minGroupCoverage) {
-            this.minGroupCoverage = minGroupCoverage;
-            return this;
-        }
-
         public Builder setMinActiveDocsCoverage(Double minCoverage) {
             this.minActiveDocsCoverage = minCoverage;
             return this;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomTuningDispatchBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomTuningDispatchBuilder.java
index 64911acae1f..f429e40baa9 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomTuningDispatchBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/DomTuningDispatchBuilder.java
@@ -25,12 +25,14 @@ public class DomTuningDispatchBuilder {
         builder.setMaxHitsPerPartition(dispatchElement.childAsInteger("max-hits-per-partition"));
         builder.setTopKProbability(dispatchElement.childAsDouble("top-k-probability"));
         builder.setDispatchPolicy(dispatchElement.childAsString("dispatch-policy"));
-        builder.setMinGroupCoverage(dispatchElement.childAsDouble("min-group-coverage"));
         builder.setMinActiveDocsCoverage(dispatchElement.childAsDouble("min-active-docs-coverage"));
 
+        if (dispatchElement.child("min-group-coverage") != null)
+            logger.logApplicationPackage(Level.WARNING, "Attribute 'min-group-coverage' is deprecated and ignored: " +
+                                                        "Use min-active-docs-coverage instead.");
         if (dispatchElement.child("use-local-node") != null)
             logger.logApplicationPackage(Level.WARNING, "Attribute 'use-local-node' is deprecated and ignored: " +
-                                      "The local node will automatically be preferred when appropriate.");
+                                                        "The local node will automatically be preferred when appropriate.");
         return builder.build();
     }
 
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/DispatchGroup.java b/config-model/src/main/java/com/yahoo/vespa/model/search/DispatchGroup.java
index 384f77737c1..3e70bda216b 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/search/DispatchGroup.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/search/DispatchGroup.java
@@ -54,19 +54,18 @@ public class DispatchGroup {
 
     public int getSearchableCopies() { return sc.getSearchableCopies(); }
 
-    public int getMaxNodesDownPerFixedRow() {
-        return sc.getMaxNodesDownPerFixedRow();
-    }
-
     static class Iterator implements java.util.Iterator<SearchInterface> {
+
         private java.util.Iterator<Map<Integer, SearchInterface>> it1;
         private java.util.Iterator<SearchInterface> it2;
+
         Iterator(Map<Integer, Map<Integer, SearchInterface> > s) {
             it1 = s.values().iterator();
             if (it1.hasNext()) {
                 it2 = it1.next().values().iterator();
             }
         }
+
         @Override
         public boolean hasNext() {
             if (it2 == null) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java
index c77f44d649c..c99549e82e9 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/search/IndexedSearchCluster.java
@@ -46,7 +46,6 @@ public class IndexedSearchCluster extends SearchCluster
     private String routingSelector = null;
     private final List<DocumentDatabase> documentDbs = new LinkedList<>();
     private final UnionConfiguration unionCfg;
-    private int maxNodesDownPerFixedRow = 0;
 
     private int searchableCopies = 1;
 
@@ -261,13 +260,6 @@ public class IndexedSearchCluster extends SearchCluster
         return false;
     }
 
-    int getMaxNodesDownPerFixedRow() {
-        return maxNodesDownPerFixedRow;
-    }
-
-    public void setMaxNodesDownPerFixedRow(int value) {
-        maxNodesDownPerFixedRow = value;
-    }
     public int getSearchableCopies() {
         return searchableCopies;
     }
@@ -305,8 +297,6 @@ public class IndexedSearchCluster extends SearchCluster
         }
         if (tuning.dispatch.getMinActiveDocsCoverage() != null)
             builder.minActivedocsPercentage(tuning.dispatch.getMinActiveDocsCoverage());
-        if (tuning.dispatch.getMinGroupCoverage() != null)
-            builder.minGroupCoverage(tuning.dispatch.getMinGroupCoverage());
         if (tuning.dispatch.getDispatchPolicy() != null) {
             switch (tuning.dispatch.getDispatchPolicy()) {
                 case ADAPTIVE:
@@ -320,7 +310,6 @@ public class IndexedSearchCluster extends SearchCluster
         if (tuning.dispatch.getMaxHitsPerPartition() != null)
             builder.maxHitsPerNode(tuning.dispatch.getMaxHitsPerPartition());
 
-        builder.maxNodesDownPerGroup(rootDispatch.getMaxNodesDownPerFixedRow());
         builder.searchableCopies(rootDispatch.getSearchableCopies());
         if (searchCoverage != null) {
             if (searchCoverage.getMinimum() != null)
diff --git a/config-model/src/main/resources/schema/content.rnc b/config-model/src/main/resources/schema/content.rnc
index 7f52eae6da8..36db55c206c 100644
--- a/config-model/src/main/resources/schema/content.rnc
+++ b/config-model/src/main/resources/schema/content.rnc
@@ -83,7 +83,7 @@ ClusterControllerTuning = element cluster-controller {
 DispatchTuning = element dispatch {
     element max-hits-per-partition { xsd:nonNegativeInteger }? &
     element dispatch-policy { string "round-robin" | string "adaptive" | string "random" }? &
-    element min-group-coverage { xsd:double }? &
+    element min-group-coverage { xsd:double }? & # TODO: Ignored, remove on Vespa 8
     element min-active-docs-coverage { xsd:double }? &
     element top-k-probability { xsd:double }? &
     element use-local-node { string "true" | string "false" }?
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java
index 8a46aaaa230..27a01750210 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/DispatchTuningTest.java
@@ -17,12 +17,10 @@ public class DispatchTuningTest {
         DispatchTuning dispatch = new DispatchTuning.Builder()
                 .setMaxHitsPerPartition(69)
                 .setDispatchPolicy("round-robin")
-                .setMinGroupCoverage(7.5)
                 .setMinActiveDocsCoverage(12.5)
                 .setTopKProbability(18.3)
                 .build();
         assertEquals(69, dispatch.getMaxHitsPerPartition().intValue());
-        assertEquals(7.5, dispatch.getMinGroupCoverage().doubleValue(), 0.0);
         assertEquals(12.5, dispatch.getMinActiveDocsCoverage().doubleValue(), 0.0);
         assertTrue(DispatchTuning.DispatchPolicy.ROUNDROBIN == dispatch.getDispatchPolicy());
         assertEquals(18.3, dispatch.getTopkProbability(), 0.0);
@@ -33,7 +31,6 @@ public class DispatchTuningTest {
                 .setDispatchPolicy("random")
                 .build();
         assertTrue(DispatchTuning.DispatchPolicy.ADAPTIVE == dispatch.getDispatchPolicy());
-        assertNull(dispatch.getMinGroupCoverage());
         assertNull(dispatch.getMinActiveDocsCoverage());
     }
 
@@ -43,7 +40,6 @@ public class DispatchTuningTest {
                 .setDispatchPolicy("adaptive")
                 .build();
         assertTrue(DispatchTuning.DispatchPolicy.ADAPTIVE == dispatch.getDispatchPolicy());
-        assertNull(dispatch.getMinGroupCoverage());
         assertNull(dispatch.getMinActiveDocsCoverage());
     }
 
@@ -53,7 +49,6 @@ public class DispatchTuningTest {
         assertNull(dispatch.getMaxHitsPerPartition());
         assertNull(dispatch.getDispatchPolicy());
         assertNull(dispatch.getMinActiveDocsCoverage());
-        assertNull(dispatch.getMinGroupCoverage());
         assertNull(dispatch.getTopkProbability());
     }
 
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedHierarchicDistributionTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedHierarchicDistributionTest.java
index 3be592e54e7..6c8cb393d3f 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedHierarchicDistributionTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/IndexedHierarchicDistributionTest.java
@@ -201,7 +201,6 @@ public class IndexedHierarchicDistributionTest {
         assertEquals(8, dg.getRowBits());
         assertEquals(3, dg.getNumPartitions());
         assertEquals(true, dg.useFixedRowInDispatch());
-        assertEquals(1, dg.getMaxNodesDownPerFixedRow());
         ArrayList<SearchInterface> list = new ArrayList<>();
         for(SearchInterface si : dg.getSearchersIterable()) {
             list.add(si);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java
index afeffbbc875..1426b094971 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/ClusterTest.java
@@ -73,7 +73,6 @@ public class ClusterTest {
                                                    joinLines(
                                                            "<max-hits-per-partition>77</max-hits-per-partition>",
                                                            "<dispatch-policy>round-robin</dispatch-policy>",
-                                                           "<min-group-coverage>13</min-group-coverage>",
                                                            "<min-active-docs-coverage>93</min-active-docs-coverage>",
                                                            "<top-k-probability>0.777</top-k-probability>"),
                                                    false);
@@ -82,7 +81,6 @@ public class ClusterTest {
         DispatchConfig config = new DispatchConfig(builder);
         assertEquals(2, config.searchableCopies());
         assertEquals(93.0, config.minActivedocsPercentage(), DELTA);
-        assertEquals(13.0, config.minGroupCoverage(), DELTA);
         assertEquals(DispatchConfig.DistributionPolicy.ROUNDROBIN, config.distributionPolicy());
         assertEquals(77, config.maxHitsPerNode());
         assertEquals(0.777, config.topKProbability(), DELTA);
@@ -97,7 +95,6 @@ public class ClusterTest {
         DispatchConfig config = new DispatchConfig(builder);
         assertEquals(2, config.searchableCopies());
         assertEquals(DispatchConfig.DistributionPolicy.ADAPTIVE, config.distributionPolicy());
-        assertEquals(0, config.maxNodesDownPerGroup());
         assertEquals(1.0, config.maxWaitAfterCoverageFactor(), DELTA);
         assertEquals(0, config.minWaitAfterCoverageFactor(), DELTA);
         assertEquals(8, config.numJrtConnectionsPerNode());
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/DomDispatchTuningBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/DomDispatchTuningBuilderTest.java
index abfb03e41dd..7533bf1ef9d 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/DomDispatchTuningBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/cluster/DomDispatchTuningBuilderTest.java
@@ -44,7 +44,6 @@ public class DomDispatchTuningBuilderTest {
                 "  </tuning>" +
                 "</content>");
         assertNull(dispatch.getMaxHitsPerPartition());
-        assertNull(dispatch.getMinGroupCoverage());
         assertNull(dispatch.getMinActiveDocsCoverage());
         assertNull(dispatch.getDispatchPolicy());
         assertNull(dispatch.getTopkProbability());
@@ -57,14 +56,12 @@ public class DomDispatchTuningBuilderTest {
                 "  <tuning>" +
                 "    <dispatch>" +
                 "      <max-hits-per-partition>69</max-hits-per-partition>" +
-                "      <min-group-coverage>7.5</min-group-coverage>" +
                 "      <min-active-docs-coverage>12.5</min-active-docs-coverage>" +
                 "      <top-k-probability>0.999</top-k-probability>" +
                 "    </dispatch>" +
                 "  </tuning>" +
                 "</content>");
         assertEquals(69, dispatch.getMaxHitsPerPartition().intValue());
-        assertEquals(7.5, dispatch.getMinGroupCoverage().doubleValue(), 0.0);
         assertEquals(12.5, dispatch.getMinActiveDocsCoverage().doubleValue(), 0.0);
         assertEquals(0.999, dispatch.getTopkProbability().doubleValue(), 0.0);
     }
diff --git a/configdefinitions/src/vespa/dispatch.def b/configdefinitions/src/vespa/dispatch.def
index aa40c317d75..150fe2e1603 100644
--- a/configdefinitions/src/vespa/dispatch.def
+++ b/configdefinitions/src/vespa/dispatch.def
@@ -7,10 +7,10 @@ namespace=vespa.config.search
 # for that group to be included in queries
 minActivedocsPercentage double default=97.0
 
-# Minimum coverage for allowing a group to be considered for serving
+# Not used. TODO: Remove on Vespa 8
 minGroupCoverage double default=100
 
-# Maximum number of nodes allowed to be down for group to be considered for serving
+# Not used. TODO: Remove on Vespa 8
 maxNodesDownPerGroup int default=0
 
 # Distribution policy for group selection
diff --git a/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java
index 0d491d2f0c1..8eee7c11d3e 100644
--- a/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java
+++ b/container-search/src/main/java/com/yahoo/search/cluster/BaseNodeMonitor.java
@@ -44,7 +44,7 @@ public abstract class BaseNodeMonitor<T> {
     protected MonitorConfiguration configuration;
 
     /** Is the node we monitor part of an internal Vespa cluster or not */
-    private boolean internal;
+    private final boolean internal;
 
     public BaseNodeMonitor(boolean internal) {
         this.internal=internal;
diff --git a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java
index 27d8bb27ee8..c9b8aeee417 100644
--- a/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java
+++ b/container-search/src/main/java/com/yahoo/search/cluster/ClusterMonitor.java
@@ -25,9 +25,9 @@ import java.util.logging.Logger;
  */
 public class ClusterMonitor<T> {
 
-    private final MonitorConfiguration configuration = new MonitorConfiguration();
+    private static final Logger log = Logger.getLogger(ClusterMonitor.class.getName());
 
-    private static Logger log = Logger.getLogger(ClusterMonitor.class.getName());
+    private final MonitorConfiguration configuration = new MonitorConfiguration();
 
     private final NodeManager<T> nodeManager;
 
diff --git a/container-search/src/main/java/com/yahoo/search/cluster/MonitorConfiguration.java b/container-search/src/main/java/com/yahoo/search/cluster/MonitorConfiguration.java
index a2fb982e3c5..95f51b374d6 100644
--- a/container-search/src/main/java/com/yahoo/search/cluster/MonitorConfiguration.java
+++ b/container-search/src/main/java/com/yahoo/search/cluster/MonitorConfiguration.java
@@ -9,7 +9,7 @@ package com.yahoo.search.cluster;
 public class MonitorConfiguration  {
 
     /** The interval in ms between consecutive checks of the monitored nodes */
-    private long checkInterval=1000;
+    private long checkInterval = 1000;
 
     /** The number of milliseconds to attempt to complete a request before giving up */
     private final long requestTimeout = 980;
@@ -18,6 +18,7 @@ public class MonitorConfiguration  {
     private long failLimit = 5000;
 
     /** Sets the interval between each ping of idle or failing nodes. Default is 1000 ms. */
+    @Deprecated // TODO: Remove on Vespa 8
     public void setCheckInterval(long intervalMs) { this.checkInterval = intervalMs; }
 
     /** Returns the interval between each ping of idle or failing nodes. Default is 1000 ms. */
@@ -59,6 +60,7 @@ public class MonitorConfiguration  {
      * Sets the number of milliseconds a node is allowed to fail before we
      * mark it as not working
      */
+    @Deprecated // TODO: Remove on Vespa 8
     public void setFailLimit(long failLimit) { this.failLimit=failLimit; }
 
     /**
@@ -86,6 +88,7 @@ public class MonitorConfiguration  {
     @Deprecated // TODO: Remove on Vespa 8
     public void setQuarantineTime(long quarantineTime) { }
 
+    @Override
     public String toString() {
         return "monitor configuration [" +
                "checkInterval: " + checkInterval +
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..9ae25518969 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
@@ -368,7 +368,7 @@ public class SearchCluster implements NodeManager<Node> {
      */
     public boolean isPartialGroupCoverageSufficient(List<Node> nodes) {
         if (orderedGroups().size() == 1)
-            return nodes.size() >= wantedGroupSize() - dispatchConfig.maxNodesDownPerGroup();
+            return true;
         long activeDocuments = nodes.stream().mapToLong(Node::getActiveDocuments).sum();
         return isGroupCoverageSufficient(activeDocuments, medianDocumentsPerGroup());
     }
@@ -378,7 +378,6 @@ public class SearchCluster implements NodeManager<Node> {
         boolean changed = group.isFullCoverageStatusChanged(fullCoverage);
         if (changed || (!fullCoverage && System.currentTimeMillis() > nextLogTime)) {
             nextLogTime = System.currentTimeMillis() + 30 * 1000;
-            int requiredNodes = group.nodes().size() - dispatchConfig.maxNodesDownPerGroup();
             if (fullCoverage) {
                 log.info("Cluster " + clusterId + ": " + group + " has full coverage. " +
                          "Active documents: " + group.getActiveDocuments() + "/" + medianDocuments + ", " +
@@ -391,7 +390,7 @@ public class SearchCluster implements NodeManager<Node> {
                 }
                 log.warning("Cluster " + clusterId + ": " + group + " has reduced coverage: " +
                             "Active documents: " + group.getActiveDocuments() + "/" + medianDocuments + ", " +
-                            "working nodes: " + group.workingNodes() + "/" + group.nodes().size() + " required " + requiredNodes +
+                            "working nodes: " + group.workingNodes() + "/" + group.nodes().size() +
                             ", unresponsive nodes: " + (unresponsive.toString().isEmpty() ? " none" : unresponsive));
             }
         }
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java
index 9d96b2302d7..8db54218e56 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/MockSearchCluster.java
@@ -119,7 +119,6 @@ public class MockSearchCluster extends SearchCluster {
         DispatchConfig.Builder builder = new DispatchConfig.Builder();
         builder.minActivedocsPercentage(88.0);
         builder.minGroupCoverage(99.0);
-        builder.maxNodesDownPerGroup(0);
         builder.minSearchCoverage(minSearchCoverage);
         builder.distributionPolicy(DispatchConfig.DistributionPolicy.Enum.ROUNDROBIN);
         if (minSearchCoverage < 100.0) {