From a008959f7a1b9fbe970a9631a96c73ac7a1458df Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 5 Oct 2016 12:22:54 +0200 Subject: [PATCH] cleanup freeze methods and move them down to QueryShardContext --- .../index/query/QueryRewriteContext.java | 25 ---------- .../index/query/QueryShardContext.java | 48 +++++++++++++++++-- .../elasticsearch/indices/IndicesService.java | 2 +- .../elasticsearch/search/SearchService.java | 2 +- .../search/internal/SearchContext.java | 4 -- 5 files changed, 45 insertions(+), 36 deletions(-) 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 55ba778d012..d7637f46370 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java @@ -48,8 +48,6 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { protected final Client client; protected final IndexReader reader; protected final ClusterState clusterState; - protected boolean cachable = true; - private final SetOnce executionMode = new SetOnce<>(); public QueryRewriteContext(IndexSettings indexSettings, MapperService mapperService, ScriptService scriptService, IndicesQueriesRegistry indicesQueriesRegistry, Client client, IndexReader reader, @@ -119,34 +117,11 @@ public class QueryRewriteContext implements ParseFieldMatcherSupplier { return new QueryParseContext(defaultScriptLanguage, indicesQueriesRegistry, parser, indexSettings.getParseFieldMatcher()); } - /** - * Returns true iff the result of the processed search request is cachable. Otherwise false - */ - public boolean isCachable() { - return cachable; - } - public BytesReference getTemplateBytes(Script template) { - failIfExecutionMode(); ExecutableScript executable = scriptService.executable(template, ScriptContext.Standard.SEARCH, Collections.emptyMap()); return (BytesReference) executable.run(); } - public void setExecutionMode() { - this.executionMode.set(Boolean.TRUE); - } - /** - * This method fails if {@link #setExecutionMode()} is called before on this context. - * This is used to seal - */ - protected void failIfExecutionMode() { - this.cachable = false; - if (executionMode.get() == Boolean.TRUE) { - throw new IllegalArgumentException("features that prevent cachability are disabled on this context"); - } else { - assert executionMode.get() == null : executionMode.get(); - } - } } 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 d15c69e088e..abe573c0767 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -36,11 +36,13 @@ import org.apache.lucene.queryparser.classic.QueryParserSettings; import org.apache.lucene.search.Query; import org.apache.lucene.search.join.BitSetProducer; import org.apache.lucene.search.similarities.Similarity; +import org.apache.lucene.util.SetOnce; import org.elasticsearch.Version; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.lucene.search.Queries; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexSettings; @@ -77,6 +79,8 @@ public class QueryShardContext extends QueryRewriteContext { private final IndexFieldDataService indexFieldDataService; private final IndexSettings indexSettings; private String[] types = Strings.EMPTY_ARRAY; + private boolean cachable = true; + private final SetOnce frozen = new SetOnce<>(); public void setTypes(String... types) { this.types = types; @@ -271,7 +275,7 @@ public class QueryShardContext extends QueryRewriteContext { } public long nowInMillis() { - failIfExecutionMode(); + failIfFrozen(); return nowInMillis.getAsLong(); } @@ -339,7 +343,7 @@ public class QueryShardContext extends QueryRewriteContext { * Compiles (or retrieves from cache) and executes the provided script */ public SearchScript getSearchScript(Script script, ScriptContext context, Map params) { - failIfExecutionMode(); + failIfFrozen(); return scriptService.search(lookup(), script, context, params); } /** @@ -348,7 +352,7 @@ public class QueryShardContext extends QueryRewriteContext { */ public Function, SearchScript> getLazySearchScript(Script script, ScriptContext context, Map params) { - failIfExecutionMode(); + failIfFrozen(); CompiledScript compile = scriptService.compile(script, context, params); return (p) -> scriptService.search(lookup(), compile, p); } @@ -357,7 +361,7 @@ public class QueryShardContext extends QueryRewriteContext { * Compiles (or retrieves from cache) and executes the provided script */ public ExecutableScript getExecutableScript(Script script, ScriptContext context, Map params) { - failIfExecutionMode(); + failIfFrozen(); return scriptService.executable(script, context, params); } @@ -367,8 +371,42 @@ public class QueryShardContext extends QueryRewriteContext { */ public Function, ExecutableScript> getLazyExecutableScript(Script script, ScriptContext context, Map params) { - failIfExecutionMode(); + failIfFrozen(); CompiledScript executable = scriptService.compile(script, context, params); return (p) -> scriptService.executable(executable, p); } + + /** + * if this method is called the query context will throw exception if methods are accessed + * that could yield different results across executions like {@link #getTemplateBytes(Script)} + */ + public void freezeContext() { + this.frozen.set(Boolean.TRUE); + } + + /** + * This method fails if {@link #freezeContext()} is called before on this context. + * This is used to seal + */ + protected void failIfFrozen() { + this.cachable = false; + if (frozen.get() == Boolean.TRUE) { + throw new IllegalArgumentException("features that prevent cachability are disabled on this context"); + } else { + assert frozen.get() == null : frozen.get(); + } + } + + @Override + public BytesReference getTemplateBytes(Script template) { + failIfFrozen(); + return super.getTemplateBytes(template); + } + + /** + * Returns true iff the result of the processed search request is cachable. Otherwise false + */ + public boolean isCachable() { + return cachable; + } } diff --git a/core/src/main/java/org/elasticsearch/indices/IndicesService.java b/core/src/main/java/org/elasticsearch/indices/IndicesService.java index b658f70ed58..48947373601 100644 --- a/core/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/core/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -1082,7 +1082,7 @@ public class IndicesService extends AbstractLifecycleComponent } // if now in millis is used (or in the future, a more generic "isDeterministic" flag // then we can't cache based on "now" key within the search request, as it is not deterministic - if (context.isCachable() == false) { + if (context.getQueryShardContext().isCachable() == false) { return false; } return true; diff --git a/core/src/main/java/org/elasticsearch/search/SearchService.java b/core/src/main/java/org/elasticsearch/search/SearchService.java index ff6a359f0b0..c3fbd123fc8 100644 --- a/core/src/main/java/org/elasticsearch/search/SearchService.java +++ b/core/src/main/java/org/elasticsearch/search/SearchService.java @@ -231,7 +231,7 @@ public class SearchService extends AbstractLifecycleComponent implements IndexEv */ private void loadOrExecuteQueryPhase(final ShardSearchRequest request, final SearchContext context) throws Exception { final boolean canCache = indicesService.canCache(request, context); - context.getQueryShardContext().setExecutionMode(); + context.getQueryShardContext().freezeContext(); if (canCache) { indicesService.loadIntoContext(request, context, queryPhase); } else { diff --git a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java index 78fbc492c50..35bfb00dc46 100644 --- a/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java +++ b/core/src/main/java/org/elasticsearch/search/internal/SearchContext.java @@ -159,10 +159,6 @@ public abstract class SearchContext extends AbstractRefCounted implements Releas public abstract long getOriginNanoTime(); - public final boolean isCachable() { - return getQueryShardContext().isCachable(); - } - public abstract ScrollContext scrollContext(); public abstract SearchContext scrollContext(ScrollContext scroll);