Merge pull request #24252 from vespa-engine/balder/gc-target-active-docs-option

Remove all traces from computeCoverageFromTargetActiveDocs

11 files changed, 10 insertions, 70 deletions

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 db41549d836..e002f3ec3eb 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
@@ -319,7 +319,6 @@ public class IndexedSearchCluster extends SearchCluster
 
         builder.searchableCopies(rootDispatch.getSearchableCopies());
         builder.redundancy(rootDispatch.getRedundancy());
-        builder.computeCoverageFromTargetActiveDocs(true);
         if (searchCoverage != null) {
             if (searchCoverage.getMinimum() != null)
                 builder.minSearchCoverage(searchCoverage.getMinimum() * 100.0);
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java
index 717b70cc686..ac291fc578f 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentClusterTest.java
@@ -1036,22 +1036,6 @@ public class ContentClusterTest extends ContentBaseTest {
         }
     }
 
-    private boolean coverageIsComputedFromTargetActive() {
-        TestProperties properties = new TestProperties();
-        VespaModel model = createEnd2EndOneNode(properties);
-
-        ContentCluster cc = model.getContentClusters().get("storage");
-        DispatchConfig.Builder builder = new DispatchConfig.Builder();
-        cc.getSearch().getConfig(builder);
-
-        return (new DispatchConfig(builder)).computeCoverageFromTargetActiveDocs();
-    }
-
-    @Test
-    public void coverage_from_target_active_dispatch_config_is_controlled_by_properties() {
-        assertTrue(coverageIsComputedFromTargetActive());
-    }
-
     private boolean resolveThreePhaseUpdateConfigWithFeatureFlag(boolean flagEnableThreePhase) {
         VespaModel model = createEnd2EndOneNode(new TestProperties().setUseThreePhaseUpdates(flagEnableThreePhase));
 
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 98999d02b67..1dbbe9bea9d 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
@@ -96,7 +96,6 @@ public class ClusterTest {
         DispatchConfig config = new DispatchConfig(builder);
         assertEquals(2, config.searchableCopies());
         assertEquals(3, config.redundancy());
-        assertTrue(config.computeCoverageFromTargetActiveDocs());
         assertEquals(DispatchConfig.DistributionPolicy.ADAPTIVE, config.distributionPolicy());
         assertEquals(1.0, config.maxWaitAfterCoverageFactor(), DELTA);
         assertEquals(0, config.minWaitAfterCoverageFactor(), DELTA);
diff --git a/configdefinitions/src/vespa/dispatch.def b/configdefinitions/src/vespa/dispatch.def
index fb3fc4a331a..9addfca1559 100644
--- a/configdefinitions/src/vespa/dispatch.def
+++ b/configdefinitions/src/vespa/dispatch.def
@@ -73,7 +73,3 @@ node[].port int
 # TODO(bjorncs) Remove after May 2022
 # Temporary feature flag
 mergeGroupingResultInSearchInvokerEnabled bool default=false
-
-# Whether degraded coverage computation will take target active docs into
-# account, not just currently active docs.
-computeCoverageFromTargetActiveDocs bool default=false
diff --git a/container-core/abi-spec.json b/container-core/abi-spec.json
index e39391fa4a3..59c2941ec33 100644
--- a/container-core/abi-spec.json
+++ b/container-core/abi-spec.json
@@ -219,7 +219,6 @@
       "public int getFullResultSets()",
       "public int getResultSets()",
       "public int getResultPercentage()",
-      "public com.yahoo.container.handler.Coverage useTargetActiveForCoverageComputation(boolean)",
       "public com.yahoo.container.logging.Coverage toLoggingCoverage()"
     ],
     "fields": [
diff --git a/container-core/src/main/java/com/yahoo/container/handler/Coverage.java b/container-core/src/main/java/com/yahoo/container/handler/Coverage.java
index fdcd225ddf5..fccce9cc8fb 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/Coverage.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/Coverage.java
@@ -1,8 +1,6 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.handler;
 
-import com.yahoo.api.annotations.Beta;
-
 /**
  * The coverage report for a result set.
  *
@@ -11,7 +9,6 @@ import com.yahoo.api.annotations.Beta;
  */
 public class Coverage {
 
-    private boolean useTargetActiveForCoverageComputation = false;
     protected long docs;
     protected long active;
     protected long targetActive;
@@ -162,28 +159,18 @@ public class Coverage {
         if (getResultSets() == 0) {
             return 0;
         }
-        long total = useTargetActiveForCoverageComputation ? targetActive : active;
+        long total = targetActive;
         if (docs < total) {
             return (int) Math.round(docs * 100.0d / total);
         }
         return getFullResultSets() * 100 / getResultSets();
     }
 
-    /**
-     * Decides whether active or target active shall be used for coverage computation.
-     * DO NOT USE - This is only for temporary internal use.
-     */
-    @Beta
-    public Coverage useTargetActiveForCoverageComputation(boolean value) {
-        useTargetActiveForCoverageComputation = value;
-        return this;
-    }
-
     public com.yahoo.container.logging.Coverage toLoggingCoverage() {
         int degradation = com.yahoo.container.logging.Coverage.toDegradation(isDegradedByMatchPhase(),
                 isDegradedByTimeout(),
                 isDegradedByAdapativeTimeout());
-        return new com.yahoo.container.logging.Coverage(getDocs(), getActive(), getTargetActive(), degradation, useTargetActiveForCoverageComputation);
+        return new com.yahoo.container.logging.Coverage(getDocs(), getActive(), getTargetActive(), degradation);
     }
 
 }
diff --git a/container-core/src/main/java/com/yahoo/container/logging/Coverage.java b/container-core/src/main/java/com/yahoo/container/logging/Coverage.java
index c016a203256..d4884027c2b 100644
--- a/container-core/src/main/java/com/yahoo/container/logging/Coverage.java
+++ b/container-core/src/main/java/com/yahoo/container/logging/Coverage.java
@@ -9,19 +9,14 @@ public class Coverage {
     private final long active;
     private final long targetActive;
     private final int degradedReason;
-    private final boolean useTargetActiveForCoverageComputation;
     private final static int DEGRADED_BY_MATCH_PHASE = 1;
     private final static int DEGRADED_BY_TIMEOUT = 2;
     private final static int DEGRADED_BY_ADAPTIVE_TIMEOUT = 4;
-    public Coverage(long docs, long active, long targetActive, int degradedReason, boolean useTargetActiveForCoverageComputation) {
+    public Coverage(long docs, long active, long targetActive, int degradedReason) {
         this.docs = docs;
         this.active = active;
         this.targetActive = targetActive;
         this.degradedReason = degradedReason;
-        this.useTargetActiveForCoverageComputation = useTargetActiveForCoverageComputation;
-    }
-    Coverage(long docs, long active, long targetActive, int degradedReason) {
-        this(docs, active, targetActive, degradedReason, true);
     }
 
     public long getDocs() {
@@ -60,7 +55,7 @@ public class Coverage {
      * about had.
      */
     public int getResultPercentage() {
-        long total = useTargetActiveForCoverageComputation ? targetActive : active;
+        long total = targetActive;
         if (docs < total) {
             return (int) Math.round(docs * 100.0d / total);
         }
diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/CoverageAggregator.java b/container-search/src/main/java/com/yahoo/search/dispatch/CoverageAggregator.java
index 4e19c8a2e22..e3801ad64f4 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/CoverageAggregator.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/CoverageAggregator.java
@@ -54,11 +54,10 @@ public class CoverageAggregator {
         this.failedNodes = failedNodes;
     }
 
-    public Coverage createCoverage(TimeoutHandler timeoutHandler, boolean useTargetActiveForCoverageComputation) {
+    public Coverage createCoverage(TimeoutHandler timeoutHandler) {
         Coverage coverage = new Coverage(answeredDocs, answeredActiveDocs, answeredNodesParticipated, 1);
         coverage.setNodesTried(askedNodes);
         coverage.setTargetActive(answeredTargetActiveDocs);
-        coverage.useTargetActiveForCoverageComputation(useTargetActiveForCoverageComputation);
         int degradedReason = 0;
         if (timedOut) {
             degradedReason |= timeoutHandler.reason();
diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java
index 2717b48839e..b4f04da5986 100644
--- a/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java
+++ b/container-search/src/main/java/com/yahoo/search/dispatch/InterleavedSearchInvoker.java
@@ -127,8 +127,8 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM
         groupingResultAggregator.toAggregatedHit().ifPresent(h -> result.getResult().hits().add(h));
 
         insertNetworkErrors(result.getResult());
-        CoverageAggregator adjusted = coverageAggregator.adjustedDegradedCoverage(redundancyForCoverage(searchCluster.dispatchConfig()), timeoutHandler);
-        result.getResult().setCoverage(adjusted.createCoverage(timeoutHandler, searchCluster.dispatchConfig().computeCoverageFromTargetActiveDocs()));
+        CoverageAggregator adjusted = coverageAggregator.adjustedDegradedCoverage((int)searchCluster.dispatchConfig().redundancy(), timeoutHandler);
+        result.getResult().setCoverage(adjusted.createCoverage(timeoutHandler));
 
         int needed = query.getOffset() + query.getHits();
         for (int index = query.getOffset(); (index < merged.size()) && (index < needed); index++) {
@@ -138,10 +138,6 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM
         return result;
     }
 
-    private int redundancyForCoverage(DispatchConfig config) {
-        return (int)(config.computeCoverageFromTargetActiveDocs() ? config.redundancy() : config.searchableCopies());
-    }
-
     private void insertNetworkErrors(Result result) {
         // Network errors will be reported as errors only when all nodes fail, otherwise they are just traced
         boolean asErrors = coverageAggregator.hasNoAnswers();
diff --git a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java
index 180beb43ee8..938256b6055 100644
--- a/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java
+++ b/container-search/src/test/java/com/yahoo/search/dispatch/InterleavedSearchInvokerTest.java
@@ -424,20 +424,12 @@ public class InterleavedSearchInvokerTest {
             assertTrue(cov.isDegradedByTimeout());
         }
     }
-    @Test
-    void requireCorrectCoverageCalculationWhenDegradedCoverageIsExpectedUsingActiveDocs() throws IOException {
-        verifyCorrectCoverageCalculationWhenDegradedCoverageIsExpected(MockSearchCluster.createDispatchConfig(100.0, List.of())
-                .searchableCopies(1)
-                .redundancy(2)
-                .build(),
-                50);
-    }
+
     @Test
     void requireCorrectCoverageCalculationWhenDegradedCoverageIsExpectedUsingTargetActiveDocs() throws IOException {
         verifyCorrectCoverageCalculationWhenDegradedCoverageIsExpected(MockSearchCluster.createDispatchConfig(100.0, List.of())
                 .searchableCopies(1)
                 .redundancy(1)
-                .computeCoverageFromTargetActiveDocs(true)
                 .build(),
                 42);
     }
diff --git a/container-search/src/test/java/com/yahoo/search/result/CoverageTestCase.java b/container-search/src/test/java/com/yahoo/search/result/CoverageTestCase.java
index e2675b09307..4d521e3e8b4 100644
--- a/container-search/src/test/java/com/yahoo/search/result/CoverageTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/result/CoverageTestCase.java
@@ -33,13 +33,8 @@ public class CoverageTestCase {
     }
 
     @Test
-    void testCoverageBasedOnActive() {
-        var c = new Coverage(8, 10).setTargetActive(16);
-        assertEquals(80, c.getResultPercentage());
-    }
-    @Test
     void testCoverageBasedOnTargetActive() {
-        var c = new Coverage(8, 10).setTargetActive(16).useTargetActiveForCoverageComputation(true);
+        var c = new Coverage(8, 10).setTargetActive(16);
         assertEquals(50, c.getResultPercentage());
     }
 
@@ -92,8 +87,7 @@ public class CoverageTestCase {
 
     @Test
     void testCoverageConversion() {
-        verifyCoverageConversion(new Coverage(6, 10).setDegradedReason(7).setTargetActive(12).useTargetActiveForCoverageComputation(false));
-        verifyCoverageConversion(new Coverage(6, 10).setDegradedReason(7).setTargetActive(12).useTargetActiveForCoverageComputation(true));
+        verifyCoverageConversion(new Coverage(6, 10).setDegradedReason(7).setTargetActive(12));
     }
 
 }