diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2022-09-29 08:19:53 +0200 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2022-09-29 08:19:53 +0200 |
commit | b3713cfabf097191635c4545d5d4d958997878e3 (patch) | |
tree | 81eb82506edd7da764157792c7554ddfa07d738d | |
parent | 8d5ae86958bf7bab1d2d06cb94dc81da8aff56e7 (diff) |
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)); } } |