diff options
11 files changed, 128 insertions, 7 deletions
diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java index e0e4318ccc8..7cb36374568 100644 --- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java +++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java @@ -116,6 +116,7 @@ public interface ModelContext { @ModelFeatureFlag(owners = {"arnej"}) default boolean useQrserverServiceName() { return true; } @ModelFeatureFlag(owners = {"bjorncs", "baldersheim"}) default boolean enableJdiscPreshutdownCommand() { return true; } @ModelFeatureFlag(owners = {"arnej"}) default boolean avoidRenamingSummaryFeatures() { return false; } + @ModelFeatureFlag(owners = {"bjorncs", "baldersheim"}) default boolean mergeGroupingResultInSearchInvoker() { return false; } } /** Warning: As elsewhere in this package, do not make backwards incompatible changes that will break old config models! */ 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 22b752777e9..e483351a25a 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 @@ -158,7 +158,7 @@ public class ContentSearchCluster extends AbstractConfigProducer<SearchCluster> String clusterName, ContentSearchCluster search) { List<ModelElement> indexedDefs = getIndexedSchemas(clusterElem); if (!indexedDefs.isEmpty()) { - IndexedSearchCluster isc = new IndexedSearchCluster(search, clusterName, 0); + IndexedSearchCluster isc = new IndexedSearchCluster(deployState, search, clusterName, 0); isc.setRoutingSelector(clusterElem.childAsString("documents.selection")); Double visibilityDelay = clusterElem.childAsDouble("engine.proton.visibility-delay"); 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 fb7c6696b54..53aac23135a 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 @@ -6,7 +6,6 @@ import com.yahoo.config.model.producer.AbstractConfigProducer; import com.yahoo.prelude.fastsearch.DocumentdbInfoConfig; import com.yahoo.search.config.IndexInfoConfig; import com.yahoo.searchdefinition.DocumentOnlySchema; -import com.yahoo.searchdefinition.Schema; import com.yahoo.searchdefinition.derived.DerivedConfiguration; import com.yahoo.vespa.config.search.AttributesConfig; import com.yahoo.vespa.config.search.DispatchConfig; @@ -34,6 +33,7 @@ public class IndexedSearchCluster extends SearchCluster IlscriptsConfig.Producer, DispatchConfig.Producer { + private final boolean mergeGroupingResultInSearchInvoker; private String indexingClusterName = null; // The name of the docproc cluster to run indexing, by config. private String indexingChainName = null; @@ -63,10 +63,11 @@ public class IndexedSearchCluster extends SearchCluster return routingSelector; } - public IndexedSearchCluster(AbstractConfigProducer<SearchCluster> parent, String clusterName, int index) { + public IndexedSearchCluster(DeployState deployState, AbstractConfigProducer<SearchCluster> parent, String clusterName, int index) { super(parent, clusterName, index); unionCfg = new UnionConfiguration(this, documentDbs); rootDispatch = new DispatchGroup(this); + mergeGroupingResultInSearchInvoker = deployState.featureFlags().mergeGroupingResultInSearchInvoker(); } @Override @@ -320,6 +321,7 @@ public class IndexedSearchCluster extends SearchCluster builder.maxWaitAfterCoverageFactor(searchCoverage.getMaxWaitAfterCoverageFactor()); } builder.warmuptime(5.0); + builder.mergeGroupingResultInSearchInvokerEnabled(mergeGroupingResultInSearchInvoker); } @Override diff --git a/configdefinitions/src/vespa/dispatch.def b/configdefinitions/src/vespa/dispatch.def index 17f42a73bfd..fef9300a410 100644 --- a/configdefinitions/src/vespa/dispatch.def +++ b/configdefinitions/src/vespa/dispatch.def @@ -71,3 +71,6 @@ node[].host string # The rpc port of this search node node[].port int + +# Temporary feature flag +mergeGroupingResultInSearchInvokerEnabled bool default=false diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java index 5a813c7886a..6222e7b1788 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java @@ -209,6 +209,7 @@ public class ModelContextImpl implements ModelContext { private final boolean inhibitDefaultMergesWhenGlobalMergesPending; private final boolean useQrserverServiceName; private final boolean avoidRenamingSummaryFeatures; + private final boolean mergeGroupingResultInSearchInvoker; public FeatureFlags(FlagSource source, ApplicationId appId) { this.defaultTermwiseLimit = flagValue(source, appId, Flags.DEFAULT_TERM_WISE_LIMIT); @@ -256,6 +257,7 @@ public class ModelContextImpl implements ModelContext { this.inhibitDefaultMergesWhenGlobalMergesPending = flagValue(source, appId, Flags.INHIBIT_DEFAULT_MERGES_WHEN_GLOBAL_MERGES_PENDING); this.useQrserverServiceName = flagValue(source, appId, Flags.USE_QRSERVER_SERVICE_NAME); this.avoidRenamingSummaryFeatures = flagValue(source, appId, Flags.AVOID_RENAMING_SUMMARY_FEATURES); + this.mergeGroupingResultInSearchInvoker = flagValue(source, appId, Flags.MERGE_GROUPING_RESULT_IN_SEARCH_INVOKER); } @Override public double defaultTermwiseLimit() { return defaultTermwiseLimit; } @@ -305,6 +307,7 @@ public class ModelContextImpl implements ModelContext { @Override public boolean inhibitDefaultMergesWhenGlobalMergesPending() { return inhibitDefaultMergesWhenGlobalMergesPending; } @Override public boolean useQrserverServiceName() { return useQrserverServiceName; } @Override public boolean avoidRenamingSummaryFeatures() { return avoidRenamingSummaryFeatures; } + @Override public boolean mergeGroupingResultInSearchInvoker() { return mergeGroupingResultInSearchInvoker; } private static <V> V flagValue(FlagSource source, ApplicationId appId, UnboundFlag<? extends V, ?, ?> flag) { return flag.bindTo(source) diff --git a/container-search/src/main/java/com/yahoo/search/dispatch/GroupingResultAggregator.java b/container-search/src/main/java/com/yahoo/search/dispatch/GroupingResultAggregator.java new file mode 100644 index 00000000000..5ce7accfdd4 --- /dev/null +++ b/container-search/src/main/java/com/yahoo/search/dispatch/GroupingResultAggregator.java @@ -0,0 +1,50 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.search.dispatch; + +import com.yahoo.prelude.fastsearch.DocsumDefinitionSet; +import com.yahoo.prelude.fastsearch.GroupingListHit; +import com.yahoo.searchlib.aggregation.Grouping; + +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Incrementally merges underlying {@link Grouping} instances from {@link GroupingListHit} hits. + * + * @author bjorncs + */ +class GroupingResultAggregator { + private static final Logger log = Logger.getLogger(GroupingResultAggregator.class.getName()); + + private final Map<Integer, Grouping> groupings = new LinkedHashMap<>(); + private DocsumDefinitionSet documentDefinitions = null; + private int groupingHitsMerged = 0; + + void mergeWith(GroupingListHit result) { + if (groupingHitsMerged == 0) documentDefinitions = result.getDocsumDefinitionSet(); + ++groupingHitsMerged; + log.log(Level.FINE, () -> + String.format("Merging hit #%d having %d groupings", + groupingHitsMerged, result.getGroupingList().size())); + for (Grouping grouping : result.getGroupingList()) { + groupings.merge(grouping.getId(), grouping, (existingGrouping, newGrouping) -> { + existingGrouping.merge(newGrouping); + return existingGrouping; + }); + } + } + + Optional<GroupingListHit> toAggregatedHit() { + if (groupingHitsMerged == 0) return Optional.empty(); + log.log(Level.FINE, () -> + String.format("Creating aggregated hit containing %d groupings from %d hits", + groupings.size(), groupingHitsMerged)); + groupings.values().forEach(Grouping::postMerge); + return Optional.of(new GroupingListHit(List.copyOf(groupings.values()), documentDefinitions)); + } + +} 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 4e658122cdf..d7c9f1dce53 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 @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.search.dispatch; +import com.yahoo.prelude.fastsearch.GroupingListHit; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.searchcluster.Group; @@ -44,6 +45,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM private final Group group; private final LinkedBlockingQueue<SearchInvoker> availableForProcessing; private final Set<Integer> alreadyFailedNodes; + private final boolean mergeGroupingResult; private Query query; private boolean adaptiveTimeoutCalculated = false; @@ -71,6 +73,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM this.group = group; this.availableForProcessing = newQueue(); this.alreadyFailedNodes = alreadyFailedNodes; + this.mergeGroupingResult = searchCluster.dispatchConfig().mergeGroupingResultInSearchInvokerEnabled(); } /** @@ -115,6 +118,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM long nextTimeout = query.getTimeLeft(); boolean extraDebug = (query.getOffset() == 0) && (query.getHits() == 7) && log.isLoggable(java.util.logging.Level.FINE); List<InvokerResult> processed = new ArrayList<>(); + var groupingResultAggregator = new GroupingResultAggregator(); try { while (!invokers.isEmpty() && nextTimeout >= 0) { SearchInvoker invoker = availableForProcessing.poll(nextTimeout, TimeUnit.MILLISECONDS); @@ -126,7 +130,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM if (extraDebug) { processed.add(toMerge); } - merged = mergeResult(result.getResult(), toMerge, merged); + merged = mergeResult(result.getResult(), toMerge, merged, groupingResultAggregator); ejectInvoker(invoker); } nextTimeout = nextTimeout(); @@ -134,6 +138,7 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM } catch (InterruptedException e) { throw new RuntimeException("Interrupted while waiting for search results", e); } + groupingResultAggregator.toAggregatedHit().ifPresent(h -> result.getResult().hits().add(h)); insertNetworkErrors(result.getResult()); result.getResult().setCoverage(createCoverage()); @@ -238,14 +243,20 @@ public class InterleavedSearchInvoker extends SearchInvoker implements ResponseM return nextAdaptive; } - private List<LeanHit> mergeResult(Result result, InvokerResult partialResult, List<LeanHit> current) { + private List<LeanHit> mergeResult(Result result, InvokerResult partialResult, List<LeanHit> current, + GroupingResultAggregator groupingResultAggregator) { collectCoverage(partialResult.getResult().getCoverage(true)); result.mergeWith(partialResult.getResult()); List<Hit> partialNonLean = partialResult.getResult().hits().asUnorderedHits(); for(Hit hit : partialNonLean) { if (hit.isAuxiliary()) { - result.hits().add(hit); + if (hit instanceof GroupingListHit && mergeGroupingResult) { + var groupingHit = (GroupingListHit) hit; + groupingResultAggregator.mergeWith(groupingHit); + } else { + result.hits().add(hit); + } } } if (current.isEmpty() ) { 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 6e08b1c6fa5..347276d680d 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 @@ -4,6 +4,7 @@ package com.yahoo.search.dispatch; import com.yahoo.document.GlobalId; import com.yahoo.document.idstring.IdString; import com.yahoo.prelude.fastsearch.FastHit; +import com.yahoo.prelude.fastsearch.GroupingListHit; import com.yahoo.search.Query; import com.yahoo.search.Result; import com.yahoo.search.dispatch.searchcluster.Group; @@ -13,6 +14,11 @@ import com.yahoo.search.result.Coverage; import com.yahoo.search.result.ErrorMessage; import com.yahoo.search.result.Hit; import com.yahoo.search.result.Relevance; +import com.yahoo.searchlib.aggregation.Grouping; +import com.yahoo.searchlib.aggregation.MaxAggregationResult; +import com.yahoo.searchlib.aggregation.MinAggregationResult; +import com.yahoo.searchlib.expression.IntegerResultNode; +import com.yahoo.searchlib.expression.StringResultNode; import com.yahoo.test.ManualClock; import org.junit.Test; @@ -320,6 +326,39 @@ public class InterleavedSearchInvokerTest { assertEquals(3, result.getQuery().getHits()); } + @Test + public void requireThatGroupingsAreMerged() throws IOException { + SearchCluster cluster = new MockSearchCluster("!", 1, 2); + List<SearchInvoker> invokers = new ArrayList<>(); + + Grouping grouping1 = new Grouping(0); + grouping1.setRoot(new com.yahoo.searchlib.aggregation.Group() + .addChild(new com.yahoo.searchlib.aggregation.Group() + .setId(new StringResultNode("uniqueA")) + .addAggregationResult(new MaxAggregationResult().setMax(new IntegerResultNode(6)).setTag(4))) + .addChild(new com.yahoo.searchlib.aggregation.Group() + .setId(new StringResultNode("common")) + .addAggregationResult(new MaxAggregationResult().setMax(new IntegerResultNode(9)).setTag(4)))); + invokers.add(new MockInvoker(0).setHits(List.of(new GroupingListHit(List.of(grouping1))))); + + Grouping grouping2 = new Grouping(0); + grouping2.setRoot(new com.yahoo.searchlib.aggregation.Group() + .addChild(new com.yahoo.searchlib.aggregation.Group() + .setId(new StringResultNode("uniqueB")) + .addAggregationResult(new MaxAggregationResult().setMax(new IntegerResultNode(9)).setTag(4))) + .addChild(new com.yahoo.searchlib.aggregation.Group() + .setId(new StringResultNode("common")) + .addAggregationResult(new MinAggregationResult().setMin(new IntegerResultNode(6)).setTag(3)))); + invokers.add(new MockInvoker(0).setHits(List.of(new GroupingListHit(List.of(grouping2))))); + + InterleavedSearchInvoker invoker = new InterleavedSearchInvoker(invokers, cluster, new Group(0, List.of()), Collections.emptySet()); + invoker.responseAvailable(invokers.get(0)); + invoker.responseAvailable(invokers.get(1)); + Result result = invoker.search(query, null); + assertEquals(1, ((GroupingListHit) result.hits().get(0)).getGroupingList().size()); + + } + private static InterleavedSearchInvoker createInterLeavedTestInvoker(List<Double> a, List<Double> b, Group group) { SearchCluster cluster = new MockSearchCluster("!", 1, 2); List<SearchInvoker> invokers = new ArrayList<>(); 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 2acce0f8d2d..54c8c1e0522 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 @@ -120,7 +120,8 @@ public class MockSearchCluster extends SearchCluster { builder.minActivedocsPercentage(88.0); builder.minGroupCoverage(99.0); builder.minSearchCoverage(minSearchCoverage); - builder.distributionPolicy(DispatchConfig.DistributionPolicy.Enum.ROUNDROBIN); + builder.distributionPolicy(DispatchConfig.DistributionPolicy.Enum.ROUNDROBIN) + .mergeGroupingResultInSearchInvokerEnabled(true); if (minSearchCoverage < 100.0) { builder.minWaitAfterCoverageFactor(0); builder.maxWaitAfterCoverageFactor(0.5); diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java index 881b62d1e04..5376fa983af 100644 --- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java +++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java @@ -390,6 +390,13 @@ public class Flags { "Takes effect immediately", TENANT_ID); + public static final UnboundBooleanFlag MERGE_GROUPING_RESULT_IN_SEARCH_INVOKER = defineFeatureFlag( + "merge-grouping-result-in-search-invoker", false, + List.of("bjorncs", "baldersheim"), "2022-02-23", "2022-08-01", + "Merge grouping results incrementally in interleaved search invoker", + "Takes effect at redeployment", + APPLICATION_ID); + /** WARNING: public for testing: All flags should be defined in {@link Flags}. */ public static UnboundBooleanFlag defineFeatureFlag(String flagId, boolean defaultValue, List<String> owners, String createdAt, String expiresAt, String description, diff --git a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Grouping.java b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Grouping.java index 25b3cb18ff9..c88a567c559 100644 --- a/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Grouping.java +++ b/searchlib/src/main/java/com/yahoo/searchlib/aggregation/Grouping.java @@ -46,6 +46,8 @@ public class Grouping extends Identifiable { // Actual root group, does not require level details. private Group root = new Group(); + private boolean postMergeCompleted = false; + /** * <p>Constructs an empty result node. <b>NOTE:</b> This instance is broken until non-optional member data is * set.</p> @@ -78,7 +80,9 @@ public class Grouping extends Identifiable { * that might have changes due to the merge.</p> */ public void postMerge() { + if (postMergeCompleted) return; root.postMerge(groupingLevels, firstLevel, 0); + postMergeCompleted = true; } /** |