Forbid expensive query parts in ranking evaluation #30151
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cbuescher
added
review
:Search Relevance/Ranking
|
Pinging @elastic/es-search-aggs |
Currently the ranking evaluation API accepts the full query syntax for the queries specified in the evaluation set and executes them via multi search. This potentially runs costly aggregations and suggestions too. This change adds checks that forbid using aggregations, suggesters or highlighters in the queries that are run as part of the ranking evaluation since they are irrelevent in the context of this API. Closes elastic#29674
cbuescher
force-pushed
the
rankEval-forbidQueryElements
branch
from
68ed7f5 to
6084785
Compare
|
@elasticmachine run sample packaging tests |
polyfractal
reviewed
May 9, 2018
| this(id, ratedDocs, testRequest, new HashMap<>(), null); | ||
| static void validateEvaluatedQuery(SearchSourceBuilder evaluationRequest) { | ||
| // ensure that testRequest, if set, does not contain aggregation, suggest or highlighting section | ||
| if (evaluationRequest != null) { |
There was a problem hiding this comment.
I wonder if profile and explain should be forbidden too? Both have non-negligible impact on performance, and seem irrelevant to ranking as well.
There was a problem hiding this comment.
Good point, I think I will add that before I merge. Thanks.
|
Left a minor comment but otherwise LGTM :) |
cbuescher
pushed a commit
that referenced
this pull request
May 14, 2018
Currently the ranking evaluation API accepts the full query syntax for the queries specified in the evaluation set and executes them via multi search. This potentially runs costly aggregations and suggestions too. This change adds checks that forbid using aggregations, suggesters, highlighters and the explain and profile options in the queries that are run as part of the ranking evaluation since they are irrelevent in the context of this API.
dnhatn
added a commit
that referenced
this pull request
May 14, 2018
* master: Default to one shard (#30539) Unmute IndexUpgradeIT tests Forbid expensive query parts in ranking evaluation (#30151) Docs: Update HighLevelRestClient migration docs (#30544) Clients: Switch to new performRequest (#30543) [TEST] Fix typo in MovAvgIT test Add missing dependencies on testClasses (#30527) [TEST] Mute ML test that needs updating to following ml-cpp changes Document woes between auto-expand-replicas and allocation filtering (#30531) Moved tokenizers to analysis common module (#30538) Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (#30530) Deprecate not copy settings and explicitly disallow (#30404) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (#30535) SQL: Use request flavored methods in tests (#30345) Suppress hdfsFixture if there are spaces in the path (#30302) Delete temporary blobs before creating index file (#30528) Watcher: Remove TriggerEngine.getJobCount() (#30395) [ML] Fix wire BWC for JobUpdate (#30512) Use simpler write-once semantics for FS repository (#30435) Derive max composite buffers from max content len Use simpler write-once semantics for HDFS repository (#30439) SQL: Improve correctness of SYS COLUMNS & TYPES (#30418) Mute two tests in FlushIT with @AwaitsFix. Fix incorrect template name in test case Build: Remove legacy bwc files from xpack (#30485) Mute UnicastZenPingTests#testSimplePings with @AwaitsFix. Security: cleanup code in file stores (#30348) Security: fix TokenMetaData equals and hashcode (#30347) Mute two tests from SmokeTestWatcherWithSecurityClientYamlTestSuiteIT. Mute SharedClusterSnapshotRestoreIT#testSnapshotSucceedsAfterSnapshotFailure with @AwaitsFix. SQL: Improve compatibility with MS query (#30516) SQL: Fix parsing of dates with milliseconds (#30419)
dnhatn
added a commit
that referenced
this pull request
May 14, 2018
* 6.x: Unmute IndexUpgradeIT tests Forbid expensive query parts in ranking evaluation (#30151) Docs: Update HighLevelRestClient migration docs (#30544) Clients: Switch to new performRequest (#30543) [TEST] Fix typo in MovAvgIT test [TEST] Mute ML test that needs updating to following ml-cpp changes Moved tokenizers to analysis common module (#30538) Document woes between auto-expand-replicas and allocation filtering (#30531) [ML] Hide internal Job update options from the REST API (#30537) Deprecate not copy settings and explicitly disallow (#30404) Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (#30530) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (#30535) Bump Gradle heap to 1792m (#30484) SQL: Use request flavored methods in tests (#30345) Suppress hdfsFixture if there are spaces in the path (#30302) Delete temporary blobs before creating index file (#30528) Watcher: Remove TriggerEngine.getJobCount() (#30395) Use simpler write-once semantics for FS repository (#30435) Use simpler write-once semantics for HDFS repository (#30439) SQL: Improve correctness of SYS COLUMNS & TYPES (#30418) Mute two tests in FlushIT with @AwaitsFix. Fix incorrect template name in test case Build: Remove legacy bwc files from xpack (#30485) Security: Simplify security index listeners (#30466) Mute SharedClusterSnapshotRestoreIT#testSnapshotSucceedsAfterSnapshotFailure with @AwaitsFix. Add proper longitude validation in geo_polygon_query (#30497) Mute UnicastZenPingTests#testSimplePings with @AwaitsFix. Security: cleanup code in file stores (#30348) Security: fix TokenMetaData equals and hashcode (#30347) Mute two tests from SmokeTestWatcherWithSecurityClientYamlTestSuiteIT. Fix incorrect merged entry in changelog SQL: Improve compatibility with MS query (#30516) SQL: Fix parsing of dates with milliseconds (#30419)
martijnvg
added a commit
that referenced
this pull request
May 15, 2018
* es/ccr: (37 commits) Default to one shard (#30539) Unmute IndexUpgradeIT tests Forbid expensive query parts in ranking evaluation (#30151) Docs: Update HighLevelRestClient migration docs (#30544) Clients: Switch to new performRequest (#30543) [TEST] Fix typo in MovAvgIT test Add missing dependencies on testClasses (#30527) [TEST] Mute ML test that needs updating to following ml-cpp changes Document woes between auto-expand-replicas and allocation filtering (#30531) Moved tokenizers to analysis common module (#30538) Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (#30530) Deprecate not copy settings and explicitly disallow (#30404) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (#30535) SQL: Use request flavored methods in tests (#30345) Suppress hdfsFixture if there are spaces in the path (#30302) ...
martijnvg
added a commit
that referenced
this pull request
May 15, 2018
* es/ccr: (37 commits) Default to one shard (#30539) Unmute IndexUpgradeIT tests Forbid expensive query parts in ranking evaluation (#30151) Docs: Update HighLevelRestClient migration docs (#30544) Clients: Switch to new performRequest (#30543) [TEST] Fix typo in MovAvgIT test Add missing dependencies on testClasses (#30527) [TEST] Mute ML test that needs updating to following ml-cpp changes Document woes between auto-expand-replicas and allocation filtering (#30531) Moved tokenizers to analysis common module (#30538) Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (#30530) Deprecate not copy settings and explicitly disallow (#30404) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (#30535) SQL: Use request flavored methods in tests (#30345) Suppress hdfsFixture if there are spaces in the path (#30302) ...
martijnvg
added a commit
to martijnvg/elasticsearch
that referenced
this pull request
May 15, 2018
* es/ccr: (37 commits) Default to one shard (elastic#30539) Unmute IndexUpgradeIT tests Forbid expensive query parts in ranking evaluation (elastic#30151) Docs: Update HighLevelRestClient migration docs (elastic#30544) Clients: Switch to new performRequest (elastic#30543) [TEST] Fix typo in MovAvgIT test Add missing dependencies on testClasses (elastic#30527) [TEST] Mute ML test that needs updating to following ml-cpp changes Document woes between auto-expand-replicas and allocation filtering (elastic#30531) Moved tokenizers to analysis common module (elastic#30538) Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (elastic#30530) Deprecate not copy settings and explicitly disallow (elastic#30404) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (elastic#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (elastic#30535) SQL: Use request flavored methods in tests (elastic#30345) Suppress hdfsFixture if there are spaces in the path (elastic#30302) ...
martijnvg
added a commit
to martijnvg/elasticsearch
that referenced
this pull request
May 15, 2018
* es/ccr: (37 commits) Default to one shard (elastic#30539) Unmute IndexUpgradeIT tests Forbid expensive query parts in ranking evaluation (elastic#30151) Docs: Update HighLevelRestClient migration docs (elastic#30544) Clients: Switch to new performRequest (elastic#30543) [TEST] Fix typo in MovAvgIT test Add missing dependencies on testClasses (elastic#30527) [TEST] Mute ML test that needs updating to following ml-cpp changes Document woes between auto-expand-replicas and allocation filtering (elastic#30531) Moved tokenizers to analysis common module (elastic#30538) Adjust copy settings versions Mute ShrinkIndexIT suite SQL: SYS TABLES ordered according to *DBC specs (elastic#30530) Deprecate not copy settings and explicitly disallow (elastic#30404) [ML] Improve state persistence log message Build: Add mavenPlugin cluster configuration method (elastic#30541) Re-enable FlushIT tests Bump Gradle heap to 2 GB (elastic#30535) SQL: Use request flavored methods in tests (elastic#30345) Suppress hdfsFixture if there are spaces in the path (elastic#30302) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently the ranking evaluation API accepts the full query syntax for
the queries specified in the evaluation set and executes them via multi
search. This potentially runs costly aggregations and suggestions too.
This change adds checks that forbid using aggregations, suggesters or
highlighters in the queries that are run as part of the ranking
evaluation since they are irrelevent in the context of this API.
Closes #29674