From 94b7873b49a83ac769b6b83ddd715b88e0697dbf Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 4 Oct 2016 18:05:00 +0200 Subject: [PATCH] Add a #markAsNotCachable() method to context to mark requests as not cachable --- .../index/query/InnerHitBuilder.java | 3 ++- .../index/query/QueryRewriteContext.java | 9 ++++++++ .../index/query/QueryShardContext.java | 9 ++++++++ .../index/query/ScriptQueryBuilder.java | 4 +++- .../ScriptScoreFunctionBuilder.java | 1 + .../elasticsearch/indices/IndicesService.java | 2 +- .../elasticsearch/search/SearchService.java | 4 ++-- .../SignificantTermsAggregationBuilder.java | 3 +++ .../heuristics/ScriptHeuristic.java | 6 ++++++ .../heuristics/SignificanceHeuristic.java | 4 ++++ .../ScriptedMetricAggregationBuilder.java | 1 + .../scripted/ScriptedMetricAggregator.java | 2 ++ .../tophits/TopHitsAggregationBuilder.java | 3 +++ .../tophits/TopHitsAggregatorFactory.java | 1 + .../ValuesSourceAggregationBuilder.java | 8 +++++-- .../search/internal/SearchContext.java | 16 ++++++++------ .../search/sort/ScriptSortBuilder.java | 3 +-- .../phrase/PhraseSuggestionBuilder.java | 1 + .../index/query/ScriptQueryBuilderTests.java | 21 ++++++++++++------- .../FunctionScoreQueryBuilderTests.java | 11 ++++++++++ .../script/mustache/TemplateQueryBuilder.java | 1 + .../test/AbstractQueryTestCase.java | 12 +++++++++++ 22 files changed, 102 insertions(+), 23 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java index 454c808a5d5..0f4a1f3c429 100644 --- a/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/InnerHitBuilder.java @@ -574,8 +574,9 @@ public final class InnerHitBuilder extends ToXContentToBytes implements Writeabl innerHitsContext.docValueFieldsContext(new DocValueFieldsContext(docValueFields)); } if (scriptFields != null) { + context.markAsNotCachable(); for (ScriptField field : scriptFields) { - SearchScript searchScript = innerHitsContext.scriptService().search(innerHitsContext.lookup(), field.script(), + SearchScript searchScript = innerHitsContext.getQueryShardContext().getScriptService().search(innerHitsContext.lookup(), field.script(), ScriptContext.Standard.SEARCH, Collections.emptyMap()); innerHitsContext.scriptFields().add(new org.elasticsearch.search.fetch.subphase.ScriptFieldsContext.ScriptField( field.fieldName(), searchScript, field.ignoreFailure())); diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java index f12605088e6..64329c314dd 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -41,6 +41,7 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { protected final Client client; protected final IndexReader reader; protected final ClusterState clusterState; + protected boolean cachable; public QueryRewriteContext(IndexSettings indexSettings, MapperService mapperService, ScriptService scriptService, IndicesQueriesRegistry indicesQueriesRegistry, Client client, IndexReader reader, @@ -116,4 +117,12 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { String defaultScriptLanguage = ScriptSettings.getLegacyDefaultLang(indexSettings.getNodeSettings()); return new QueryParseContext(defaultScriptLanguage, indicesQueriesRegistry, parser, indexSettings.getParseFieldMatcher()); } + + public void markAsNotCachable() { + this.cachable = false; + } + + public boolean isCachable() { + return cachable; + } } diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index 377afca2f68..25b7ef35961 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -324,6 +324,15 @@ public class QueryShardContext extends QueryRewriteContext { } } + @Override + public void markAsNotCachable() { + super.markAsNotCachable(); + SearchContext current = SearchContext.current(); + if (current != null) { + current.markAsNotCachable(); + } + } + public final Index index() { return indexSettings.getIndex(); } diff --git a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java index 3ff924b28db..5a65b4438f0 100644 --- a/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/ScriptQueryBuilder.java @@ -41,7 +41,6 @@ import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; import java.util.Collections; -import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -134,6 +133,7 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder @Override protected Query doToQuery(QueryShardContext context) throws IOException { + context.markAsNotCachable(); return new ScriptQuery(script, context.getScriptService(), context.lookup()); } @@ -216,4 +216,6 @@ public class ScriptQueryBuilder extends AbstractQueryBuilder protected boolean doEquals(ScriptQueryBuilder other) { return Objects.equals(script, other.script); } + + } diff --git a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java index e2fbc5955d7..d7bff2bdd44 100644 --- a/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/functionscore/ScriptScoreFunctionBuilder.java @@ -95,6 +95,7 @@ public class ScriptScoreFunctionBuilder extends ScoreFunctionBuilder innerBuild(AggregationContext context, ValuesSourceConfig config, AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException { + if (significanceHeuristic.canCache()) { + context.searchContext().markAsNotCachable(); + } return new SignificantTermsAggregatorFactory(name, type, config, includeExclude, executionHint, filterBuilder, bucketCountThresholds, significanceHeuristic, context, parent, subFactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java index c933f9ef596..1c3f5b7ed56 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/ScriptHeuristic.java @@ -78,6 +78,7 @@ public class ScriptHeuristic extends SignificanceHeuristic { @Override public void initialize(SearchContext context) { + context.markAsNotCachable(); initialize(context.scriptService()); } @@ -217,5 +218,10 @@ public class ScriptHeuristic extends SignificanceHeuristic { return Long.toString(value); } } + + @Override + public boolean canCache() { + return false; + } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java index db9711c1a8d..094f529e92c 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/bucket/significant/heuristics/SignificanceHeuristic.java @@ -57,4 +57,8 @@ public abstract class SignificanceHeuristic implements NamedWriteable, ToXConten public void initialize(SearchContext context) { } + + public boolean canCache() { + return true; + } } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java index 244881a5155..b13b6fe3019 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregationBuilder.java @@ -182,6 +182,7 @@ public class ScriptedMetricAggregationBuilder extends AbstractAggregationBuilder @Override protected ScriptedMetricAggregatorFactory doBuild(AggregationContext context, AggregatorFactory parent, Builder subfactoriesBuilder) throws IOException { + context.searchContext().markAsNotCachable(); return new ScriptedMetricAggregatorFactory(name, type, initScript, mapScript, combineScript, reduceScript, params, context, parent, subfactoriesBuilder, metaData); } diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java index e2d3034fa11..89ec5183c32 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/scripted/ScriptedMetricAggregator.java @@ -53,10 +53,12 @@ public class ScriptedMetricAggregator extends MetricsAggregator { this.params = params; ScriptService scriptService = context.searchContext().scriptService(); if (initScript != null) { + context.searchContext().markAsNotCachable(); scriptService.executable(initScript, ScriptContext.Standard.AGGS, Collections.emptyMap()).run(); } this.mapScript = scriptService.search(context.searchContext().lookup(), mapScript, ScriptContext.Standard.AGGS, Collections.emptyMap()); if (combineScript != null) { + context.searchContext().markAsNotCachable(); this.combineScript = scriptService.executable(combineScript, ScriptContext.Standard.AGGS, Collections.emptyMap()); } else { this.combineScript = null; diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregationBuilder.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregationBuilder.java index 3547db7140c..6c26743d4dc 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregationBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregationBuilder.java @@ -527,6 +527,9 @@ public class TopHitsAggregationBuilder extends AbstractAggregationBuilder parent, Builder subfactoriesBuilder) throws IOException { + if (scriptFields != null && scriptFields.isEmpty() == false) { + context.searchContext().markAsNotCachable(); + } return new TopHitsAggregatorFactory(name, type, from, size, explain, version, trackScores, sorts, highlightBuilder, storedFieldsContext, fieldDataFields, scriptFields, fetchSourceContext, context, parent, subfactoriesBuilder, metaData); diff --git a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java index 7c6a743a20b..5dff86c5eed 100644 --- a/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java +++ b/core/src/main/java/org/elasticsearch/search/aggregations/metrics/tophits/TopHitsAggregatorFactory.java @@ -99,6 +99,7 @@ public class TopHitsAggregatorFactory extends AggregatorFactory, Collector> queryCollectors(); + public final void markAsNotCachable() { + this.canCache = false; + } + /** * The life time of an object that is used during search execution. */ diff --git a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java index 0a7cb5e1b36..d41e8c66d6f 100644 --- a/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/sort/ScriptSortBuilder.java @@ -55,9 +55,7 @@ import org.elasticsearch.search.MultiValueMode; import java.io.IOException; import java.util.Collections; -import java.util.HashMap; import java.util.Locale; -import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -286,6 +284,7 @@ public class ScriptSortBuilder extends SortBuilder { @Override public SortFieldAndFormat build(QueryShardContext context) throws IOException { + context.markAsNotCachable(); final SearchScript searchScript = context.getScriptService().search( context.lookup(), script, ScriptContext.Standard.SEARCH, Collections.emptyMap()); diff --git a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java index b0cd6a20499..a17a4f5de80 100644 --- a/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java +++ b/core/src/main/java/org/elasticsearch/search/suggest/phrase/PhraseSuggestionBuilder.java @@ -633,6 +633,7 @@ public class PhraseSuggestionBuilder extends SuggestionBuilder> } private QueryBuilder rewriteQuery(QB queryBuilder, QueryRewriteContext rewriteContext) throws IOException { + final boolean wasCachable = rewriteContext.isCachable(); QueryBuilder rewritten = QueryBuilder.rewriteQuery(queryBuilder, rewriteContext); + if (wasCachable == true) { // it's not resettable so we can only check it once + if (isCachable(queryBuilder)) { + assert rewriteContext.isCachable() : queryBuilder.toString(); + } else { + assert rewriteContext.isCachable() == false; + } + } // extra safety to fail fast - serialize the rewritten version to ensure it's serializable. assertSerialization(rewritten); return rewritten; } + protected boolean isCachable(QB queryBuilder) { + return true; + } + /** * Few queries allow you to set the boost and queryName on the java api, although the corresponding parser * doesn't parse them as they are not supported. This method allows to disable boost and queryName related tests for those queries.