Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize innerhits query performance #16937

Merged
merged 1 commit into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Allow extended plugins to be optional ([#16909](https://github.com/opensearch-project/OpenSearch/pull/16909))
- Use the correct type to widen the sort fields when merging top docs ([#16881](https://github.com/opensearch-project/OpenSearch/pull/16881))
- Limit reader writer separation to remote store enabled clusters [#16760](https://github.com/opensearch-project/OpenSearch/pull/16760)
- Optimize innerhits query performance [#16937](https://github.com/opensearch-project/OpenSearch/pull/16937)

### Deprecated
- Performing update operation with default pipeline or final pipeline is deprecated ([#16712](https://github.com/opensearch-project/OpenSearch/pull/16712))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@
import org.apache.lucene.document.StoredField;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.ScoreMode;
import org.apache.lucene.search.Scorer;
import org.apache.lucene.search.Weight;
import org.apache.lucene.util.BitSet;
import org.apache.lucene.util.BytesRef;
import org.opensearch.OpenSearchGenerationException;
import org.opensearch.common.annotation.PublicApi;
Expand All @@ -53,6 +51,7 @@
import org.opensearch.index.analysis.IndexAnalyzers;
import org.opensearch.index.mapper.MapperService.MergeReason;
import org.opensearch.index.mapper.MetadataFieldMapper.TypeParser;
import org.opensearch.index.query.NestedQueryBuilder;
import org.opensearch.search.internal.SearchContext;

import java.io.IOException;
Expand Down Expand Up @@ -270,25 +269,15 @@
* Returns the best nested {@link ObjectMapper} instances that is in the scope of the specified nested docId.
*/
public ObjectMapper findNestedObjectMapper(int nestedDocId, SearchContext sc, LeafReaderContext context) throws IOException {
if (sc instanceof NestedQueryBuilder.NestedInnerHitSubContext) {
ObjectMapper objectMapper = ((NestedQueryBuilder.NestedInnerHitSubContext) sc).getChildObjectMapper();

Check warning on line 273 in server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java#L273

Added line #L273 was not covered by tests
assert objectMappers().containsKey(objectMapper.fullPath());
assert containSubDocIdWithObjectMapper(nestedDocId, objectMapper, sc, context);
return objectMapper;

Check warning on line 276 in server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java#L276

Added line #L276 was not covered by tests
}
ObjectMapper nestedObjectMapper = null;
for (ObjectMapper objectMapper : objectMappers().values()) {
if (!objectMapper.nested().isNested()) {
continue;
}

Query filter = objectMapper.nestedTypeFilter();
if (filter == null) {
continue;
}
// We can pass down 'null' as acceptedDocs, because nestedDocId is a doc to be fetched and
// therefore is guaranteed to be a live doc.
final Weight nestedWeight = filter.createWeight(sc.searcher(), ScoreMode.COMPLETE_NO_SCORES, 1f);
Scorer scorer = nestedWeight.scorer(context);
if (scorer == null) {
continue;
}

if (scorer.iterator().advance(nestedDocId) == nestedDocId) {
if (containSubDocIdWithObjectMapper(nestedDocId, objectMapper, sc, context)) {
if (nestedObjectMapper == null) {
nestedObjectMapper = objectMapper;
} else {
Expand All @@ -301,6 +290,25 @@
return nestedObjectMapper;
}

private boolean containSubDocIdWithObjectMapper(int nestedDocId, ObjectMapper objectMapper, SearchContext sc, LeafReaderContext context)
throws IOException {
if (!objectMapper.nested().isNested()) {
return false;

Check warning on line 296 in server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java#L296

Added line #L296 was not covered by tests
}
Query filter = objectMapper.nestedTypeFilter();

Check warning on line 298 in server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java#L298

Added line #L298 was not covered by tests
if (filter == null) {
return false;

Check warning on line 300 in server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java#L300

Added line #L300 was not covered by tests
}
// We can pass down 'null' as acceptedDocs, because nestedDocId is a doc to be fetched and
// therefore is guaranteed to be a live doc.
BitSet nestedDocIds = sc.bitsetFilterCache().getBitSetProducer(filter).getBitSet(context);

Check warning on line 304 in server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java#L304

Added line #L304 was not covered by tests
if (nestedDocIds != null && nestedDocIds.get(nestedDocId)) {
return true;

Check warning on line 306 in server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java#L306

Added line #L306 was not covered by tests
} else {
return false;

Check warning on line 308 in server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/mapper/DocumentMapper.java#L308

Added line #L308 was not covered by tests
}
}

public DocumentMapper merge(Mapping mapping, MergeReason reason) {
Mapping merged = this.mapping.merge(mapping, reason);
return new DocumentMapper(mapperService, merged);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -438,7 +438,7 @@
*
* @opensearch.internal
*/
static final class NestedInnerHitSubContext extends InnerHitsContext.InnerHitSubContext {
public static final class NestedInnerHitSubContext extends InnerHitsContext.InnerHitSubContext {

private final ObjectMapper parentObjectMapper;
private final ObjectMapper childObjectMapper;
Expand Down Expand Up @@ -507,6 +507,10 @@
return new TopDocsAndMaxScore(td, maxScore);
}
}

public ObjectMapper getChildObjectMapper() {
return childObjectMapper;

Check warning on line 512 in server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/index/query/NestedQueryBuilder.java#L512

Added line #L512 was not covered by tests
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,7 @@
import org.apache.lucene.index.ReaderUtil;
import org.apache.lucene.search.DocIdSetIterator;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.ScoreMode;
import org.apache.lucene.search.Scorer;
import org.apache.lucene.search.TotalHits;
import org.apache.lucene.search.Weight;
import org.apache.lucene.util.BitSet;
import org.opensearch.common.CheckedBiConsumer;
import org.opensearch.common.annotation.PublicApi;
Expand All @@ -55,7 +52,6 @@
import org.opensearch.core.common.text.Text;
import org.opensearch.core.tasks.TaskCancelledException;
import org.opensearch.core.xcontent.MediaType;
import org.opensearch.index.IndexSettings;
import org.opensearch.index.fieldvisitor.CustomFieldsVisitor;
import org.opensearch.index.fieldvisitor.FieldsVisitor;
import org.opensearch.index.mapper.DocumentMapper;
Expand Down Expand Up @@ -501,7 +497,6 @@
ObjectMapper current = nestedObjectMapper;
String originalName = nestedObjectMapper.name();
SearchHit.NestedIdentity nestedIdentity = null;
final IndexSettings indexSettings = context.getQueryShardContext().getIndexSettings();
do {
Query parentFilter;
nestedParentObjectMapper = current.getParentObjectMapper(mapperService);
Expand All @@ -520,14 +515,13 @@
current = nestedParentObjectMapper;
continue;
}
final Weight childWeight = context.searcher()
.createWeight(context.searcher().rewrite(childFilter), ScoreMode.COMPLETE_NO_SCORES, 1f);
Scorer childScorer = childWeight.scorer(subReaderContext);
if (childScorer == null) {
BitSet childIter = context.bitsetFilterCache()
.getBitSetProducer(context.searcher().rewrite(childFilter))
.getBitSet(subReaderContext);

Check warning on line 520 in server/src/main/java/org/opensearch/search/fetch/FetchPhase.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/fetch/FetchPhase.java#L518-L520

Added lines #L518 - L520 were not covered by tests
if (childIter == null) {
current = nestedParentObjectMapper;
continue;
}
DocIdSetIterator childIter = childScorer.iterator();

BitSet parentBits = context.bitsetFilterCache().getBitSetProducer(parentFilter).getBitSet(subReaderContext);

Expand All @@ -541,8 +535,8 @@
* that appear before him.
*/
int previousParent = parentBits.prevSetBit(currentParent);
for (int docId = childIter.advance(previousParent + 1); docId < nestedSubDocId
&& docId != DocIdSetIterator.NO_MORE_DOCS; docId = childIter.nextDoc()) {
for (int docId = childIter.nextSetBit(previousParent + 1); docId < nestedSubDocId

Check warning on line 538 in server/src/main/java/org/opensearch/search/fetch/FetchPhase.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/org/opensearch/search/fetch/FetchPhase.java#L538

Added line #L538 was not covered by tests
&& docId != DocIdSetIterator.NO_MORE_DOCS; docId = childIter.nextSetBit(docId + 1)) {
offset++;
}
currentParent = nestedSubDocId;
Expand Down
Loading