Fail queries with scroll that explicitely set request_cache (#27342)

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).
This commit is contained in:
Jim Ferenczi 2017-11-10 16:02:06 +01:00 committed by GitHub
parent f6c2ea0f7d
commit 29331f1127
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 35 additions and 0 deletions

View File

@ -169,6 +169,10 @@ public final class SearchRequest extends ActionRequest implements IndicesRequest
validationException = validationException =
addValidationError("using [from] is not allowed in a scroll context", validationException); addValidationError("using [from] is not allowed in a scroll context", validationException);
} }
if (requestCache != null && requestCache && scroll() != null) {
validationException =
addValidationError("[request_cache] cannot be used in a a scroll context", validationException);
}
return validationException; return validationException;
} }

View File

@ -1072,6 +1072,12 @@ public class IndicesService extends AbstractLifecycleComponent
* Can the shard request be cached at all? * Can the shard request be cached at all?
*/ */
public boolean canCache(ShardSearchRequest request, SearchContext context) { public boolean canCache(ShardSearchRequest request, SearchContext context) {
// Queries that create a scroll context cannot use the cache.
// They modify the search context during their execution so using the cache
// may invalidate the scroll for the next query.
if (request.scroll() != null) {
return false;
}
// We cannot cache with DFS because results depend not only on the content of the index but also // We cannot cache with DFS because results depend not only on the content of the index but also
// on the overridden statistics. So if you ran two queries on the same index with different stats // on the overridden statistics. So if you ran two queries on the same index with different stats
@ -1080,6 +1086,7 @@ public class IndicesService extends AbstractLifecycleComponent
if (SearchType.QUERY_THEN_FETCH != context.searchType()) { if (SearchType.QUERY_THEN_FETCH != context.searchType()) {
return false; return false;
} }
IndexSettings settings = context.indexShard().indexSettings(); IndexSettings settings = context.indexShard().indexSettings();
// if not explicitly set in the request, use the index setting, if not, use the request // if not explicitly set in the request, use the index setting, if not, use the request
if (request.requestCache() == null) { if (request.requestCache() == null) {

View File

@ -33,3 +33,10 @@ The Search API returns `400 - Bad request` while it would previously return
* the number of slices is too large * the number of slices is too large
* keep alive for scroll is too large * keep alive for scroll is too large
* number of filters in the adjacency matrix aggregation is too large * number of filters in the adjacency matrix aggregation is too large
==== Scroll queries cannot use the request_cache anymore
Setting `request_cache:true` on a query that creates a scroll ('scroll=1m`)
has been deprecated in 6 and will now return a `400 - Bad request`.
Scroll queries are not meant to be cached.

View File

@ -197,3 +197,20 @@
clear_scroll: clear_scroll:
scroll_id: $scroll_id scroll_id: $scroll_id
---
"Scroll cannot used the request cache":
- skip:
version: " - 6.99.99"
reason: the error message has been added in v7.0.0
- do:
indices.create:
index: test_scroll
- do:
catch: /\[request_cache\] cannot be used in a a scroll context/
search:
index: test_scroll
scroll: 1m
request_cache: true
body:
query:
match_all: {}