Skip to content

Don't count hits via the collector if the hit count can be computed from index stats. #33701

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.

Sign up for GitHub

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

Merged
jpountz merged 2 commits into from
Sep 14, 2018
Merged

Don't count hits via the collector if the hit count can be computed from index stats. #33701

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5948e8c
Don't count hits via the collector if the hit count can be computed f…
jpountz Sep 14, 2018
eb03841
iter
jpountz Sep 14, 2018
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
75 changes: 36 additions & 39 deletions server/src/main/java/org/elasticsearch/search/query/TopDocsCollectorContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,16 @@ void postProcess(QuerySearchResult result) throws IOException {
}

abstract static class SimpleTopDocsCollectorContext extends TopDocsCollectorContext {

private static TopDocsCollector<?> createCollector(@Nullable SortAndFormats sortAndFormats, int numHits,
@Nullable ScoreDoc searchAfter, int hitCountThreshold) {
if (sortAndFormats == null) {
return TopScoreDocCollector.create(numHits, searchAfter, hitCountThreshold);
} else {
return TopFieldCollector.create(sortAndFormats.sort, numHits, (FieldDoc) searchAfter, hitCountThreshold);
}
}

private final @Nullable SortAndFormats sortAndFormats;
private final Collector collector;
private final Supplier<TotalHits> totalHitsSupplier;
Expand Down Expand Up @@ -201,55 +211,42 @@ private SimpleTopDocsCollectorContext(IndexReader reader,
boolean hasFilterCollector) throws IOException {
super(REASON_SEARCH_TOP_HITS, numHits);
this.sortAndFormats = sortAndFormats;

// implicit total hit counts are valid only when there is no filter collector in the chain
final int hitCount = hasFilterCollector ? -1 : shortcutTotalHitCount(reader, query);
final TopDocsCollector<?> topDocsCollector;
if (hitCount == -1 && trackTotalHits) {
topDocsCollector = createCollector(sortAndFormats, numHits, searchAfter, Integer.MAX_VALUE);
topDocsSupplier = new CachedSupplier<>(topDocsCollector::topDocs);
totalHitsSupplier = () -> topDocsSupplier.get().totalHits;
} else {
topDocsCollector = createCollector(sortAndFormats, numHits, searchAfter, 1); // don't compute hit counts via the collector
topDocsSupplier = new CachedSupplier<>(topDocsCollector::topDocs);
if (hitCount == -1) {
assert trackTotalHits == false;
totalHitsSupplier = () -> new TotalHits(topDocsSupplier.get().scoreDocs.length,
Copy link
Contributor

@jimczi jimczi Sep 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's return new TotalHits(0, TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO) for now ? It is translated to -1 by the coordinating node anyway since track_total_hits is false in this case. We also need to handle bwc for mixed search with 6x node. Currently we have an exception if the hit count is greater than 0 and not an exact count so I suspect that this change could break some bwc tests randomly. We can revise this when we add the support for track_total_hits as a number ?

Copy link
Contributor Author

@jpountz jpountz Sep 14, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Will do.

TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO);
} else {
totalHitsSupplier = () -> new TotalHits(hitCount, TotalHits.Relation.EQUAL_TO);
}
}
MaxScoreCollector maxScoreCollector = null;
if (sortAndFormats == null) {
final TopDocsCollector<?> topDocsCollector = TopScoreDocCollector.create(numHits, searchAfter, Integer.MAX_VALUE);
this.collector = topDocsCollector;
this.topDocsSupplier = new CachedSupplier<>(topDocsCollector::topDocs);
this.totalHitsSupplier = () -> topDocsSupplier.get().totalHits;
this.maxScoreSupplier = () -> {
maxScoreSupplier = () -> {
TopDocs topDocs = topDocsSupplier.get();
if (topDocs.scoreDocs.length == 0) {
return Float.NaN;
} else {
return topDocs.scoreDocs[0].score;
}
};
} else if (trackMaxScore) {
maxScoreCollector = new MaxScoreCollector();
maxScoreSupplier = maxScoreCollector::getMaxScore;
} else {
/**
* We explicitly don't track total hits in the topdocs collector, it can early terminate
* if the sort matches the index sort.
*/
final TopDocsCollector<?> topDocsCollector = TopFieldCollector.create(sortAndFormats.sort, numHits,
(FieldDoc) searchAfter, 1);
this.topDocsSupplier = new CachedSupplier<>(topDocsCollector::topDocs);
TotalHitCountCollector hitCountCollector = null;
if (trackTotalHits) {
// implicit total hit counts are valid only when there is no filter collector in the chain
int count = hasFilterCollector ? -1 : shortcutTotalHitCount(reader, query);
if (count != -1) {
// we can extract the total count from the shard statistics directly
this.totalHitsSupplier = () -> new TotalHits(count, TotalHits.Relation.EQUAL_TO);
} else {
// wrap a collector that counts the total number of hits even
// if the top docs collector terminates early
final TotalHitCountCollector countingCollector = new TotalHitCountCollector();
hitCountCollector = countingCollector;
this.totalHitsSupplier = () -> new TotalHits(countingCollector.getTotalHits(), TotalHits.Relation.EQUAL_TO);
}
} else {
// total hit count is not needed
// for bwc hit count is set to 0, it will be converted to -1 by the coordinating node
this.totalHitsSupplier = () -> new TotalHits(0, TotalHits.Relation.GREATER_THAN_OR_EQUAL_TO);
}
MaxScoreCollector maxScoreCollector = null;
if (trackMaxScore) {
maxScoreCollector = new MaxScoreCollector();
maxScoreSupplier = maxScoreCollector::getMaxScore;
} else {
maxScoreSupplier = () -> Float.NaN;
}
collector = MultiCollector.wrap(topDocsCollector, hitCountCollector, maxScoreCollector);
maxScoreSupplier = () -> Float.NaN;
}
this.collector = MultiCollector.wrap(topDocsCollector, maxScoreCollector);
}

@Override
Expand Down