Fail queries with scroll that explicitely set request_cache #27342
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
Queries that create a scroll context cannot use the cache. They modify the search context during their execution so using the cache can lead to duplicate result for the next scroll query. This change fails the entire request if the request_cache option is explictely set on a query that creates a scroll context (`scroll=1m`) and make sure internally that we never use the cache for these queries when the option is not explicitely used. For 6.x a deprecation log will be printed instead of failing the entire request and the request_cache hint will be ignored (forced to false).
martijnvg
approved these changes
Nov 10, 2017
| @@ -169,6 +169,10 @@ public ActionRequestValidationException validate() { | |||
| validationException = | |||
| addValidationError("using [from] is not allowed in a scroll context", validationException); | |||
| } | |||
| if (requestCache != null && requestCache && scroll() != null) { | |||
There was a problem hiding this comment.
So this will not be backported to 6.x and instead we will print a deprecation log?
|
Should this be backported for 6.0.1 once 6.0 is out? |
jimczi
added a commit
that referenced
this pull request
Nov 10, 2017
jimczi
added a commit
that referenced
this pull request
Nov 13, 2017
Queries that create a scroll context cannot use the cache. They modify the search context during their execution so using the cache can lead to duplicate result for the next scroll query. This change fails the entire request if the request_cache option is explictely set on a query that creates a scroll context (`scroll=1m`) and make sure internally that we never use the cache for these queries when the option is not explicitely used. For 6.x a deprecation log will be printed instead of failing the entire request and the request_cache hint will be ignored (forced to false).
jimczi
added a commit
that referenced
this pull request
Nov 13, 2017
martijnvg
added a commit
that referenced
this pull request
Nov 14, 2017
* es/master: (24 commits) Reduce synchronization on field data cache add json-processor support for non-map json types (#27335) Properly format IndexGraveyard deletion date as date (#27362) Upgrade AWS SDK Jackson Databind to 2.6.7.1 Stop responding to ping requests before master abdication (#27329) [Test] Fix POI version in packaging tests Allow affix settings to specify dependencies (#27161) Tests: Improve size regex in documentation test (#26879) reword comment Remove unnecessary logger creation for doc values field data [Geo] Decouple geojson parse logic from ShapeBuilders [DOCS] Fixed link to docker content Plugins: Add versionless alias to all security policy codebase properties (#26756) [Test] #27342 Fix SearchRequests#testValidate [DOCS] Move X-Pack-specific Docker content (#27333) Fail queries with scroll that explicitely set request_cache (#27342) [Test] Fix S3BlobStoreContainerTests.testNumberOfMultiparts() Set minimum_master_nodes to all nodes for REST tests (#27344) [Tests] Relax allowed delta in extended_stats aggregation (#27171) Remove S3 output stream (#27280) ...
martijnvg
added a commit
that referenced
this pull request
Nov 14, 2017
* 6.x: (27 commits) Reduce synchronization on field data cache [Test] #27342 Fix SearchRequests#testValidate Fail queries with scroll that explicitely set request_cache (#27342) add json-processor support for non-map json types (#27335) Upgrade AWS SDK Jackson Databind to 2.6.7.1 Properly format IndexGraveyard deletion date as date (#27362) Stop responding to ping requests before master abdication (#27329) [Test] Fix POI version in packaging tests Added release notes for 6.0.0 Update 6.0.0-beta1.asciidoc Allow affix settings to specify dependencies (#27161) Tests: Improve size regex in documentation test (#26879) reword comment Ensure external refreshes will also refresh internal searcher to minimize segment creation (#27253) Remove unnecessary logger creation for doc values field data [DOCS] Fixed link to docker content Plugins: Add versionless alias to all security policy codebase properties (#26756) [DOCS] Move X-Pack-specific Docker content (#27333) [Test] Fix S3BlobStoreContainerTests.testNumberOfMultiparts() Set minimum_master_nodes to all nodes for REST tests (#27344) ...
jimczi
added a commit
that referenced
this pull request
Nov 21, 2017
Queries that create a scroll context cannot use the cache. They modify the search context during their execution so using the cache can lead to duplicate result for the next scroll query. This change fails the entire request if the request_cache option is explictely set on a query that creates a scroll context (`scroll=1m`) and make sure internally that we never use the cache for these queries when the option is not explicitely used. For 6.x a deprecation log will be printed instead of failing the entire request and the request_cache hint will be ignored (forced to false).
jimczi
added a commit
that referenced
this pull request
Nov 21, 2017
jimczi
added a commit
that referenced
this pull request
Nov 21, 2017
clintongormley
added
:Search/Search
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.
Queries that create a scroll context cannot use the cache.
They modify the search context during their execution so using the cache
can lead to duplicate result for the next scroll query.
This change fails the entire request if the request_cache option is explictely set
on a query that creates a scroll context (
scroll=1m) and make sure internally that we neveruse the cache for these queries when the option is not explicitely used.
For 6.x a deprecation log will be printed instead of failing the entire request and the request_cache hint will be ignored (forced to false).