From 8f23bc76a7cab3840bd9d9861066bb48f56054bd Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Mon, 23 Mar 2020 14:47:02 +0100 Subject: [PATCH 1/3] Reduce performance impact of ExitableDirectoryReader Benchmarking showed that the effect of the ExitableDirectoryReader is reduced considerably when checking every 4095 docs. Moreover, set the cancellable task before calling QueryPhase.preProcess() and make sure we don't wrap with an ExitableDirectoryReader at all when lowLevelCancellation() is set to false to avoid completely any performance impact. Follows: #52822 Follows: #53166 Follows: #53496 --- .../search/DefaultSearchContext.java | 11 ++++------ .../elasticsearch/search/SearchService.java | 16 ++++++-------- .../search/internal/ContextIndexSearcher.java | 21 ++++++++---------- .../internal/ExitableDirectoryReader.java | 4 ++-- .../search/query/QueryPhase.java | 8 +++---- .../search/DefaultSearchContextTests.java | 8 +++---- .../search/SearchCancellationTests.java | 14 ++++++------ .../search/SearchServiceTests.java | 22 +++++++++---------- .../internal/ContextIndexSearcherTests.java | 2 +- .../profile/query/QueryProfilerTests.java | 4 ++-- .../search/query/QueryPhaseTests.java | 6 ++--- .../aggregations/AggregatorTestCase.java | 2 +- ...ityIndexReaderWrapperIntegrationTests.java | 10 +++++---- 13 files changed, 61 insertions(+), 67 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java index bed38deb28a29..a4cdd540e3503 100644 --- a/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java +++ b/server/src/main/java/org/elasticsearch/search/DefaultSearchContext.java @@ -98,6 +98,7 @@ final class DefaultSearchContext extends SearchContext { private final QuerySearchResult queryResult; private final FetchSearchResult fetchResult; private final float queryBoost; + private final boolean lowLevelCancellation; private TimeValue timeout; // terminate after count private int terminateAfter = DEFAULT_TERMINATE_AFTER; @@ -118,7 +119,6 @@ final class DefaultSearchContext extends SearchContext { private int trackTotalHitsUpTo = SearchContext.DEFAULT_TRACK_TOTAL_HITS_UP_TO; private FieldDoc searchAfter; private CollapseContext collapse; - private boolean lowLevelCancellation; // filter for sliced scroll private SliceBuilder sliceBuilder; private SearchShardTask task; @@ -156,7 +156,7 @@ final class DefaultSearchContext extends SearchContext { DefaultSearchContext(SearchContextId id, ShardSearchRequest request, SearchShardTarget shardTarget, Engine.Searcher engineSearcher, ClusterService clusterService, IndexService indexService, IndexShard indexShard, BigArrays bigArrays, LongSupplier relativeTimeSupplier, TimeValue timeout, - FetchPhase fetchPhase) throws IOException { + FetchPhase fetchPhase, boolean lowLevelCancellation) throws IOException { this.id = id; this.request = request; this.fetchPhase = fetchPhase; @@ -172,12 +172,13 @@ final class DefaultSearchContext extends SearchContext { this.indexService = indexService; this.clusterService = clusterService; this.searcher = new ContextIndexSearcher(engineSearcher.getIndexReader(), engineSearcher.getSimilarity(), - engineSearcher.getQueryCache(), engineSearcher.getQueryCachingPolicy()); + engineSearcher.getQueryCache(), engineSearcher.getQueryCachingPolicy(), lowLevelCancellation); this.relativeTimeSupplier = relativeTimeSupplier; this.timeout = timeout; queryShardContext = indexService.newQueryShardContext(request.shardId().id(), searcher, request::nowInMillis, shardTarget.getClusterAlias()); queryBoost = request.indexBoost(); + this.lowLevelCancellation = lowLevelCancellation; } @Override @@ -563,10 +564,6 @@ public boolean lowLevelCancellation() { return lowLevelCancellation; } - public void lowLevelCancellation(boolean lowLevelCancellation) { - this.lowLevelCancellation = lowLevelCancellation; - } - @Override public FieldDoc searchAfter() { return searchAfter; diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 451c906500449..9821f8f212d8c 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -339,10 +339,9 @@ public void executeDfsPhase(ShardSearchRequest request, SearchShardTask task, Ac } private DfsSearchResult executeDfsPhase(SearchRewriteContext rewriteContext, SearchShardTask task) throws IOException { - final SearchContext context = createAndPutContext(rewriteContext); + final SearchContext context = createAndPutContext(rewriteContext, task); context.incRef(); try { - context.setTask(task); contextProcessing(context); dfsPhase.execute(context); contextProcessedSuccessfully(context); @@ -422,11 +421,10 @@ private void runAsync(SearchContextId contextId, Supplier executable, Act } private SearchPhaseResult executeQueryPhase(SearchRewriteContext rewriteContext, SearchShardTask task) throws Exception { - final SearchContext context = createAndPutContext(rewriteContext); + final SearchContext context = createAndPutContext(rewriteContext, task); final ShardSearchRequest request = rewriteContext.request; context.incRef(); try { - context.setTask(task); final long afterQueryTime; try (SearchOperationListenerExecutor executor = new SearchOperationListenerExecutor(context)) { contextProcessing(context); @@ -626,8 +624,8 @@ private SearchContext findContext(SearchContextId contextId, TransportRequest re } } - final SearchContext createAndPutContext(SearchRewriteContext rewriteContext) throws IOException { - SearchContext context = createContext(rewriteContext); + final SearchContext createAndPutContext(SearchRewriteContext rewriteContext, SearchShardTask task) throws IOException { + SearchContext context = createContext(rewriteContext, task); onNewContext(context); boolean success = false; try { @@ -660,7 +658,7 @@ private void onNewContext(SearchContext context) { } } - final SearchContext createContext(SearchRewriteContext rewriteContext) throws IOException { + final SearchContext createContext(SearchRewriteContext rewriteContext, SearchShardTask searchTask) throws IOException { final DefaultSearchContext context = createSearchContext(rewriteContext, defaultSearchTimeout); try { final ShardSearchRequest request = rewriteContext.request; @@ -684,6 +682,7 @@ final SearchContext createContext(SearchRewriteContext rewriteContext) throws IO if (context.size() == -1) { context.size(DEFAULT_SIZE); } + context.setTask(searchTask); // pre process dfsPhase.preProcess(context); @@ -696,7 +695,6 @@ final SearchContext createContext(SearchRewriteContext rewriteContext) throws IO keepAlive = request.scroll().keepAlive().millis(); } contextScrollKeepAlive(context, keepAlive); - context.lowLevelCancellation(lowLevelCancellation); } catch (Exception e) { context.close(); throw e; @@ -731,7 +729,7 @@ private DefaultSearchContext createSearchContext(SearchRewriteContext rewriteCon DefaultSearchContext searchContext = new DefaultSearchContext( new SearchContextId(readerId, idGenerator.incrementAndGet()), request, shardTarget, searcher, clusterService, indexService, indexShard, bigArrays, - threadPool::relativeTimeInMillis, timeout, fetchPhase); + threadPool::relativeTimeInMillis, timeout, fetchPhase, lowLevelCancellation); success = true; return searchContext; } finally { diff --git a/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java b/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java index d3c4f5641dc23..ed41939b1cb9c 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java @@ -82,23 +82,20 @@ public class ContextIndexSearcher extends IndexSearcher { private MutableQueryTimeout cancellable; public ContextIndexSearcher(IndexReader reader, Similarity similarity, - QueryCache queryCache, QueryCachingPolicy queryCachingPolicy) throws IOException { - this(reader, similarity, queryCache, queryCachingPolicy, new MutableQueryTimeout()); + QueryCache queryCache, QueryCachingPolicy queryCachingPolicy, + boolean wrapWithExitableDirectoryReader) throws IOException { + this(reader, similarity, queryCache, queryCachingPolicy, new MutableQueryTimeout(), wrapWithExitableDirectoryReader); } - // TODO: Make the 2nd constructor private so that the IndexReader is always wrapped. - // Some issues must be fixed: - // - regarding tests deriving from AggregatorTestCase and more specifically the use of searchAndReduce and - // the ShardSearcher sub-searchers. - // - tests that use a MultiReader - public ContextIndexSearcher(IndexReader reader, Similarity similarity, - QueryCache queryCache, QueryCachingPolicy queryCachingPolicy, - MutableQueryTimeout cancellable) throws IOException { - super(cancellable != null ? new ExitableDirectoryReader((DirectoryReader) reader, cancellable) : reader); + private ContextIndexSearcher(IndexReader reader, Similarity similarity, + QueryCache queryCache, QueryCachingPolicy queryCachingPolicy, + MutableQueryTimeout cancellable, + boolean wrapWithExitableDirectoryReader) throws IOException { + super(wrapWithExitableDirectoryReader ? new ExitableDirectoryReader((DirectoryReader) reader, cancellable) : reader); setSimilarity(similarity); setQueryCache(queryCache); setQueryCachingPolicy(queryCachingPolicy); - this.cancellable = cancellable != null ? cancellable : new MutableQueryTimeout(); + this.cancellable = cancellable; } public void setProfiler(QueryProfiler profiler) { diff --git a/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java b/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java index 51a94f4023b65..5306738f862ca 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java @@ -245,7 +245,7 @@ public int getDocCount() { private static class ExitableIntersectVisitor implements PointValues.IntersectVisitor { - private static final int MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = (1 << 10) - 1; // 1023 + private static final int MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = (1 << 12) - 1; // 4095 private final PointValues.IntersectVisitor in; private final QueryCancellation queryCancellation; @@ -276,7 +276,7 @@ public void visit(int docID, byte[] packedValue) throws IOException { @Override public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { - queryCancellation.checkCancelled(); + checkAndThrowWithSampling(); return in.compare(minPackedValue, maxPackedValue); } diff --git a/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java b/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java index 104dfdbee21a6..f1c416ca3b632 100644 --- a/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java +++ b/server/src/main/java/org/elasticsearch/search/query/QueryPhase.java @@ -112,9 +112,9 @@ public QueryPhase() { public void preProcess(SearchContext context) { final Runnable cancellation; if (context.lowLevelCancellation()) { - SearchShardTask task = context.getTask(); cancellation = context.searcher().addQueryCancellation(() -> { - if (task.isCancelled()) { + SearchShardTask task = context.getTask(); + if (task != null && task.isCancelled()) { throw new TaskCancelledException("cancelled"); } }); @@ -282,9 +282,9 @@ static boolean executeInternal(SearchContext searchContext) throws QueryPhaseExe } if (searchContext.lowLevelCancellation()) { - SearchShardTask task = searchContext.getTask(); searcher.addQueryCancellation(() -> { - if (task.isCancelled()) { + SearchShardTask task = searchContext.getTask(); + if (task != null && task.isCancelled()) { throw new TaskCancelledException("cancelled"); } }); diff --git a/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java b/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java index 0a68c54e75b47..75dec54b0b4fe 100644 --- a/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java +++ b/server/src/test/java/org/elasticsearch/search/DefaultSearchContextTests.java @@ -121,7 +121,7 @@ public void testPreProcess() throws Exception { SearchShardTarget target = new SearchShardTarget("node", shardId, null, OriginalIndices.NONE); DefaultSearchContext context1 = new DefaultSearchContext(new SearchContextId(UUIDs.randomBase64UUID(), 1L), - shardSearchRequest, target, searcher, null, indexService, indexShard, bigArrays, null, timeout, null); + shardSearchRequest, target, searcher, null, indexService, indexShard, bigArrays, null, timeout, null, false); context1.from(300); // resultWindow greater than maxResultWindow and scrollContext is null @@ -162,7 +162,7 @@ public void testPreProcess() throws Exception { // rescore is null but sliceBuilder is not null DefaultSearchContext context2 = new DefaultSearchContext(new SearchContextId(UUIDs.randomBase64UUID(), 2L), - shardSearchRequest, target, searcher, null, indexService, indexShard, bigArrays, null, timeout, null); + shardSearchRequest, target, searcher, null, indexService, indexShard, bigArrays, null, timeout, null, false); SliceBuilder sliceBuilder = mock(SliceBuilder.class); int numSlices = maxSlicesPerScroll + randomIntBetween(1, 100); @@ -179,7 +179,7 @@ public void testPreProcess() throws Exception { when(shardSearchRequest.indexBoost()).thenReturn(AbstractQueryBuilder.DEFAULT_BOOST); DefaultSearchContext context3 = new DefaultSearchContext(new SearchContextId(UUIDs.randomBase64UUID(), 3L), - shardSearchRequest, target, searcher, null, indexService, indexShard, bigArrays, null, timeout, null); + shardSearchRequest, target, searcher, null, indexService, indexShard, bigArrays, null, timeout, null, false); ParsedQuery parsedQuery = ParsedQuery.parsedMatchAllQuery(); context3.sliceBuilder(null).parsedQuery(parsedQuery).preProcess(false); assertEquals(context3.query(), context3.buildFilteredQuery(parsedQuery.query())); @@ -189,7 +189,7 @@ public void testPreProcess() throws Exception { when(shardSearchRequest.indexRoutings()).thenReturn(new String[0]); DefaultSearchContext context4 = new DefaultSearchContext(new SearchContextId(UUIDs.randomBase64UUID(), 4L), - shardSearchRequest, target, searcher, null, indexService, indexShard, bigArrays, null, timeout, null); + shardSearchRequest, target, searcher, null, indexService, indexShard, bigArrays, null, timeout, null, false); context4.sliceBuilder(new SliceBuilder(1,2)).parsedQuery(parsedQuery).preProcess(false); Query query1 = context4.query(); context4.sliceBuilder(new SliceBuilder(0,2)).parsedQuery(parsedQuery).preProcess(false); diff --git a/server/src/test/java/org/elasticsearch/search/SearchCancellationTests.java b/server/src/test/java/org/elasticsearch/search/SearchCancellationTests.java index 3aafdcc575230..1ea831eb7750d 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchCancellationTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchCancellationTests.java @@ -86,8 +86,8 @@ public static void cleanup() throws IOException { } public void testAddingCancellationActions() throws IOException { - ContextIndexSearcher searcher = new ContextIndexSearcher(reader, - IndexSearcher.getDefaultSimilarity(), IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()); + ContextIndexSearcher searcher = new ContextIndexSearcher(reader, IndexSearcher.getDefaultSimilarity(), + IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy(), true); NullPointerException npe = expectThrows(NullPointerException.class, () -> searcher.addQueryCancellation(null)); assertEquals("cancellation runnable should not be null", npe.getMessage()); @@ -100,8 +100,8 @@ public void testAddingCancellationActions() throws IOException { public void testCancellableCollector() throws IOException { TotalHitCountCollector collector1 = new TotalHitCountCollector(); Runnable cancellation = () -> { throw new TaskCancelledException("cancelled"); }; - ContextIndexSearcher searcher = new ContextIndexSearcher(reader, - IndexSearcher.getDefaultSimilarity(), IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()); + ContextIndexSearcher searcher = new ContextIndexSearcher(reader, IndexSearcher.getDefaultSimilarity(), + IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy(), true); searcher.search(new MatchAllDocsQuery(), collector1); assertThat(collector1.getTotalHits(), equalTo(reader.numDocs())); @@ -116,14 +116,14 @@ public void testCancellableCollector() throws IOException { assertThat(collector2.getTotalHits(), equalTo(reader.numDocs())); } - public void testCancellableDirectoryReader() throws IOException { + public void testExitableDirectoryReader() throws IOException { AtomicBoolean cancelled = new AtomicBoolean(true); Runnable cancellation = () -> { if (cancelled.get()) { throw new TaskCancelledException("cancelled"); }}; - ContextIndexSearcher searcher = new ContextIndexSearcher(reader, - IndexSearcher.getDefaultSimilarity(), IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()); + ContextIndexSearcher searcher = new ContextIndexSearcher(reader, IndexSearcher.getDefaultSimilarity(), + IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy(), true); searcher.addQueryCancellation(cancellation); CompiledAutomaton automaton = new CompiledAutomaton(new RegExp("a.*").toAutomaton()); diff --git a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java index e1e6003695d3a..7c5d573ce226c 100644 --- a/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java +++ b/server/src/test/java/org/elasticsearch/search/SearchServiceTests.java @@ -368,7 +368,7 @@ public void testTimeout() throws IOException { new ShardSearchRequest(OriginalIndices.NONE, searchRequest, indexShard.shardId(), 1, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, -1, null, null), indexShard); - final SearchContext contextWithDefaultTimeout = service.createContext(rewriteContext); + final SearchContext contextWithDefaultTimeout = service.createContext(rewriteContext, null); try { // the search context should inherit the default timeout assertThat(contextWithDefaultTimeout.timeout(), equalTo(TimeValue.timeValueSeconds(5))); @@ -383,7 +383,7 @@ public void testTimeout() throws IOException { new ShardSearchRequest(OriginalIndices.NONE, searchRequest, indexShard.shardId(), 1, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, -1, null, null), indexShard); - final SearchContext context = service.createContext(rewriteContext); + final SearchContext context = service.createContext(rewriteContext, null); try { // the search context should inherit the query timeout assertThat(context.timeout(), equalTo(TimeValue.timeValueSeconds(seconds))); @@ -417,7 +417,7 @@ public void testMaxDocvalueFieldsSearch() throws IOException { { SearchService.SearchRewriteContext rewriteContext = service.acquireSearcherAndRewrite(shardRequest, indexShard); - try (SearchContext context = service.createContext(rewriteContext)) { + try (SearchContext context = service.createContext(rewriteContext, null)) { assertNotNull(context); } } @@ -426,7 +426,7 @@ public void testMaxDocvalueFieldsSearch() throws IOException { SearchService.SearchRewriteContext rewriteContext = service.acquireSearcherAndRewrite(shardRequest, indexShard); searchSourceBuilder.docValueField("one_field_too_much"); IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> service.createContext(rewriteContext)); + () -> service.createContext(rewriteContext, null)); assertEquals( "Trying to retrieve too many docvalue_fields. Must be less than or equal to: [100] but was [101]. " + "This limit can be set by changing the [index.max_docvalue_fields_search] index level setting.", ex.getMessage()); @@ -458,7 +458,7 @@ public void testMaxScriptFieldsSearch() throws IOException { { SearchService.SearchRewriteContext rewriteContext = service.acquireSearcherAndRewrite(shardRequest, indexShard); - try (SearchContext context = service.createContext(rewriteContext)) { + try (SearchContext context = service.createContext(rewriteContext, null)) { assertNotNull(context); } } @@ -468,7 +468,7 @@ public void testMaxScriptFieldsSearch() throws IOException { new Script(ScriptType.INLINE, MockScriptEngine.NAME, CustomScriptPlugin.DUMMY_SCRIPT, Collections.emptyMap())); SearchService.SearchRewriteContext rewriteContext = service.acquireSearcherAndRewrite(shardRequest, indexShard); IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, - () -> service.createContext(rewriteContext)); + () -> service.createContext(rewriteContext, null)); assertEquals( "Trying to retrieve too many script_fields. Must be less than or equal to: [" + maxScriptFields + "] but was [" + (maxScriptFields + 1) @@ -494,7 +494,7 @@ public void testIgnoreScriptfieldIfSizeZero() throws IOException { new ShardSearchRequest(OriginalIndices.NONE, searchRequest, indexShard.shardId(), 1, new AliasFilter(null, Strings.EMPTY_ARRAY), 1.0f, -1, null, null), indexShard); - try (SearchContext context = service.createContext(rewriteContext)) { + try (SearchContext context = service.createContext(rewriteContext, null)) { assertEquals(0, context.scriptFields().fields().size()); } } @@ -531,7 +531,7 @@ public void testMaxOpenScrollContexts() throws RuntimeException, IOException { SearchService.SearchRewriteContext rewriteContext = service.acquireSearcherAndRewrite(new ShardScrollRequestTest(indexShard.shardId()), indexShard); ElasticsearchException ex = expectThrows(ElasticsearchException.class, - () -> service.createAndPutContext(rewriteContext)); + () -> service.createAndPutContext(rewriteContext, null)); assertEquals( "Trying to create too many scroll contexts. Must be less than or equal to: [" + SearchService.MAX_OPEN_SCROLL_CONTEXT.get(Settings.EMPTY) + "]. " + @@ -557,7 +557,7 @@ public void testOpenScrollContextsConcurrently() throws Exception { SearchService.SearchRewriteContext rewriteContext = searchService.acquireSearcherAndRewrite(new ShardScrollRequestTest(indexShard.shardId()), indexShard); try { - searchService.createAndPutContext(rewriteContext); + searchService.createAndPutContext(rewriteContext, null); } catch (ElasticsearchException e) { assertThat(e.getMessage(), equalTo( "Trying to create too many scroll contexts. Must be less than or equal to: " + @@ -835,7 +835,7 @@ public SearchType searchType() { } }, indexShard); NullPointerException e = expectThrows(NullPointerException.class, - () -> service.createContext(rewriteContext)); + () -> service.createContext(rewriteContext, null)); assertEquals("expected", e.getMessage()); assertEquals("should have 2 store refs (IndexService + InternalEngine)", 2, indexService.getShard(0).store().refCount()); } @@ -988,7 +988,7 @@ OriginalIndices.NONE, new SearchRequest().allowPartialSearchResults(true), int numContexts = randomIntBetween(1, 10); for (int i = 0; i < numContexts; i++) { SearchService.SearchRewriteContext rewriteContext = searchService.acquireSearcherAndRewrite(shardSearchRequest, indexShard); - final SearchContext searchContext = searchService.createContext(rewriteContext); + final SearchContext searchContext = searchService.createContext(rewriteContext, null); assertThat(searchContext.id().getId(), equalTo((long) (i + 1))); searchService.putContext(searchContext); contextIds.add(searchContext.id()); diff --git a/server/src/test/java/org/elasticsearch/search/internal/ContextIndexSearcherTests.java b/server/src/test/java/org/elasticsearch/search/internal/ContextIndexSearcherTests.java index 61a75a0716fd2..b2c3a4e85455b 100644 --- a/server/src/test/java/org/elasticsearch/search/internal/ContextIndexSearcherTests.java +++ b/server/src/test/java/org/elasticsearch/search/internal/ContextIndexSearcherTests.java @@ -240,7 +240,7 @@ public void onRemoval(ShardId shardId, Accountable accountable) { DocumentSubsetDirectoryReader filteredReader = new DocumentSubsetDirectoryReader(reader, cache, roleQuery); ContextIndexSearcher searcher = new ContextIndexSearcher(filteredReader, IndexSearcher.getDefaultSimilarity(), - IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()); + IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy(), true); // Assert wrapping assertEquals(ExitableDirectoryReader.class, searcher.getIndexReader().getClass()); diff --git a/server/src/test/java/org/elasticsearch/search/profile/query/QueryProfilerTests.java b/server/src/test/java/org/elasticsearch/search/profile/query/QueryProfilerTests.java index f05684930a9c5..96b2dc57a86f5 100644 --- a/server/src/test/java/org/elasticsearch/search/profile/query/QueryProfilerTests.java +++ b/server/src/test/java/org/elasticsearch/search/profile/query/QueryProfilerTests.java @@ -43,8 +43,8 @@ import org.apache.lucene.search.TotalHitCountCollector; import org.apache.lucene.search.Weight; import org.apache.lucene.store.Directory; -import org.elasticsearch.core.internal.io.IOUtils; import org.apache.lucene.util.TestUtil; +import org.elasticsearch.core.internal.io.IOUtils; import org.elasticsearch.search.internal.ContextIndexSearcher; import org.elasticsearch.search.profile.ProfileResult; import org.elasticsearch.test.ESTestCase; @@ -83,7 +83,7 @@ public static void setup() throws IOException { reader = w.getReader(); w.close(); searcher = new ContextIndexSearcher(reader, IndexSearcher.getDefaultSimilarity(), - IndexSearcher.getDefaultQueryCache(), ALWAYS_CACHE_POLICY); + IndexSearcher.getDefaultQueryCache(), ALWAYS_CACHE_POLICY, true); } @After diff --git a/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java b/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java index 489285748efcf..c7c3fd856d587 100644 --- a/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java +++ b/server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java @@ -912,12 +912,12 @@ public boolean lowLevelCancellation() { private static ContextIndexSearcher newContextSearcher(IndexReader reader) throws IOException { return new ContextIndexSearcher(reader, IndexSearcher.getDefaultSimilarity(), - IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()); + IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy(), true); } private static ContextIndexSearcher newEarlyTerminationContextSearcher(IndexReader reader, int size) throws IOException { return new ContextIndexSearcher(reader, IndexSearcher.getDefaultSimilarity(), - IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()) { + IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy(), true) { @Override public void search(List leaves, Weight weight, Collector collector) throws IOException { @@ -930,7 +930,7 @@ public void search(List leaves, Weight weight, Collector coll // used to check that numeric long or date sort optimization was run private static ContextIndexSearcher newOptimizedContextSearcher(IndexReader reader, int queryType) throws IOException { return new ContextIndexSearcher(reader, IndexSearcher.getDefaultSimilarity(), - IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()) { + IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy(), true) { @Override public void search(List leaves, Weight weight, CollectorManager manager, diff --git a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java index 40678a58d6e92..287935b035011 100644 --- a/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/search/aggregations/AggregatorTestCase.java @@ -246,7 +246,7 @@ public boolean shouldCache(Query query) { } }; ContextIndexSearcher contextIndexSearcher = new ContextIndexSearcher(indexSearcher.getIndexReader(), - indexSearcher.getSimilarity(), queryCache, queryCachingPolicy, null); + indexSearcher.getSimilarity(), queryCache, queryCachingPolicy, false); SearchContext searchContext = mock(SearchContext.class); when(searchContext.numberOfShards()).thenReturn(1); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperIntegrationTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperIntegrationTests.java index a66be9616600c..422e5dd0128ba 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperIntegrationTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authz/accesscontrol/SecurityIndexReaderWrapperIntegrationTests.java @@ -161,8 +161,9 @@ protected IndicesAccessControl getIndicesAccessControl() { when(queryShardContext.toQuery(new TermsQueryBuilder("field", values[i]))).thenReturn(parsedQuery); DirectoryReader wrappedDirectoryReader = wrapper.apply(directoryReader); - IndexSearcher indexSearcher = new ContextIndexSearcher(wrappedDirectoryReader, - IndexSearcher.getDefaultSimilarity(), IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()); + IndexSearcher indexSearcher = new ContextIndexSearcher( + wrappedDirectoryReader, IndexSearcher.getDefaultSimilarity(), IndexSearcher.getDefaultQueryCache(), + IndexSearcher.getDefaultQueryCachingPolicy(), true); int expectedHitCount = valuesHitCount[i]; logger.info("Going to verify hit count with query [{}] with expected total hits [{}]", parsedQuery.query(), expectedHitCount); @@ -270,8 +271,9 @@ protected IndicesAccessControl getIndicesAccessControl() { DirectoryReader directoryReader = ElasticsearchDirectoryReader.wrap(DirectoryReader.open(directory), shardId); DirectoryReader wrappedDirectoryReader = wrapper.apply(directoryReader); - IndexSearcher indexSearcher = new ContextIndexSearcher(wrappedDirectoryReader, - IndexSearcher.getDefaultSimilarity(), IndexSearcher.getDefaultQueryCache(), IndexSearcher.getDefaultQueryCachingPolicy()); + IndexSearcher indexSearcher = new ContextIndexSearcher( + wrappedDirectoryReader, IndexSearcher.getDefaultSimilarity(), IndexSearcher.getDefaultQueryCache(), + IndexSearcher.getDefaultQueryCachingPolicy(), true); ScoreDoc[] hits = indexSearcher.search(new MatchAllDocsQuery(), 1000).scoreDocs; Set actualDocIds = new HashSet<>(); From 8f62cd42f6d1c4be0f4bd1574a6edf14beddcfea Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Mon, 23 Mar 2020 17:53:26 +0100 Subject: [PATCH 2/3] revert throttling in compare --- .../elasticsearch/search/internal/ExitableDirectoryReader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java b/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java index 5306738f862ca..e6b5737480fa3 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java @@ -276,7 +276,7 @@ public void visit(int docID, byte[] packedValue) throws IOException { @Override public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { - checkAndThrowWithSampling(); + queryCancellation.checkCancelled(); return in.compare(minPackedValue, maxPackedValue); } From f32491949d699823e784d06f0a2b1da22056d2a8 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Mon, 23 Mar 2020 18:05:10 +0100 Subject: [PATCH 3/3] change to 8191 --- .../elasticsearch/search/internal/ExitableDirectoryReader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java b/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java index e6b5737480fa3..0a2dd476cf3b1 100644 --- a/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java +++ b/server/src/main/java/org/elasticsearch/search/internal/ExitableDirectoryReader.java @@ -245,7 +245,7 @@ public int getDocCount() { private static class ExitableIntersectVisitor implements PointValues.IntersectVisitor { - private static final int MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = (1 << 12) - 1; // 4095 + private static final int MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = (1 << 13) - 1; // 8191 private final PointValues.IntersectVisitor in; private final QueryCancellation queryCancellation;