Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more context to QueryShardContext #46584

Merged
merged 1 commit into from
Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -552,8 +552,10 @@ private static Response prepareRamIndex(Request request,
indexWriter.addDocuments(parsedDocument.docs());
try (IndexReader indexReader = DirectoryReader.open(indexWriter)) {
final long absoluteStartMillis = System.currentTimeMillis();
final IndexSearcher searcher = new IndexSearcher(indexReader);
searcher.setQueryCache(null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a small question on this. where did we do setQueryCache(null) before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't be required, I added this by precaution but I can remove it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it's fine by me, I was just curious

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and what about the places where we were previously setting the query cache and the policy?

QueryShardContext context =
indexService.newQueryShardContext(0, indexReader, () -> absoluteStartMillis, null);
indexService.newQueryShardContext(0, searcher, () -> absoluteStartMillis, null);
return handler.apply(context, indexReader.leaves().get(0));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ public void testRangeQueriesWithNow() throws Exception {
try (Engine.Searcher searcher = indexService.getShard(0).acquireSearcher("test")) {
long[] currentTime = new long[] {System.currentTimeMillis()};
QueryShardContext queryShardContext =
indexService.newQueryShardContext(0, searcher.getIndexReader(), () -> currentTime[0], null);
indexService.newQueryShardContext(0, searcher, () -> currentTime[0], null);

BytesReference source = BytesReference.bytes(jsonBuilder().startObject()
.field("field1", "value")
Expand Down
19 changes: 5 additions & 14 deletions server/src/main/java/org/elasticsearch/index/IndexService.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.logging.log4j.message.ParameterizedMessage;
import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.IndexReaderContext;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Sort;
import org.apache.lucene.store.AlreadyClosedException;
Expand Down Expand Up @@ -522,24 +521,16 @@ public IndexSettings getIndexSettings() {
return indexSettings;
}

private IndexSearcher newCachedSearcher(int shardId, IndexReaderContext context) {
IndexSearcher searcher = new IndexSearcher(context);
searcher.setQueryCache(cache().query());
searcher.setQueryCachingPolicy(getShard(shardId).getQueryCachingPolicy());
return searcher;
}

/**
* Creates a new QueryShardContext.
*
* Passing a {@code null} {@link IndexReader} will return a valid context, however it won't be able to make {@link IndexReader}-specific
* optimizations, such as rewriting containing range queries.
* Passing a {@code null} {@link IndexSearcher} will return a valid context, however it won't be able to make
* {@link IndexReader}-specific optimizations, such as rewriting containing range queries.
*/
public QueryShardContext newQueryShardContext(int shardId, IndexReader indexReader, LongSupplier nowInMillis, String clusterAlias) {
public QueryShardContext newQueryShardContext(int shardId, IndexSearcher searcher, LongSupplier nowInMillis, String clusterAlias) {
return new QueryShardContext(
shardId, indexSettings, indexCache.bitsetFilterCache(), context -> newCachedSearcher(shardId, context),
indexFieldData::getForField, mapperService(), similarityService(), scriptService, xContentRegistry, namedWriteableRegistry,
client, indexReader, nowInMillis, clusterAlias);
shardId, indexSettings, bigArrays, indexCache.bitsetFilterCache(), indexFieldData::getForField, mapperService(),
similarityService(), scriptService, xContentRegistry, namedWriteableRegistry, client, searcher, nowInMillis, clusterAlias);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@
package org.elasticsearch.index.fielddata;

import org.apache.lucene.index.DirectoryReader;
import org.apache.lucene.index.IndexReaderContext;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.ReaderUtil;
import org.apache.lucene.search.DocIdSet;
import org.apache.lucene.search.DocIdSetIterator;
import org.apache.lucene.search.FieldComparatorSource;
Expand All @@ -47,7 +45,6 @@
import org.elasticsearch.search.sort.NestedSortBuilder;

import java.io.IOException;
import java.util.function.Function;

/**
* Thread-safe utility class that allows to get per-segment values via the
Expand Down Expand Up @@ -115,24 +112,19 @@ public static class Nested {
private final BitSetProducer rootFilter;
private final Query innerQuery;
private final NestedSortBuilder nestedSort;
private final Function<IndexReaderContext, IndexSearcher> searcherFactory;
private final IndexSearcher searcher;

public Nested(BitSetProducer rootFilter, Query innerQuery, NestedSortBuilder nestedSort,
Function<IndexReaderContext, IndexSearcher> searcherFactory) {
public Nested(BitSetProducer rootFilter, Query innerQuery, NestedSortBuilder nestedSort, IndexSearcher searcher) {
this.rootFilter = rootFilter;
this.innerQuery = innerQuery;
this.nestedSort = nestedSort;
this.searcherFactory = searcherFactory;
this.searcher = searcher;
}

public Query getInnerQuery() {
return innerQuery;
}

public BitSetProducer getRootFilter() {
return rootFilter;
}

public NestedSortBuilder getNestedSort() { return nestedSort; }

/**
Expand All @@ -146,9 +138,7 @@ public BitSet rootDocs(LeafReaderContext ctx) throws IOException {
* Get a {@link DocIdSet} that matches the inner documents.
*/
public DocIdSetIterator innerDocs(LeafReaderContext ctx) throws IOException {
final IndexReaderContext topLevelCtx = ReaderUtil.getTopLevelContext(ctx);
IndexSearcher indexSearcher = searcherFactory.apply(topLevelCtx);
Weight weight = indexSearcher.createWeight(indexSearcher.rewrite(innerQuery), ScoreMode.COMPLETE_NO_SCORES, 1f);
Weight weight = searcher.createWeight(searcher.rewrite(innerQuery), ScoreMode.COMPLETE_NO_SCORES, 1f);
Scorer s = weight.scorer(ctx);
return s == null ? null : s.iterator();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import org.apache.logging.log4j.LogManager;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.index.IndexReader;
import org.apache.lucene.index.IndexReaderContext;
import org.apache.lucene.search.IndexSearcher;
import org.apache.lucene.search.Query;
import org.apache.lucene.search.join.BitSetProducer;
Expand All @@ -36,6 +35,7 @@
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.lucene.search.Queries;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.Index;
Expand Down Expand Up @@ -64,7 +64,6 @@
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.function.LongSupplier;

/**
Expand All @@ -78,13 +77,13 @@ public class QueryShardContext extends QueryRewriteContext {

private final ScriptService scriptService;
private final IndexSettings indexSettings;
private final BigArrays bigArrays;
private final MapperService mapperService;
private final SimilarityService similarityService;
private final BitsetFilterCache bitsetFilterCache;
private final Function<IndexReaderContext, IndexSearcher> searcherFactory;
private final BiFunction<MappedFieldType, String, IndexFieldData<?>> indexFieldDataService;
private final int shardId;
private final IndexReader reader;
private final IndexSearcher searcher;
private boolean cacheable = true;
private final SetOnce<Boolean> frozen = new SetOnce<>();
private final Index fullyQualifiedIndex;
Expand All @@ -94,42 +93,58 @@ public class QueryShardContext extends QueryRewriteContext {
private boolean mapUnmappedFieldAsString;
private NestedScope nestedScope;

public QueryShardContext(int shardId, IndexSettings indexSettings, BitsetFilterCache bitsetFilterCache,
Function<IndexReaderContext, IndexSearcher> searcherFactory,
BiFunction<MappedFieldType, String, IndexFieldData<?>> indexFieldDataLookup, MapperService mapperService,
SimilarityService similarityService, ScriptService scriptService, NamedXContentRegistry xContentRegistry,
NamedWriteableRegistry namedWriteableRegistry, Client client, IndexReader reader, LongSupplier nowInMillis,
String clusterAlias) {
this(shardId, indexSettings, bitsetFilterCache, searcherFactory, indexFieldDataLookup, mapperService, similarityService,
scriptService, xContentRegistry, namedWriteableRegistry, client, reader, nowInMillis,
public QueryShardContext(int shardId,
IndexSettings indexSettings,
BigArrays bigArrays,
BitsetFilterCache bitsetFilterCache,
BiFunction<MappedFieldType, String, IndexFieldData<?>> indexFieldDataLookup,
MapperService mapperService,
SimilarityService similarityService,
ScriptService scriptService,
NamedXContentRegistry xContentRegistry,
NamedWriteableRegistry namedWriteableRegistry,
Client client,
IndexSearcher searcher,
LongSupplier nowInMillis,
String clusterAlias) {
this(shardId, indexSettings, bigArrays, bitsetFilterCache, indexFieldDataLookup, mapperService, similarityService,
scriptService, xContentRegistry, namedWriteableRegistry, client, searcher, nowInMillis,
new Index(RemoteClusterAware.buildRemoteIndexName(clusterAlias, indexSettings.getIndex().getName()),
indexSettings.getIndex().getUUID()));
}

public QueryShardContext(QueryShardContext source) {
this(source.shardId, source.indexSettings, source.bitsetFilterCache, source.searcherFactory, source.indexFieldDataService,
source.mapperService, source.similarityService, source.scriptService, source.getXContentRegistry(),
source.getWriteableRegistry(), source.client, source.reader, source.nowInMillis, source.fullyQualifiedIndex);
}

private QueryShardContext(int shardId, IndexSettings indexSettings, BitsetFilterCache bitsetFilterCache,
Function<IndexReaderContext, IndexSearcher> searcherFactory,
BiFunction<MappedFieldType, String, IndexFieldData<?>> indexFieldDataLookup, MapperService mapperService,
SimilarityService similarityService, ScriptService scriptService, NamedXContentRegistry xContentRegistry,
NamedWriteableRegistry namedWriteableRegistry, Client client, IndexReader reader, LongSupplier nowInMillis,
Index fullyQualifiedIndex) {
this(source.shardId, source.indexSettings, source.bigArrays, source.bitsetFilterCache, source.indexFieldDataService,
source.mapperService, source.similarityService, source.scriptService, source.getXContentRegistry(),
source.getWriteableRegistry(), source.client, source.searcher, source.nowInMillis, source.fullyQualifiedIndex);
}

private QueryShardContext(int shardId,
IndexSettings indexSettings,
BigArrays bigArrays,
BitsetFilterCache bitsetFilterCache,
BiFunction<MappedFieldType, String, IndexFieldData<?>> indexFieldDataLookup,
MapperService mapperService,
SimilarityService similarityService,
ScriptService scriptService,
NamedXContentRegistry xContentRegistry,
NamedWriteableRegistry namedWriteableRegistry,
Client client,
IndexSearcher searcher,
LongSupplier nowInMillis,
Index fullyQualifiedIndex) {
super(xContentRegistry, namedWriteableRegistry, client, nowInMillis);
this.shardId = shardId;
this.similarityService = similarityService;
this.mapperService = mapperService;
this.bitsetFilterCache = bitsetFilterCache;
this.searcherFactory = searcherFactory;
this.indexFieldDataService = indexFieldDataLookup;
this.allowUnmappedFields = indexSettings.isDefaultAllowUnmappedFields();
this.nestedScope = new NestedScope();
this.scriptService = scriptService;
this.indexSettings = indexSettings;
this.reader = reader;
this.bigArrays = bigArrays;
this.searcher = searcher;
this.fullyQualifiedIndex = fullyQualifiedIndex;
}

Expand Down Expand Up @@ -168,10 +183,6 @@ public BitSetProducer bitsetFilter(Query filter) {
return bitsetFilterCache.getBitSetProducer(filter);
}

public IndexSearcher newCachedSearcher(IndexReaderContext context) {
return searcherFactory.apply(context);
}

public <IFD extends IndexFieldData<?>> IFD getForField(MappedFieldType fieldType) {
return (IFD) indexFieldDataService.apply(fieldType, fullyQualifiedIndex.getName());
}
Expand Down Expand Up @@ -397,7 +408,13 @@ public MapperService getMapperService() {
/** Return the current {@link IndexReader}, or {@code null} if no index reader is available,
* for instance if this rewrite context is used to index queries (percolation). */
public IndexReader getIndexReader() {
return reader;
return searcher != null ? searcher.getIndexReader() : null;
}

/** Return the current {@link IndexSearcher}, or {@code null} if no index reader is available,
* for instance if this rewrite context is used to index queries (percolation). */
public IndexSearcher searcher() {
return searcher;
}

/**
Expand All @@ -406,4 +423,11 @@ public IndexReader getIndexReader() {
public Index getFullyQualifiedIndex() {
return fullyQualifiedIndex;
}

/**
* Return the {@link BigArrays} instance for this node.
*/
public BigArrays bigArrays() {
return bigArrays;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ final class DefaultSearchContext extends SearchContext {
this.relativeTimeSupplier = relativeTimeSupplier;
this.timeout = timeout;
this.minNodeVersion = minNodeVersion;
queryShardContext = indexService.newQueryShardContext(request.shardId().id(), searcher.getIndexReader(), request::nowInMillis,
queryShardContext = indexService.newQueryShardContext(request.shardId().id(), searcher, request::nowInMillis,
shardTarget.getClusterAlias());
queryBoost = request.indexBoost();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ protected static Nested resolveNested(QueryShardContext context, NestedSortBuild
} else {
parentQuery = objectMapper.nestedTypeFilter();
}
return new Nested(context.bitsetFilter(parentQuery), childQuery, nestedSort, context::newCachedSearcher);
return new Nested(context.bitsetFilter(parentQuery), childQuery, nestedSort, context.searcher());
}

private static Query resolveNestedQuery(QueryShardContext context, NestedSortBuilder nestedSort, Query parentQuery) throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ public void tearDown() throws Exception {

protected Nested createNested(IndexSearcher searcher, Query parentFilter, Query childFilter) throws IOException {
BitsetFilterCache s = indexService.cache().bitsetFilterCache();
return new Nested(s.getBitSetProducer(parentFilter), childFilter, null, IndexSearcher::new);
return new Nested(s.getBitSetProducer(parentFilter), childFilter, null, searcher);
}

public void testEmpty() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.common.time.DateFormatters;
import org.elasticsearch.common.time.DateMathParser;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.fielddata.AtomicNumericFieldData;
Expand Down Expand Up @@ -176,9 +177,9 @@ public void testTermQuery() {
Settings indexSettings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT)
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1).build();
QueryShardContext context = new QueryShardContext(0,
new IndexSettings(IndexMetaData.builder("foo").settings(indexSettings).build(),
indexSettings),
null, null, null, null, null, null, xContentRegistry(), writableRegistry(), null, null, () -> nowInMillis, null);
new IndexSettings(IndexMetaData.builder("foo").settings(indexSettings).build(), indexSettings),
BigArrays.NON_RECYCLING_INSTANCE, null, null, null, null, null,
xContentRegistry(), writableRegistry(), null, null, () -> nowInMillis, null);
MappedFieldType ft = createDefaultFieldType();
ft.setName("field");
String date = "2015-10-12T14:10:55";
Expand All @@ -200,7 +201,8 @@ public void testRangeQuery() throws IOException {
.put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1).put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 1).build();
QueryShardContext context = new QueryShardContext(0,
new IndexSettings(IndexMetaData.builder("foo").settings(indexSettings).build(), indexSettings),
null, null, null, null, null, null, xContentRegistry(), writableRegistry(), null, null, () -> nowInMillis, null);
BigArrays.NON_RECYCLING_INSTANCE, null, null, null, null, null, xContentRegistry(), writableRegistry(),
null, null, () -> nowInMillis, null);
MappedFieldType ft = createDefaultFieldType();
ft.setName("field");
String date1 = "2015-10-12T14:10:55";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.elasticsearch.Version;
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.BigArrays;
import org.elasticsearch.index.IndexSettings;
import org.elasticsearch.index.query.QueryShardContext;
import org.junit.Before;
Expand Down Expand Up @@ -66,7 +67,8 @@ public void testTermQuery() {
when(mapperService.simpleMatchToFullName("field_name")).thenReturn(Collections.singleton("field_name"));

QueryShardContext queryShardContext = new QueryShardContext(0,
indexSettings, null, null, null, mapperService, null, null, null, null, null, null, () -> 0L, null);
indexSettings, BigArrays.NON_RECYCLING_INSTANCE, null, null, mapperService,
null, null, null, null, null, null, () -> 0L, null);
fieldNamesFieldType.setEnabled(true);
Query termQuery = fieldNamesFieldType.termQuery("field_name", queryShardContext);
assertEquals(new TermQuery(new Term(FieldNamesFieldMapper.CONTENT_TYPE, "field_name")), termQuery);
Expand Down
Loading