Skip to content

Handle total hits equal to track_total_hits #37907

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

Jump to bottom
Merged
merged 4 commits into from
Mar 5, 2019
Merged

Handle total hits equal to track_total_hits #37907

merged 4 commits into from
Mar 5, 2019

Conversation

@jimczi
Copy link
Contributor

@jimczi jimczi commented Jan 28, 2019

This change ensures that a total hits equal to the value set for
track_total_hits is not considered as a lower bound.

Closes #37897

@jimczi
Handle total hits equal to track_total_hits
bdd7078 
This change ensures that a total hits equal to the value set for
track_total_hits is not considered as a lower bound.

Closes elastic#37897
@jimczi jimczi added >bug :Search/Search Search-related issues that do not fall into other categories >test-failure Triaged test failures from CI v7.0.0 labels Jan 28, 2019
@elasticmachine
Copy link
Collaborator

elasticmachine commented Jan 28, 2019

Pinging @elastic/es-search

jpountz
jpountz suggested changes Jan 28, 2019
View reviewed changes
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

We might still return gte in case of exact match if there is a single shard that has matches since Lucene sets the relation as gte as soon as the number of matches is greater than or equal to the threshold: https://github.com/apache/lucene-solr/blob/947f82679afa6d984c246b686d0133085982c376/lucene/core/src/java/org/apache/lucene/search/TopScoreDocCollector.java#L75

So I think we either need to change the behavior in Lucene as well or change expectations on our side?

@jimczi
Copy link
Contributor Author

jimczi commented Jan 28, 2019 •
edited
Loading

We discussed with @jpountz offline and agreed that the behavior in Lucene is not intuitive (return a lower bound when the doc count is equal to the threshold). I opened https://issues.apache.org/jira/browse/LUCENE-8660 to change the heuristic to count accurately up to the threshold instead of threshold-1.

tlrx added a commit that referenced this pull request Jan 28, 2019
@tlrx
Track accurate total hits in CloseIndexIT
758eb9d 
The test was not using the TRACK_TOTAL_HITS_ACCURATE and thus
encountered a different issue tracked in #37907. In the meanwhile
we can adapt the test to not fail anymore.

Closes #37897
@tlrx tlrx mentioned this pull request Jan 28, 2019
Closed
@jasontedor jasontedor added v8.0.0 and removed v7.0.0 labels Feb 6, 2019
jpountz
jpountz approved these changes Feb 15, 2019
View reviewed changes
Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

LGTM

@jimczi jimczi removed the >test-failure Triaged test failures from CI label Mar 5, 2019
@jimczi jimczi merged commit 91d8c4e into elastic:master Mar 5, 2019
@jimczi jimczi deleted the bug/track_total_hits_equals branch March 5, 2019 11:03
jimczi added a commit that referenced this pull request Mar 5, 2019
@jimczi
Handle total hits equal to track_total_hits (#37907)
160dc29 
This change ensures that a total hits equal to the value set for
track_total_hits is not considered as a lower bound.
jimczi added a commit that referenced this pull request Mar 5, 2019
@jimczi
Handle total hits equal to track_total_hits (#37907)
c8aa4ed 
This change ensures that a total hits equal to the value set for
track_total_hits is not considered as a lower bound.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jpountz jpountz jpountz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@jimczi jimczi

Labels

>bug :Search/Search Search-related issues that do not fall into other categories v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

CloseIndexIT#testCloseWhileIndexingDocuments can fail with assertion error

5 participants

@jimczi @elasticmachine @jpountz @jakelandis @jasontedor