mikemccand commented on code in PR #12345: URL: https://github.com/apache/lucene/pull/12345#discussion_r1478503121
########## lucene/core/src/java/org/apache/lucene/index/ExitableIndexReader.java: ########## @@ -0,0 +1,539 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.index; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.automaton.CompiledAutomaton; + +/** + * The {@link ExitableIndexReader} is used to timeout I/O operation which is done during query + * rewrite. After this time is exceeded, the search thread is stopped by throwing a {@link + * ExitableIndexReader.TimeExceededException} + */ +public final class ExitableIndexReader extends IndexReader { + private final IndexReader indexReader; + private final QueryTimeout queryTimeout; + + /** + * Create a ExitableIndexReader wrapper over another {@link IndexReader} with a specified timeout. + * + * @param indexReader the wrapped {@link IndexReader} + * @param queryTimeout max time allowed for collecting hits after which {@link + * ExitableIndexReader.TimeExceededException} is thrown + */ + public ExitableIndexReader(IndexReader indexReader, QueryTimeout queryTimeout) + throws IOException { + this.indexReader = new ExitableIndexReaderWrapper((DirectoryReader) indexReader, queryTimeout); + this.queryTimeout = queryTimeout; + } + + /** Returns queryTimeout instance. */ + public QueryTimeout getQueryTimeout() { + return queryTimeout; + } + + /** Thrown when elapsed search time exceeds allowed search time. */ + @SuppressWarnings("serial") + static class TimeExceededException extends RuntimeException { + private TimeExceededException() { + super("TimeLimit Exceeded"); + } + + private TimeExceededException(Exception e) { + super(e); + } + } + + @Override + public TermVectors termVectors() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.termVectors(); + } + + @Override + public int numDocs() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.numDocs(); + } + + @Override + public int maxDoc() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.maxDoc(); + } + + @Override + public StoredFields storedFields() throws IOException { + if (queryTimeout.shouldExit()) { Review Comment: Can we factor out a `private` helper method that does this `if` and the `throw`? Just like `ExitableDirectoryReader.checkAndThrow`. ########## lucene/core/src/java/org/apache/lucene/index/ExitableIndexReader.java: ########## @@ -0,0 +1,539 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.index; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.automaton.CompiledAutomaton; + +/** + * The {@link ExitableIndexReader} is used to timeout I/O operation which is done during query + * rewrite. After this time is exceeded, the search thread is stopped by throwing a {@link + * ExitableIndexReader.TimeExceededException} + */ +public final class ExitableIndexReader extends IndexReader { + private final IndexReader indexReader; + private final QueryTimeout queryTimeout; + + /** + * Create a ExitableIndexReader wrapper over another {@link IndexReader} with a specified timeout. + * + * @param indexReader the wrapped {@link IndexReader} + * @param queryTimeout max time allowed for collecting hits after which {@link + * ExitableIndexReader.TimeExceededException} is thrown + */ + public ExitableIndexReader(IndexReader indexReader, QueryTimeout queryTimeout) + throws IOException { + this.indexReader = new ExitableIndexReaderWrapper((DirectoryReader) indexReader, queryTimeout); Review Comment: Hmm I think we should not be casting incoming `indexReader` to a `DirectoryReader`? The idea of this new class is to enforce timeouts for any `IndexReader`, not just `DirectoryReader` (which is a subclass of `IndexReader`), I think? In fact, you might be able to start by copying `ExitableDirectoryReader.java` to `ExitableIndexReader.java` and then change all `DirectoryReader` to `IndexReader` and iterate from there? This way we would also get `points` timeout implemented. I realize this is all quite tricky!! ########## lucene/core/src/java/org/apache/lucene/index/ExitableIndexReader.java: ########## @@ -0,0 +1,539 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.index; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.automaton.CompiledAutomaton; + +/** + * The {@link ExitableIndexReader} is used to timeout I/O operation which is done during query + * rewrite. After this time is exceeded, the search thread is stopped by throwing a {@link + * ExitableIndexReader.TimeExceededException} + */ +public final class ExitableIndexReader extends IndexReader { + private final IndexReader indexReader; + private final QueryTimeout queryTimeout; + + /** + * Create a ExitableIndexReader wrapper over another {@link IndexReader} with a specified timeout. + * + * @param indexReader the wrapped {@link IndexReader} + * @param queryTimeout max time allowed for collecting hits after which {@link + * ExitableIndexReader.TimeExceededException} is thrown + */ + public ExitableIndexReader(IndexReader indexReader, QueryTimeout queryTimeout) + throws IOException { + this.indexReader = new ExitableIndexReaderWrapper((DirectoryReader) indexReader, queryTimeout); + this.queryTimeout = queryTimeout; + } + + /** Returns queryTimeout instance. */ + public QueryTimeout getQueryTimeout() { + return queryTimeout; + } + + /** Thrown when elapsed search time exceeds allowed search time. */ + @SuppressWarnings("serial") + static class TimeExceededException extends RuntimeException { + private TimeExceededException() { + super("TimeLimit Exceeded"); + } + + private TimeExceededException(Exception e) { + super(e); + } + } + + @Override + public TermVectors termVectors() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.termVectors(); + } + + @Override + public int numDocs() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.numDocs(); + } + + @Override + public int maxDoc() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.maxDoc(); + } + + @Override + public StoredFields storedFields() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.storedFields(); + } + + @Override + protected void doClose() throws IOException { + indexReader.doClose(); + } + + @Override + public IndexReaderContext getContext() { + + return indexReader.getContext(); + } + + @Override + public CacheHelper getReaderCacheHelper() { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getReaderCacheHelper(); + } + + @Override + public int docFreq(Term term) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.docFreq(term); + } + + @Override + public long totalTermFreq(Term term) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.totalTermFreq(term); + } + + @Override + public long getSumDocFreq(String field) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getSumDocFreq(field); + } + + @Override + public int getDocCount(String field) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getDocCount(field); + } + + @Override + public long getSumTotalTermFreq(String field) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getSumTotalTermFreq(field); + } + + private static class ExitableIndexReaderWrapper extends FilterDirectoryReader { Review Comment: We should not be subclassing `FilterDirectoryReader` here since we want to work with any `IndexReader`. ########## lucene/core/src/java/org/apache/lucene/index/ExitableIndexReader.java: ########## @@ -0,0 +1,539 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.index; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.automaton.CompiledAutomaton; + +/** + * The {@link ExitableIndexReader} is used to timeout I/O operation which is done during query + * rewrite. After this time is exceeded, the search thread is stopped by throwing a {@link + * ExitableIndexReader.TimeExceededException} + */ +public final class ExitableIndexReader extends IndexReader { + private final IndexReader indexReader; + private final QueryTimeout queryTimeout; + + /** + * Create a ExitableIndexReader wrapper over another {@link IndexReader} with a specified timeout. + * + * @param indexReader the wrapped {@link IndexReader} + * @param queryTimeout max time allowed for collecting hits after which {@link + * ExitableIndexReader.TimeExceededException} is thrown + */ + public ExitableIndexReader(IndexReader indexReader, QueryTimeout queryTimeout) + throws IOException { + this.indexReader = new ExitableIndexReaderWrapper((DirectoryReader) indexReader, queryTimeout); + this.queryTimeout = queryTimeout; + } + + /** Returns queryTimeout instance. */ + public QueryTimeout getQueryTimeout() { + return queryTimeout; + } + + /** Thrown when elapsed search time exceeds allowed search time. */ + @SuppressWarnings("serial") + static class TimeExceededException extends RuntimeException { + private TimeExceededException() { + super("TimeLimit Exceeded"); + } + + private TimeExceededException(Exception e) { + super(e); + } + } + + @Override + public TermVectors termVectors() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.termVectors(); + } + + @Override + public int numDocs() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.numDocs(); + } + + @Override + public int maxDoc() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.maxDoc(); + } + + @Override + public StoredFields storedFields() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.storedFields(); + } + + @Override + protected void doClose() throws IOException { + indexReader.doClose(); + } + + @Override + public IndexReaderContext getContext() { + + return indexReader.getContext(); Review Comment: I think this isn't quite right -- we can't just return the underlying `IndexReaderContext`. I think instead you need to get this underlying context, and then make a new `CompositeReaderContext` that wraps each of the leaf readers wrapped in `ExitableFilterAtomicReader`, and children wrapped with this new class (`ExitableIndexReader`)? ########## lucene/core/src/java/org/apache/lucene/index/ExitableIndexReader.java: ########## @@ -0,0 +1,539 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.index; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.automaton.CompiledAutomaton; + +/** + * The {@link ExitableIndexReader} is used to timeout I/O operation which is done during query + * rewrite. After this time is exceeded, the search thread is stopped by throwing a {@link + * ExitableIndexReader.TimeExceededException} + */ +public final class ExitableIndexReader extends IndexReader { + private final IndexReader indexReader; + private final QueryTimeout queryTimeout; + + /** + * Create a ExitableIndexReader wrapper over another {@link IndexReader} with a specified timeout. + * + * @param indexReader the wrapped {@link IndexReader} + * @param queryTimeout max time allowed for collecting hits after which {@link + * ExitableIndexReader.TimeExceededException} is thrown + */ + public ExitableIndexReader(IndexReader indexReader, QueryTimeout queryTimeout) + throws IOException { + this.indexReader = new ExitableIndexReaderWrapper((DirectoryReader) indexReader, queryTimeout); + this.queryTimeout = queryTimeout; + } + + /** Returns queryTimeout instance. */ + public QueryTimeout getQueryTimeout() { + return queryTimeout; + } + + /** Thrown when elapsed search time exceeds allowed search time. */ + @SuppressWarnings("serial") + static class TimeExceededException extends RuntimeException { + private TimeExceededException() { + super("TimeLimit Exceeded"); + } + + private TimeExceededException(Exception e) { + super(e); + } + } + + @Override + public TermVectors termVectors() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.termVectors(); + } + + @Override + public int numDocs() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.numDocs(); + } + + @Override + public int maxDoc() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.maxDoc(); + } + + @Override + public StoredFields storedFields() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.storedFields(); + } + + @Override + protected void doClose() throws IOException { + indexReader.doClose(); + } + + @Override + public IndexReaderContext getContext() { + + return indexReader.getContext(); + } + + @Override + public CacheHelper getReaderCacheHelper() { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getReaderCacheHelper(); + } + + @Override + public int docFreq(Term term) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.docFreq(term); + } + + @Override + public long totalTermFreq(Term term) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.totalTermFreq(term); + } + + @Override + public long getSumDocFreq(String field) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getSumDocFreq(field); + } + + @Override + public int getDocCount(String field) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getDocCount(field); + } + + @Override + public long getSumTotalTermFreq(String field) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getSumTotalTermFreq(field); + } + + private static class ExitableIndexReaderWrapper extends FilterDirectoryReader { + private final QueryTimeout queryTimeout; + + private final CacheHelper readerCacheHelper; + + public ExitableIndexReaderWrapper(DirectoryReader in, QueryTimeout queryTimeout) Review Comment: And maybe reuse the existing `ExitableDirectoryReader.ExitableSubReaderWrapper` instead of making a new one here? ########## lucene/facet/src/test/org/apache/lucene/facet/TestStringValueFacetCounts.java: ########## @@ -77,7 +78,11 @@ public void testBasicSingleValued() throws Exception { new StringDocValuesReaderState(searcher.getIndexReader(), "field"); checkTopNFacetResult(expectedCounts, expectedTotalDocCount, searcher, state, 10, 2, 1, 0); - IOUtils.close(searcher.getIndexReader(), dir); + IndexReader r = searcher.getIndexReader(); + if (r instanceof ExitableIndexReader) { Review Comment: Hmm why was this needed? In general users of `ExitableIndexReader` should not have to special case which exact instance of `IndexReader` they have. In this case, maybe `ExitableIndexReader` is not implementing `close` correctly? ########## lucene/core/src/java/org/apache/lucene/index/ExitableIndexReader.java: ########## @@ -0,0 +1,539 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.index; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.automaton.CompiledAutomaton; + +/** + * The {@link ExitableIndexReader} is used to timeout I/O operation which is done during query + * rewrite. After this time is exceeded, the search thread is stopped by throwing a {@link + * ExitableIndexReader.TimeExceededException} + */ +public final class ExitableIndexReader extends IndexReader { + private final IndexReader indexReader; + private final QueryTimeout queryTimeout; + + /** + * Create a ExitableIndexReader wrapper over another {@link IndexReader} with a specified timeout. + * + * @param indexReader the wrapped {@link IndexReader} + * @param queryTimeout max time allowed for collecting hits after which {@link + * ExitableIndexReader.TimeExceededException} is thrown + */ + public ExitableIndexReader(IndexReader indexReader, QueryTimeout queryTimeout) + throws IOException { + this.indexReader = new ExitableIndexReaderWrapper((DirectoryReader) indexReader, queryTimeout); + this.queryTimeout = queryTimeout; + } + + /** Returns queryTimeout instance. */ + public QueryTimeout getQueryTimeout() { + return queryTimeout; + } + + /** Thrown when elapsed search time exceeds allowed search time. */ + @SuppressWarnings("serial") + static class TimeExceededException extends RuntimeException { + private TimeExceededException() { + super("TimeLimit Exceeded"); + } + + private TimeExceededException(Exception e) { + super(e); + } + } + + @Override + public TermVectors termVectors() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.termVectors(); + } + + @Override + public int numDocs() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.numDocs(); + } + + @Override + public int maxDoc() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.maxDoc(); + } + + @Override + public StoredFields storedFields() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.storedFields(); + } + + @Override + protected void doClose() throws IOException { + indexReader.doClose(); + } + + @Override + public IndexReaderContext getContext() { + + return indexReader.getContext(); + } + + @Override + public CacheHelper getReaderCacheHelper() { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getReaderCacheHelper(); + } + + @Override + public int docFreq(Term term) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.docFreq(term); + } + + @Override + public long totalTermFreq(Term term) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.totalTermFreq(term); + } + + @Override + public long getSumDocFreq(String field) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getSumDocFreq(field); + } + + @Override + public int getDocCount(String field) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getDocCount(field); + } + + @Override + public long getSumTotalTermFreq(String field) throws IOException { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getSumTotalTermFreq(field); + } + + private static class ExitableIndexReaderWrapper extends FilterDirectoryReader { + private final QueryTimeout queryTimeout; + + private final CacheHelper readerCacheHelper; + + public ExitableIndexReaderWrapper(DirectoryReader in, QueryTimeout queryTimeout) + throws IOException { + this( + in, + new ExitableIndexReaderWrapper.ExitableIndexSubReaderWrapper( + Collections.emptyMap(), queryTimeout), + queryTimeout); + } + + /** + * Create a new ExitableIndexReaderWrapper that filters a passed in DirectoryReader, using the + * supplied SubReaderWrapper to wrap its subreader. + * + * @param in the DirectoryReader to filter + * @param wrapper the SubReaderWrapper to use to wrap subreaders + */ + private ExitableIndexReaderWrapper( + DirectoryReader in, SubReaderWrapper wrapper, QueryTimeout queryTimeout) + throws IOException { + super(in, wrapper); + this.queryTimeout = queryTimeout; + readerCacheHelper = + in.getReaderCacheHelper() == null + ? null + : new DelegatingCacheHelper(in.getReaderCacheHelper()); + } + + @Override + protected DirectoryReader doWrapDirectoryReader(DirectoryReader in) throws IOException { + Map<CacheKey, LeafReader> readerCache = new HashMap<>(); + for (LeafReader reader : getSequentialSubReaders()) { + // we try to reuse the life docs instances here if the reader cache key didn't change + if (reader instanceof ExitableIndexReaderWrapper.TimeoutLeafReader + && reader.getReaderCacheHelper() != null) { + readerCache.put( + ((ExitableIndexReaderWrapper.TimeoutLeafReader) reader) + .getReaderCacheHelper() + .getKey(), + reader); + } + } + return new ExitableIndexReaderWrapper( + in, + new ExitableIndexReaderWrapper.ExitableIndexSubReaderWrapper(readerCache, queryTimeout), + queryTimeout); + } + + private static class ExitableIndexSubReaderWrapper extends SubReaderWrapper { + private final Map<CacheKey, LeafReader> mapping; + private final QueryTimeout queryTimeout; + + public ExitableIndexSubReaderWrapper( + Map<CacheKey, LeafReader> oldReadersCache, QueryTimeout queryTimeout) { + assert oldReadersCache != null; + this.mapping = oldReadersCache; + this.queryTimeout = queryTimeout; + } + + @Override + protected LeafReader[] wrap(List<? extends LeafReader> readers) { + List<LeafReader> wrapped = new ArrayList<>(readers.size()); + for (LeafReader reader : readers) { + LeafReader wrap = wrap(reader); + assert wrap != null; + if (wrap.numDocs() != 0) { + wrapped.add(wrap); + } + } + return wrapped.toArray(new LeafReader[0]); + } + + @Override + public LeafReader wrap(LeafReader reader) { + CacheHelper readerCacheHelper = reader.getReaderCacheHelper(); + if (readerCacheHelper != null && mapping.containsKey(readerCacheHelper.getKey())) { + // if the reader cache helper didn't change and we have it in the cache don't bother + // creating a new one + return mapping.get(readerCacheHelper.getKey()); + } + return new TimeoutLeafReader(reader, queryTimeout); + } + } + + @Override + public CacheHelper getReaderCacheHelper() { + return readerCacheHelper; + } + + /** + * TimeoutLeafReader is wrapper class for FilterLeafReader which is imposing timeout on + * different operations of FilterLeafReader + */ + private static class TimeoutLeafReader extends FilterLeafReader { Review Comment: I think you could reuse the existing `ExitableDirectoryReader.ExitableFilterAtomicReader` class, instead of making a new class here, maybe? And that class already implements timeouts for points, postings, doc values, etc. ########## lucene/core/src/java/org/apache/lucene/index/ExitableIndexReader.java: ########## @@ -0,0 +1,539 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.index; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.automaton.CompiledAutomaton; + +/** + * The {@link ExitableIndexReader} is used to timeout I/O operation which is done during query + * rewrite. After this time is exceeded, the search thread is stopped by throwing a {@link + * ExitableIndexReader.TimeExceededException} + */ +public final class ExitableIndexReader extends IndexReader { + private final IndexReader indexReader; + private final QueryTimeout queryTimeout; + + /** + * Create a ExitableIndexReader wrapper over another {@link IndexReader} with a specified timeout. + * + * @param indexReader the wrapped {@link IndexReader} + * @param queryTimeout max time allowed for collecting hits after which {@link + * ExitableIndexReader.TimeExceededException} is thrown + */ + public ExitableIndexReader(IndexReader indexReader, QueryTimeout queryTimeout) + throws IOException { + this.indexReader = new ExitableIndexReaderWrapper((DirectoryReader) indexReader, queryTimeout); + this.queryTimeout = queryTimeout; + } + + /** Returns queryTimeout instance. */ + public QueryTimeout getQueryTimeout() { + return queryTimeout; + } + + /** Thrown when elapsed search time exceeds allowed search time. */ + @SuppressWarnings("serial") + static class TimeExceededException extends RuntimeException { Review Comment: I think we should have both `ExitableDirectoryReader` and this class to use a single class (the existing `ExitingReaderException`) when timeout happens? ########## lucene/core/src/java/org/apache/lucene/index/ExitableIndexReader.java: ########## @@ -0,0 +1,539 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.index; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import org.apache.lucene.util.Bits; +import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.automaton.CompiledAutomaton; + +/** + * The {@link ExitableIndexReader} is used to timeout I/O operation which is done during query + * rewrite. After this time is exceeded, the search thread is stopped by throwing a {@link + * ExitableIndexReader.TimeExceededException} + */ +public final class ExitableIndexReader extends IndexReader { + private final IndexReader indexReader; + private final QueryTimeout queryTimeout; + + /** + * Create a ExitableIndexReader wrapper over another {@link IndexReader} with a specified timeout. + * + * @param indexReader the wrapped {@link IndexReader} + * @param queryTimeout max time allowed for collecting hits after which {@link + * ExitableIndexReader.TimeExceededException} is thrown + */ + public ExitableIndexReader(IndexReader indexReader, QueryTimeout queryTimeout) + throws IOException { + this.indexReader = new ExitableIndexReaderWrapper((DirectoryReader) indexReader, queryTimeout); + this.queryTimeout = queryTimeout; + } + + /** Returns queryTimeout instance. */ + public QueryTimeout getQueryTimeout() { + return queryTimeout; + } + + /** Thrown when elapsed search time exceeds allowed search time. */ + @SuppressWarnings("serial") + static class TimeExceededException extends RuntimeException { + private TimeExceededException() { + super("TimeLimit Exceeded"); + } + + private TimeExceededException(Exception e) { + super(e); + } + } + + @Override + public TermVectors termVectors() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.termVectors(); + } + + @Override + public int numDocs() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.numDocs(); + } + + @Override + public int maxDoc() { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.maxDoc(); + } + + @Override + public StoredFields storedFields() throws IOException { + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.storedFields(); + } + + @Override + protected void doClose() throws IOException { + indexReader.doClose(); + } + + @Override + public IndexReaderContext getContext() { + + return indexReader.getContext(); + } + + @Override + public CacheHelper getReaderCacheHelper() { + + if (queryTimeout.shouldExit()) { + throw new ExitableIndexReader.TimeExceededException(); + } + return indexReader.getReaderCacheHelper(); + } + + @Override + public int docFreq(Term term) throws IOException { + Review Comment: Maybe remove this newline at the top of each of these methods? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
