From 6aca2ab24f55604a74baa0cf7ec1459640098d59 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 13:53:21 +0100 Subject: [PATCH 01/15] LUCENE-8026: ExitableDirectoryReader does not instrument points --- .../lucene/index/ExitableDirectoryReader.java | 95 ++++++++++++++++++- .../index/TestExitableDirectoryReader.java | 79 ++++++++++++++- 2 files changed, 170 insertions(+), 4 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index fa1f6bae38ee..027853580675 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -77,6 +77,15 @@ public ExitableFilterAtomicReader(LeafReader in, QueryTimeout queryTimeout) { this.queryTimeout = queryTimeout; } + @Override + public PointValues getPointValues(String field) throws IOException { + final PointValues pointValues = in.getPointValues(field); + if (pointValues == null) { + return null; + } + return (queryTimeout.isTimeoutEnabled()) ? new ExitablePointValues(pointValues, queryTimeout) : pointValues; + } + @Override public Terms terms(String field) throws IOException { Terms terms = in.terms(field); @@ -100,13 +109,97 @@ public CacheHelper getCoreCacheHelper() { } + /** + * Wrapper class for another PointValues implementation that is used by ExitableFields. + */ + public static class ExitablePointValues extends PointValues { + + private final PointValues in; + private final QueryTimeout queryTimeout; + + public ExitablePointValues(PointValues in, QueryTimeout queryTimeout) { + this.in = in; + this.queryTimeout = queryTimeout; + checkAndThrow(); + } + + /** + * Throws {@link ExitingReaderException} if {@link QueryTimeout#shouldExit()} returns true, + * or if {@link Thread#interrupted()} returns true. + */ + private void checkAndThrow() { + if (queryTimeout.shouldExit()) { + throw new ExitingReaderException("The request took too long to iterate over terms. Timeout: " + + queryTimeout.toString() + + ", PointValues=" + in + ); + } else if (Thread.interrupted()) { + throw new ExitingReaderException("Interrupted while iterating over terms. PointValues=" + in); + } + } + + @Override + public void intersect(IntersectVisitor visitor) throws IOException { + checkAndThrow(); + in.intersect(visitor); + } + + @Override + public long estimatePointCount(IntersectVisitor visitor) { + checkAndThrow(); + return in.estimatePointCount(visitor); + } + + @Override + public byte[] getMinPackedValue() throws IOException { + checkAndThrow(); + return in.getMinPackedValue(); + } + + @Override + public byte[] getMaxPackedValue() throws IOException { + checkAndThrow(); + return in.getMaxPackedValue(); + } + + @Override + public int getNumDataDimensions() throws IOException { + checkAndThrow(); + return in.getNumDataDimensions(); + } + + @Override + public int getNumIndexDimensions() throws IOException { + checkAndThrow(); + return in.getNumIndexDimensions(); + } + + @Override + public int getBytesPerDimension() throws IOException { + checkAndThrow(); + return in.getBytesPerDimension(); + } + + @Override + public long size() { + checkAndThrow(); + return in.size(); + } + + @Override + public int getDocCount() { + checkAndThrow(); + return in.getDocCount(); + } + } + /** * Wrapper class for another Terms implementation that is used by ExitableFields. */ public static class ExitableTerms extends FilterTerms { private QueryTimeout queryTimeout; - + /** Constructor **/ public ExitableTerms(Terms terms, QueryTimeout queryTimeout) { super(terms); diff --git a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java index 14b200230a60..5ee2b556c747 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java @@ -21,6 +21,7 @@ import org.apache.lucene.analysis.MockAnalyzer; import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; +import org.apache.lucene.document.IntPoint; import org.apache.lucene.index.ExitableDirectoryReader.ExitingReaderException; import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.PrefixQuery; @@ -93,7 +94,7 @@ public CacheHelper getReaderCacheHelper() { * @throws Exception on error */ @Ignore("this test relies on wall clock time and sometimes false fails") - public void testExitableFilterIndexReader() throws Exception { + public void testExitableFilterTermsIndexReader() throws Exception { Directory directory = newDirectory(); IndexWriter writer = new IndexWriter(directory, newIndexWriterConfig(new MockAnalyzer(random()))); @@ -108,8 +109,8 @@ public void testExitableFilterIndexReader() throws Exception { Document d3 = new Document(); d3.add(newTextField("default", "ones two four", Field.Store.YES)); writer.addDocument(d3); - writer.forceMerge(1); + writer.forceMerge(1); writer.commit(); writer.close(); @@ -129,7 +130,6 @@ public void testExitableFilterIndexReader() throws Exception { searcher.search(query, 10); reader.close(); - // Set a really low timeout value (1 millisecond) and expect an Exception directoryReader = DirectoryReader.open(directory); exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(1)); @@ -160,5 +160,78 @@ public void testExitableFilterIndexReader() throws Exception { directory.close(); } + + /** + * Tests timing out of PointValues queries + * + * @throws Exception on error + */ + @Ignore("this test relies on wall clock time and sometimes false fails") + public void testExitablePointValuesIndexReader() throws Exception { + Directory directory = newDirectory(); + IndexWriter writer = new IndexWriter(directory, newIndexWriterConfig(new MockAnalyzer(random()))); + + Document d1 = new Document(); + d1.add(new IntPoint("default", 10)); + writer.addDocument(d1); + + Document d2 = new Document(); + d2.add(new IntPoint("default", 100)); + writer.addDocument(d2); + + Document d3 = new Document(); + d3.add(new IntPoint("default", 1000)); + writer.addDocument(d3); + + writer.forceMerge(1); + writer.commit(); + writer.close(); + + DirectoryReader directoryReader; + DirectoryReader exitableDirectoryReader; + IndexReader reader; + IndexSearcher searcher; + + Query query = IntPoint.newRangeQuery("default", 10, 20); + + // Set a fairly high timeout value (1 second) and expect the query to complete in that time frame. + // Not checking the validity of the result, all we are bothered about in this test is the timing out. + directoryReader = DirectoryReader.open(directory); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(1000)); + reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); + searcher = new IndexSearcher(reader); + searcher.search(query, 10); + reader.close(); + + // Set a really low timeout value (1 millisecond) and expect an Exception + directoryReader = DirectoryReader.open(directory); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(1)); + reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); + IndexSearcher slowSearcher = new IndexSearcher(reader); + expectThrows(ExitingReaderException.class, () -> { + slowSearcher.search(query, 10); + }); + reader.close(); + + // Set maximum time out and expect the query to complete. + // Not checking the validity of the result, all we are bothered about in this test is the timing out. + directoryReader = DirectoryReader.open(directory); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(Long.MAX_VALUE)); + reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); + searcher = new IndexSearcher(reader); + searcher.search(query, 10); + reader.close(); + + // Set a negative time allowed and expect the query to complete (should disable timeouts) + // Not checking the validity of the result, all we are bothered about in this test is the timing out. + directoryReader = DirectoryReader.open(directory); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(-189034L)); + reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); + searcher = new IndexSearcher(reader); + searcher.search(query, 10); + reader.close(); + + directory.close(); + } } From 6c6e45be3b645814b9871db3b33b799a7c31296d Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 15:03:31 +0100 Subject: [PATCH 02/15] LUCENE-8463: Fix typo in exception messages --- .../java/org/apache/lucene/index/ExitableDirectoryReader.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index 027853580675..32014329d9fa 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -129,12 +129,12 @@ public ExitablePointValues(PointValues in, QueryTimeout queryTimeout) { */ private void checkAndThrow() { if (queryTimeout.shouldExit()) { - throw new ExitingReaderException("The request took too long to iterate over terms. Timeout: " + throw new ExitingReaderException("The request took too long to iterate over point values. Timeout: " + queryTimeout.toString() + ", PointValues=" + in ); } else if (Thread.interrupted()) { - throw new ExitingReaderException("Interrupted while iterating over terms. PointValues=" + in); + throw new ExitingReaderException("Interrupted while iterating over point values. PointValues=" + in); } } From 5587aa2c904ba135da11a89f47cfa035901b046d Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 15:19:07 +0100 Subject: [PATCH 03/15] LUCENE-8026: Add ExitableIntersectVisitor support with sampling --- .../lucene/index/ExitableDirectoryReader.java | 62 ++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index 32014329d9fa..e67f08719210 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -141,7 +141,7 @@ private void checkAndThrow() { @Override public void intersect(IntersectVisitor visitor) throws IOException { checkAndThrow(); - in.intersect(visitor); + in.intersect(new ExitableIntersectVisitor(visitor, queryTimeout)); } @Override @@ -193,6 +193,66 @@ public int getDocCount() { } } + public static class ExitableIntersectVisitor implements PointValues.IntersectVisitor { + + public static final int DEFAULT_MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = 10; + + private final PointValues.IntersectVisitor in; + private final QueryTimeout queryTimeout; + private final int maxCallsBeforeQueryTimeoutCheck; + private int calls = 0; + + public ExitableIntersectVisitor(PointValues.IntersectVisitor in, QueryTimeout queryTimeout) { + this(in, queryTimeout, DEFAULT_MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK); + } + + public ExitableIntersectVisitor(PointValues.IntersectVisitor in, + QueryTimeout queryTimeout, int maxCallsBeforeQueryTimeoutCheck) { + this.in = in; + this.queryTimeout = queryTimeout; + this.maxCallsBeforeQueryTimeoutCheck = maxCallsBeforeQueryTimeoutCheck; + } + + /** + * Throws {@link ExitingReaderException} if {@link QueryTimeout#shouldExit()} returns true, + * or if {@link Thread#interrupted()} returns true. + */ + private void checkAndThrow() { + if (calls++ % maxCallsBeforeQueryTimeoutCheck == 0 && queryTimeout.shouldExit()) { + throw new ExitingReaderException("The request took too long to intersect point values. Timeout: " + + queryTimeout.toString() + + ", PointValues=" + in + ); + } else if (Thread.interrupted()) { + throw new ExitingReaderException("Interrupted while intersecting point values. PointValues=" + in); + } + } + + @Override + public void visit(int docID) throws IOException { + checkAndThrow(); + in.visit(docID); + } + + @Override + public void visit(int docID, byte[] packedValue) throws IOException { + checkAndThrow(); + in.visit(docID, packedValue); + } + + @Override + public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { + checkAndThrow(); + return in.compare(minPackedValue, maxPackedValue); + } + + @Override + public void grow(int count) { + checkAndThrow(); + in.grow(count); + } + } + /** * Wrapper class for another Terms implementation that is used by ExitableFields. */ From 2b3dfbcba8944dd48c22c341699da528c8525f91 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 15:32:57 +0100 Subject: [PATCH 04/15] LUCENE-8026: Mock query timeout to enabled tests --- .../index/TestExitableDirectoryReader.java | 64 +++++++++++++++---- 1 file changed, 53 insertions(+), 11 deletions(-) diff --git a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java index 5ee2b556c747..1103fdab3bd6 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java @@ -29,7 +29,6 @@ import org.apache.lucene.store.Directory; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.LuceneTestCase; -import org.junit.Ignore; /** * Test that uses a default/lucene Implementation of {@link QueryTimeout} @@ -93,7 +92,6 @@ public CacheHelper getReaderCacheHelper() { * Tests timing out of TermsEnum iterations * @throws Exception on error */ - @Ignore("this test relies on wall clock time and sometimes false fails") public void testExitableFilterTermsIndexReader() throws Exception { Directory directory = newDirectory(); IndexWriter writer = new IndexWriter(directory, newIndexWriterConfig(new MockAnalyzer(random()))); @@ -124,7 +122,7 @@ public void testExitableFilterTermsIndexReader() throws Exception { // Set a fairly high timeout value (1 second) and expect the query to complete in that time frame. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(1000)); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -132,7 +130,7 @@ public void testExitableFilterTermsIndexReader() throws Exception { // Set a really low timeout value (1 millisecond) and expect an Exception directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(1)); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, immediateQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); IndexSearcher slowSearcher = new IndexSearcher(reader); expectThrows(ExitingReaderException.class, () -> { @@ -143,7 +141,7 @@ public void testExitableFilterTermsIndexReader() throws Exception { // Set maximum time out and expect the query to complete. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(Long.MAX_VALUE)); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -152,7 +150,7 @@ public void testExitableFilterTermsIndexReader() throws Exception { // Set a negative time allowed and expect the query to complete (should disable timeouts) // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(-189034L)); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, disabledQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -166,7 +164,6 @@ public void testExitableFilterTermsIndexReader() throws Exception { * * @throws Exception on error */ - @Ignore("this test relies on wall clock time and sometimes false fails") public void testExitablePointValuesIndexReader() throws Exception { Directory directory = newDirectory(); IndexWriter writer = new IndexWriter(directory, newIndexWriterConfig(new MockAnalyzer(random()))); @@ -197,7 +194,7 @@ public void testExitablePointValuesIndexReader() throws Exception { // Set a fairly high timeout value (1 second) and expect the query to complete in that time frame. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(1000)); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -205,7 +202,7 @@ public void testExitablePointValuesIndexReader() throws Exception { // Set a really low timeout value (1 millisecond) and expect an Exception directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(1)); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, immediateQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); IndexSearcher slowSearcher = new IndexSearcher(reader); expectThrows(ExitingReaderException.class, () -> { @@ -216,7 +213,7 @@ public void testExitablePointValuesIndexReader() throws Exception { // Set maximum time out and expect the query to complete. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(Long.MAX_VALUE)); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -225,7 +222,7 @@ public void testExitablePointValuesIndexReader() throws Exception { // Set a negative time allowed and expect the query to complete (should disable timeouts) // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, new QueryTimeoutImpl(-189034L)); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, disabledQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -233,5 +230,50 @@ public void testExitablePointValuesIndexReader() throws Exception { directory.close(); } + + private static QueryTimeout disabledQueryTimeout() { + return new QueryTimeout() { + + @Override + public boolean shouldExit() { + return false; + } + + @Override + public boolean isTimeoutEnabled() { + return false; + } + }; + } + + private static QueryTimeout inifiniteQueryTimeout() { + return new QueryTimeout() { + + @Override + public boolean shouldExit() { + return false; + } + + @Override + public boolean isTimeoutEnabled() { + return true; + } + }; + } + + private static QueryTimeout immediateQueryTimeout() { + return new QueryTimeout() { + + @Override + public boolean shouldExit() { + return true; + } + + @Override + public boolean isTimeoutEnabled() { + return true; + } + }; + } } From dfd1403e02b270ba0712c0a40ee14dbf9e7719fd Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 20:09:54 +0100 Subject: [PATCH 05/15] LUCENE-8026: Make ExitableDirectoryReader not configurable --- .../apache/lucene/index/ExitableDirectoryReader.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index e67f08719210..6716e92ae0d0 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -195,22 +195,15 @@ public int getDocCount() { public static class ExitableIntersectVisitor implements PointValues.IntersectVisitor { - public static final int DEFAULT_MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = 10; + public static final int MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = 10; private final PointValues.IntersectVisitor in; private final QueryTimeout queryTimeout; - private final int maxCallsBeforeQueryTimeoutCheck; private int calls = 0; public ExitableIntersectVisitor(PointValues.IntersectVisitor in, QueryTimeout queryTimeout) { - this(in, queryTimeout, DEFAULT_MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK); - } - - public ExitableIntersectVisitor(PointValues.IntersectVisitor in, - QueryTimeout queryTimeout, int maxCallsBeforeQueryTimeoutCheck) { this.in = in; this.queryTimeout = queryTimeout; - this.maxCallsBeforeQueryTimeoutCheck = maxCallsBeforeQueryTimeoutCheck; } /** @@ -218,7 +211,7 @@ public ExitableIntersectVisitor(PointValues.IntersectVisitor in, * or if {@link Thread#interrupted()} returns true. */ private void checkAndThrow() { - if (calls++ % maxCallsBeforeQueryTimeoutCheck == 0 && queryTimeout.shouldExit()) { + if (calls++ % MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK == 0 && queryTimeout.shouldExit()) { throw new ExitingReaderException("The request took too long to intersect point values. Timeout: " + queryTimeout.toString() + ", PointValues=" + in From 9e11f76b2ac7b9cda9f1c78ad0c1f3a5be09f435 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 20:11:10 +0100 Subject: [PATCH 06/15] LUCENE-8026: Fix "checkAndThrow" outer condition --- .../lucene/index/ExitableDirectoryReader.java | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index 6716e92ae0d0..88a9b90b9a85 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -211,13 +211,15 @@ public ExitableIntersectVisitor(PointValues.IntersectVisitor in, QueryTimeout qu * or if {@link Thread#interrupted()} returns true. */ private void checkAndThrow() { - if (calls++ % MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK == 0 && queryTimeout.shouldExit()) { - throw new ExitingReaderException("The request took too long to intersect point values. Timeout: " - + queryTimeout.toString() - + ", PointValues=" + in - ); - } else if (Thread.interrupted()) { - throw new ExitingReaderException("Interrupted while intersecting point values. PointValues=" + in); + if (calls++ % MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK == 0) { + if (queryTimeout.shouldExit()) { + throw new ExitingReaderException("The request took too long to intersect point values. Timeout: " + + queryTimeout.toString() + + ", PointValues=" + in + ); + } else if (Thread.interrupted()) { + throw new ExitingReaderException("Interrupted while intersecting point values. PointValues=" + in); + } } } From 3d09426a81277a5344435f43a1942570c1a85f37 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 20:12:31 +0100 Subject: [PATCH 07/15] LUCENE-8026: Sample ExitableIntersectVisitor#visit calls only --- .../java/org/apache/lucene/index/ExitableDirectoryReader.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index 88a9b90b9a85..8627fa2f5ce4 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -237,13 +237,11 @@ public void visit(int docID, byte[] packedValue) throws IOException { @Override public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { - checkAndThrow(); return in.compare(minPackedValue, maxPackedValue); } @Override public void grow(int count) { - checkAndThrow(); in.grow(count); } } From 45996c885d3e52edcc21d47e40db443895982af5 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 20:13:35 +0100 Subject: [PATCH 08/15] LUCENE-8026: Fix query timeout related comment --- .../org/apache/lucene/index/TestExitableDirectoryReader.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java index 1103fdab3bd6..a67247fcef38 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java @@ -128,7 +128,7 @@ public void testExitableFilterTermsIndexReader() throws Exception { searcher.search(query, 10); reader.close(); - // Set a really low timeout value (1 millisecond) and expect an Exception + // Set a really low timeout value (immediate) and expect an Exception directoryReader = DirectoryReader.open(directory); exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, immediateQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); @@ -200,7 +200,7 @@ public void testExitablePointValuesIndexReader() throws Exception { searcher.search(query, 10); reader.close(); - // Set a really low timeout value (1 millisecond) and expect an Exception + // Set a really low timeout value (immediate) and expect an Exception directoryReader = DirectoryReader.open(directory); exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, immediateQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); From 0b8ecd1004315743f4ee6dde9218931fd369b981 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 20:21:16 +0100 Subject: [PATCH 09/15] LUCENE-8026: Fix query timeout related comment --- .../org/apache/lucene/index/TestExitableDirectoryReader.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java index a67247fcef38..1401fb7ac5b6 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java @@ -119,7 +119,7 @@ public void testExitableFilterTermsIndexReader() throws Exception { Query query = new PrefixQuery(new Term("default", "o")); - // Set a fairly high timeout value (1 second) and expect the query to complete in that time frame. + // Set a fairly high timeout value (infinite) and expect the query to complete in that time frame. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); @@ -191,7 +191,7 @@ public void testExitablePointValuesIndexReader() throws Exception { Query query = IntPoint.newRangeQuery("default", 10, 20); - // Set a fairly high timeout value (1 second) and expect the query to complete in that time frame. + // Set a fairly high timeout value (infinite) and expect the query to complete in that time frame. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); From 3e5b5627fa0a55ec037462ade3c8b12a1a73dde5 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Mon, 12 Nov 2018 20:28:33 +0100 Subject: [PATCH 10/15] LUCENE-8026: Fix typo in query timeout mock API --- .../lucene/index/TestExitableDirectoryReader.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java index 1401fb7ac5b6..6ca9800c973e 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java @@ -122,7 +122,7 @@ public void testExitableFilterTermsIndexReader() throws Exception { // Set a fairly high timeout value (infinite) and expect the query to complete in that time frame. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, infiniteQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -141,7 +141,7 @@ public void testExitableFilterTermsIndexReader() throws Exception { // Set maximum time out and expect the query to complete. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, infiniteQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -194,7 +194,7 @@ public void testExitablePointValuesIndexReader() throws Exception { // Set a fairly high timeout value (infinite) and expect the query to complete in that time frame. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, infiniteQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -213,7 +213,7 @@ public void testExitablePointValuesIndexReader() throws Exception { // Set maximum time out and expect the query to complete. // Not checking the validity of the result, all we are bothered about in this test is the timing out. directoryReader = DirectoryReader.open(directory); - exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, inifiniteQueryTimeout()); + exitableDirectoryReader = new ExitableDirectoryReader(directoryReader, infiniteQueryTimeout()); reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader)); searcher = new IndexSearcher(reader); searcher.search(query, 10); @@ -246,7 +246,7 @@ public boolean isTimeoutEnabled() { }; } - private static QueryTimeout inifiniteQueryTimeout() { + private static QueryTimeout infiniteQueryTimeout() { return new QueryTimeout() { @Override From 11fd3de51c31ef4c0e2a660ce22df018d194ce7e Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Tue, 13 Nov 2018 09:57:20 +0100 Subject: [PATCH 11/15] LUCENE-8026: Remove unnecessary integer initialization --- .../java/org/apache/lucene/index/ExitableDirectoryReader.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index 8627fa2f5ce4..715975cf924a 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -199,7 +199,7 @@ public static class ExitableIntersectVisitor implements PointValues.IntersectVis private final PointValues.IntersectVisitor in; private final QueryTimeout queryTimeout; - private int calls = 0; + private int calls; public ExitableIntersectVisitor(PointValues.IntersectVisitor in, QueryTimeout queryTimeout) { this.in = in; From 7eef7e9a8213714bdc3a90aa7fa5c5db010d5151 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Tue, 13 Nov 2018 18:33:27 +0100 Subject: [PATCH 12/15] LUCENE-8026: Check query timeout without sampling in other methods --- .../lucene/index/ExitableDirectoryReader.java | 28 +++++++++++-------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index 715975cf924a..79d06ce3b1c0 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -210,38 +210,44 @@ public ExitableIntersectVisitor(PointValues.IntersectVisitor in, QueryTimeout qu * Throws {@link ExitingReaderException} if {@link QueryTimeout#shouldExit()} returns true, * or if {@link Thread#interrupted()} returns true. */ - private void checkAndThrow() { + private void checkAndThrowWithSampling() { if (calls++ % MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK == 0) { - if (queryTimeout.shouldExit()) { - throw new ExitingReaderException("The request took too long to intersect point values. Timeout: " - + queryTimeout.toString() - + ", PointValues=" + in - ); - } else if (Thread.interrupted()) { - throw new ExitingReaderException("Interrupted while intersecting point values. PointValues=" + in); - } + checkAndThrow(); + } + } + + private void checkAndThrow() { + if (queryTimeout.shouldExit()) { + throw new ExitingReaderException("The request took too long to intersect point values. Timeout: " + + queryTimeout.toString() + + ", PointValues=" + in + ); + } else if (Thread.interrupted()) { + throw new ExitingReaderException("Interrupted while intersecting point values. PointValues=" + in); } } @Override public void visit(int docID) throws IOException { - checkAndThrow(); + checkAndThrowWithSampling(); in.visit(docID); } @Override public void visit(int docID, byte[] packedValue) throws IOException { - checkAndThrow(); + checkAndThrowWithSampling(); in.visit(docID, packedValue); } @Override public PointValues.Relation compare(byte[] minPackedValue, byte[] maxPackedValue) { + checkAndThrow(); return in.compare(minPackedValue, maxPackedValue); } @Override public void grow(int count) { + checkAndThrow(); in.grow(count); } } From 7835ef40be2fb699471fc176f4479ac793e5e3b2 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Wed, 14 Nov 2018 14:45:22 +0100 Subject: [PATCH 13/15] LUCENE-8026: Make class static to avoid precommit/javadoc issues --- .../org/apache/lucene/index/ExitableDirectoryReader.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index 79d06ce3b1c0..223502fc4e08 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -193,15 +193,15 @@ public int getDocCount() { } } - public static class ExitableIntersectVisitor implements PointValues.IntersectVisitor { + private static class ExitableIntersectVisitor implements PointValues.IntersectVisitor { - public static final int MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = 10; + private static final int MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = 10; private final PointValues.IntersectVisitor in; private final QueryTimeout queryTimeout; private int calls; - public ExitableIntersectVisitor(PointValues.IntersectVisitor in, QueryTimeout queryTimeout) { + private ExitableIntersectVisitor(PointValues.IntersectVisitor in, QueryTimeout queryTimeout) { this.in = in; this.queryTimeout = queryTimeout; } From 5c1849c7500a96cb2edaf4f526ed395be5697869 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Wed, 14 Nov 2018 17:15:42 +0100 Subject: [PATCH 14/15] LUCENE-8026: Add missing constructor documentation --- .../java/org/apache/lucene/index/ExitableDirectoryReader.java | 1 + 1 file changed, 1 insertion(+) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index 223502fc4e08..e89073dee465 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -117,6 +117,7 @@ public static class ExitablePointValues extends PointValues { private final PointValues in; private final QueryTimeout queryTimeout; + /** Constructor **/ public ExitablePointValues(PointValues in, QueryTimeout queryTimeout) { this.in = in; this.queryTimeout = queryTimeout; From 6f48d4f851b4eccf4c3d8c4cfbe6d540e13b4d85 Mon Sep 17 00:00:00 2001 From: Christophe Bismuth Date: Wed, 14 Nov 2018 19:01:26 +0100 Subject: [PATCH 15/15] LUCENE-8026: Make class private to avoid noisy javadoc requirements --- .../org/apache/lucene/index/ExitableDirectoryReader.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java index e89073dee465..03de3c66f23d 100644 --- a/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java +++ b/lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java @@ -112,13 +112,12 @@ public CacheHelper getCoreCacheHelper() { /** * Wrapper class for another PointValues implementation that is used by ExitableFields. */ - public static class ExitablePointValues extends PointValues { + private static class ExitablePointValues extends PointValues { private final PointValues in; private final QueryTimeout queryTimeout; - /** Constructor **/ - public ExitablePointValues(PointValues in, QueryTimeout queryTimeout) { + private ExitablePointValues(PointValues in, QueryTimeout queryTimeout) { this.in = in; this.queryTimeout = queryTimeout; checkAndThrow();