Skip to content

Commit f0d778f

Browse files
committed
Check for query cancellation during rewrite
With ExitableDirectoryReader in place, check for query cancellation during QueryPhase#preProcess where the query rewriting takes place. Follows: #52822
1 parent f66fbb4 commit f0d778f

File tree

2 files changed

+70
-4
lines changed

2 files changed

+70
-4
lines changed

server/src/main/java/org/elasticsearch/search/query/QueryPhase.java

+18-4
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import org.apache.lucene.search.TopFieldDocs;
4747
import org.apache.lucene.search.TotalHits;
4848
import org.apache.lucene.search.Weight;
49-
import org.elasticsearch.action.search.SearchShardTask;
5049
import org.elasticsearch.common.Booleans;
5150
import org.elasticsearch.common.CheckedConsumer;
5251
import org.elasticsearch.common.lucene.Lucene;
@@ -110,7 +109,23 @@ public QueryPhase() {
110109

111110
@Override
112111
public void preProcess(SearchContext context) {
113-
context.preProcess(true);
112+
final Runnable cancellation;
113+
if (context.lowLevelCancellation()) {
114+
cancellation = context.searcher().addQueryCancellation(() -> {
115+
if (context.getTask().isCancelled()) {
116+
throw new TaskCancelledException("cancelled");
117+
}
118+
});
119+
} else {
120+
cancellation = null;
121+
}
122+
try {
123+
context.preProcess(true);
124+
} finally {
125+
if (cancellation != null) {
126+
context.searcher().removeQueryCancellation(cancellation);
127+
}
128+
}
114129
}
115130

116131
@Override
@@ -265,9 +280,8 @@ static boolean executeInternal(SearchContext searchContext) throws QueryPhaseExe
265280
}
266281

267282
if (searchContext.lowLevelCancellation()) {
268-
SearchShardTask task = searchContext.getTask();
269283
searcher.addQueryCancellation(() -> {
270-
if (task.isCancelled()) {
284+
if (searchContext.getTask().isCancelled()) {
271285
throw new TaskCancelledException("cancelled");
272286
}
273287
});

server/src/test/java/org/elasticsearch/search/query/QueryPhaseTests.java

+52
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@
5353
import org.apache.lucene.search.LeafCollector;
5454
import org.apache.lucene.search.MatchAllDocsQuery;
5555
import org.apache.lucene.search.MatchNoDocsQuery;
56+
import org.apache.lucene.search.MultiTermQuery;
57+
import org.apache.lucene.search.PrefixQuery;
5658
import org.apache.lucene.search.Query;
5759
import org.apache.lucene.search.ScoreDoc;
5860
import org.apache.lucene.search.Sort;
@@ -75,6 +77,7 @@
7577
import org.elasticsearch.index.mapper.MapperService;
7678
import org.elasticsearch.index.mapper.NumberFieldMapper;
7779
import org.elasticsearch.index.query.ParsedQuery;
80+
import org.elasticsearch.index.query.QueryShardContext;
7881
import org.elasticsearch.index.search.ESToParentBlockJoinQuery;
7982
import org.elasticsearch.index.shard.IndexShard;
8083
import org.elasticsearch.index.shard.IndexShardTestCase;
@@ -83,6 +86,7 @@
8386
import org.elasticsearch.search.internal.ScrollContext;
8487
import org.elasticsearch.search.internal.SearchContext;
8588
import org.elasticsearch.search.sort.SortAndFormats;
89+
import org.elasticsearch.tasks.TaskCancelledException;
8690
import org.elasticsearch.test.TestSearchContext;
8791

8892
import java.io.IOException;
@@ -825,7 +829,55 @@ public void testMinScore() throws Exception {
825829

826830
reader.close();
827831
dir.close();
832+
}
833+
834+
public void testCancellationDuringPreprocess() throws IOException {
835+
try (Directory dir = newDirectory();
836+
RandomIndexWriter w = new RandomIndexWriter(random(), dir, newIndexWriterConfig())) {
837+
838+
for (int i = 0; i < 10; i++) {
839+
Document doc = new Document();
840+
doc.add(new StringField("foo", "a".repeat(i), Store.NO));
841+
w.addDocument(doc);
842+
}
843+
w.flush();
844+
w.close();
845+
846+
IndexReader reader = DirectoryReader.open(dir);
847+
TestSearchContext context = new TestSearchContextWithRewriteAndCancellation(
848+
null, indexShard, newContextSearcher(reader));
849+
PrefixQuery prefixQuery = new PrefixQuery(new Term("foo", "a"));
850+
prefixQuery.setRewriteMethod(MultiTermQuery.SCORING_BOOLEAN_REWRITE);
851+
context.parsedQuery(new ParsedQuery(prefixQuery));
852+
SearchShardTask task = mock(SearchShardTask.class);
853+
when(task.isCancelled()).thenReturn(true);
854+
context.setTask(task);
855+
expectThrows(TaskCancelledException.class, () -> new QueryPhase().preProcess(context));
856+
857+
reader.close();
858+
}
859+
}
828860

861+
private static class TestSearchContextWithRewriteAndCancellation extends TestSearchContext {
862+
863+
public TestSearchContextWithRewriteAndCancellation(QueryShardContext queryShardContext, IndexShard indexShard,
864+
ContextIndexSearcher searcher) {
865+
super(queryShardContext, indexShard, searcher);
866+
}
867+
868+
@Override
869+
public void preProcess(boolean rewrite) {
870+
try {
871+
searcher().rewrite(query());
872+
} catch (IOException e) {
873+
fail("IOException shouldn't be thrown");
874+
}
875+
}
876+
877+
@Override
878+
public boolean lowLevelCancellation() {
879+
return true;
880+
}
829881
}
830882

831883
private static ContextIndexSearcher newContextSearcher(IndexReader reader) throws IOException {

0 commit comments

Comments
 (0)