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

Disable max score optimization for queries with unbounded max scores #41361

Merged
merged 4 commits into from
Apr 25, 2019

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Apr 18, 2019

Lucene 8 has the ability to skip blocks of non-competitive documents.
However some queries don't track their maximum score (script_score, span, ...)
so they always return Float.POSITIVE_INFINITY as maximum score. This can slow down
some boolean queries if other clauses have bounded max scores. This commit disables
the max score optimization when we detect a mandatory scoring clause with unbounded
max scores. Optional clauses are not checked since they can still skip documents
when the unbounded clause is after the current document.

Lucene 8 has the ability to skip blocks of non-competitive documents.
However some queries don't track their maximum score (`script_score`, `span`, ...)
so they always return Float.POSITIVE_INFINITY as maximum score. This can slow down
some boolean queries if other clauses have bounded max scores. This commit disables
the max score optimization when we detect a mandatory scoring clause with unbounded
 max scores. Optional clauses are not checked since they can still skip documents
 when the unbounded clause is after the current document.
@jimczi jimczi added >enhancement :Search/Search Search-related issues that do not fall into other categories v8.0.0 v7.2.0 labels Apr 18, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

Copy link
Contributor

@romseygeek romseygeek left a comment

Choose a reason for hiding this comment

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

I spotted one thing that I think needs changing - would also be good to get a test for that case, with a conjunction of a SpanQuery and an ESToParentBlockJoinQuery with ScoreMode.None.

I'm glad to see the QueryVisitor API is useful!

hasInfMaxScore = true;
} else if (query instanceof ESToParentBlockJoinQuery) {
ESToParentBlockJoinQuery q = (ESToParentBlockJoinQuery) query;
hasInfMaxScore = q.getScoreMode() != org.apache.lucene.search.join.ScoreMode.None;
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be |= I think, otherwise if you have a conjunction with both SpanQuery and ESToParentBlockJoinQuery, you might end up un-setting the boolean here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, I pushed 0f4de62

Copy link
Contributor

@romseygeek romseygeek 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
Copy link
Contributor Author

jimczi commented Apr 25, 2019

run elasticsearch-ci/1

@jimczi jimczi merged commit 3567b79 into elastic:master Apr 25, 2019
@jimczi jimczi deleted the unbounded_max_score_query branch April 25, 2019 19:13
@jimczi
Copy link
Contributor Author

jimczi commented Apr 25, 2019

This cannot be backported to 7x yet since this pr relies on code that is only in Lucene 8x (QueryVisitor) and 7x is still on Lucene 8.0

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 26, 2019
…s-in-all-the-places

* elastic/master: (70 commits)
  Remove experimental label froms script_score query (elastic#41572)
  [Ml-Dataframe] Update URLs in Data frame client java doc (elastic#41539)
  Reenable bwc Tests in master (elastic#41540)
  Fix search_as_you_type's sub-fields to pick their names from the full path of the root field (elastic#41541)
  Remove search analyzers from DocumentFieldMappers (elastic#41484)
  Update community client and integration docs (elastic#41513)
  Remove Version.V_6_x_x constants use in security (elastic#41185)
  Mute testDriverConfigurationWithSSLInURL
  Remove dedicated SSL network write buffer (elastic#41283)
  Disable max score optimization for queries with unbounded max scores (elastic#41361)
  [DOCS] Explicitly set section IDs for Asciidoctor migration (elastic#41547)
  [ML] add multi node integ tests for data frames (elastic#41508)
  [Docs] Fix common word repetitions (elastic#39703)
  [DOCS] Note TESTRESPONSE can't be used immediately after TESTSETUP (elastic#41542)
  Update configuring-ldap-realm.asciidoc (elastic#40427)
  Fixed very small typo in date (elastic#41398)
  Refactor GeoHashUtils (elastic#40869)
  Make 0 as invalid value for `min_children` in `has_child` query (elastic#41347)
  field_caps: adapt bwc version after backport (elastic#41427)
  Remove Exists Check from S3 Repository Deletes (elastic#40931)
  ...
akhil10x5 pushed a commit to akhil10x5/elasticsearch that referenced this pull request May 2, 2019
…lastic#41361)

Lucene 8 has the ability to skip blocks of non-competitive documents.
However some queries don't track their maximum score (`script_score`, `span`, ...)
so they always return Float.POSITIVE_INFINITY as maximum score. This can slow down
some boolean queries if other clauses have bounded max scores. This commit disables
the max score optimization when we detect a mandatory scoring clause with unbounded
 max scores. Optional clauses are not checked since they can still skip documents
 when the unbounded clause is after the current document.
@jimczi jimczi added v7.3.0 and removed v7.2.0 labels May 23, 2019
jimczi added a commit that referenced this pull request May 23, 2019
…41361)

Lucene 8 has the ability to skip blocks of non-competitive documents.
However some queries don't track their maximum score (`script_score`, `span`, ...)
so they always return Float.POSITIVE_INFINITY as maximum score. This can slow down
some boolean queries if other clauses have bounded max scores. This commit disables
the max score optimization when we detect a mandatory scoring clause with unbounded
 max scores. Optional clauses are not checked since they can still skip documents
 when the unbounded clause is after the current document.
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
…lastic#41361)

Lucene 8 has the ability to skip blocks of non-competitive documents.
However some queries don't track their maximum score (`script_score`, `span`, ...)
so they always return Float.POSITIVE_INFINITY as maximum score. This can slow down
some boolean queries if other clauses have bounded max scores. This commit disables
the max score optimization when we detect a mandatory scoring clause with unbounded
 max scores. Optional clauses are not checked since they can still skip documents
 when the unbounded clause is after the current document.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search/Search Search-related issues that do not fall into other categories v7.3.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants