[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user asfgit closed the pull request at: https://github.com/apache/lucene-solr/pull/497 --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r233556789
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -117,6 +117,7 @@ public CacheHelper getCoreCacheHelper() {
private final PointValues in;
private final QueryTimeout queryTimeout;
+/** Constructor **/
--- End diff --
My bad, I though I used this class in tests, but I didn't. I've turned it
`private` in 6f48d4f851b4eccf4c3d8c4cfbe6d540e13b4d85, thanks :+1:
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user pzygielo commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r233553589
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -117,6 +117,7 @@ public CacheHelper getCoreCacheHelper() {
private final PointValues in;
private final QueryTimeout queryTimeout;
+/** Constructor **/
--- End diff --
My comment
> But isn't it worse now? precommit is satisfied, but nothing more than
(obvious) noise was added.
was for this line.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r233551529
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,156 @@ 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) {
--- End diff --
I'm not sure to understand, the `set -e` flag prevents the second Ant
command line from being run if the `precommit` task fails, and therefore the
latest output in my terminal is a build failure.
The `set -x` (debug option) flag would add a lot noise.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user pzygielo commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r233548542
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,156 @@ 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) {
--- End diff --
But isn't it worse now? precommit is satisfied, but nothing more than
(obvious) noise was added.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r233524064
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,156 @@ 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) {
--- End diff --
That's it, documentation fixed and `precommit` task really green
:sweat_smile: thanks!
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r233517397
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,156 @@ 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) {
--- End diff --
Issue found: I forgot to add a `set -e` statement in my tiny bash script
below to stop on first failure.
Let me run another `precommit` Ant task and push.
```bash
#!/usr/bin/env bash
set -e
cd ${HOME}/git/lucene-solr
find . -name "*.lck" -delete
ant clean compile precommit
ant -f lucene/build.xml test
```
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user jpountz commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r233506349
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,156 @@ 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) {
--- End diff --
I'm getting precommit failures about missing javadocs for this constructor,
aren't you?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user jpountz commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r233043062
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,157 @@ 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 point values. Timeout: "
++ queryTimeout.toString()
++ ", PointValues=" + in
+);
+ } else if (Thread.interrupted()) {
+throw new ExitingReaderException("Interrupted while iterating over
point values. PointValues=" + in);
+ }
+}
+
+@Override
+public void intersect(IntersectVisitor visitor) throws IOException {
+ checkAndThrow();
+ in.intersect(new ExitableIntersectVisitor(visitor, queryTimeout));
+}
+
+@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();
+}
+ }
+
+ 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()) {
--- End diff --
I was thinking of keeping the checkAndThrow in other methods too, just
without sampling. Though I guess most time is going to be spent in visit()
anyway so the current version isn't bad.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232945175
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,150 @@ 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 point values. Timeout: "
++ queryTimeout.toString()
++ ", PointValues=" + in
+);
+ } else if (Thread.interrupted()) {
+throw new ExitingReaderException("Interrupted while iterating over
point values. PointValues=" + in);
+ }
+}
+
+@Override
+public void intersect(IntersectVisitor visitor) throws IOException {
+ checkAndThrow();
+ in.intersect(new ExitableIntersectVisitor(visitor, queryTimeout));
+}
+
+@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();
+}
+ }
+
+ public static class ExitableIntersectVisitor implements
PointValues.IntersectVisitor {
+
+public static final int MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = 10;
+
+private final PointValues.IntersectVisitor in;
+private final QueryTimeout queryTimeout;
+private int calls = 0;
--- End diff --
Yes, removed in 11fd3de51c31ef4c0e2a660ce22df018d194ce7e, thank you.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232832055
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,150 @@ 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 point values. Timeout: "
++ queryTimeout.toString()
++ ", PointValues=" + in
+);
+ } else if (Thread.interrupted()) {
+throw new ExitingReaderException("Interrupted while iterating over
point values. PointValues=" + in);
+ }
+}
+
+@Override
+public void intersect(IntersectVisitor visitor) throws IOException {
+ checkAndThrow();
+ in.intersect(new ExitableIntersectVisitor(visitor, queryTimeout));
+}
+
+@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();
+}
+ }
+
+ public static class ExitableIntersectVisitor implements
PointValues.IntersectVisitor {
+
+public static final int MAX_CALLS_BEFORE_QUERY_TIMEOUT_CHECK = 10;
+
+private final PointValues.IntersectVisitor in;
+private final QueryTimeout queryTimeout;
+private int calls = 0;
--- End diff --
Minor: you don't need the `= 0` -- java does that for you by default.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232780211
--- Diff:
lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java
---
@@ -152,13 +150,130 @@ public void testExitableFilterIndexReader() 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);
reader.close();
directory.close();
}
+
+ /**
+ * Tests timing out of PointValues queries
+ *
+ * @throws Exception on error
+ */
+ 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,
inifiniteQueryTimeout());
+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
--- End diff --
Totally, comments fixed in 45996c885d3e52edcc21d47e40db443895982af5.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232779864
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,157 @@ 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 point values. Timeout: "
++ queryTimeout.toString()
++ ", PointValues=" + in
+);
+ } else if (Thread.interrupted()) {
+throw new ExitingReaderException("Interrupted while iterating over
point values. PointValues=" + in);
+ }
+}
+
+@Override
+public void intersect(IntersectVisitor visitor) throws IOException {
+ checkAndThrow();
+ in.intersect(new ExitableIntersectVisitor(visitor, queryTimeout));
+}
+
+@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();
+}
+ }
+
+ 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()) {
--- End diff --
My bad, outer condition fixed in 9e11f76b2ac7b9cda9f1c78ad0c1f3a5be09f435
and only sample `visit` methods fixed in
3d09426a81277a5344435f43a1942570c1a85f37.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232779517
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,157 @@ 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 point values. Timeout: "
++ queryTimeout.toString()
++ ", PointValues=" + in
+);
+ } else if (Thread.interrupted()) {
+throw new ExitingReaderException("Interrupted while iterating over
point values. PointValues=" + in);
+ }
+}
+
+@Override
+public void intersect(IntersectVisitor visitor) throws IOException {
+ checkAndThrow();
+ in.intersect(new ExitableIntersectVisitor(visitor, queryTimeout));
+}
+
+@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();
+}
+ }
+
+ 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;
+}
--- End diff --
Yes, fixed in dfd1403e02b270ba0712c0a40ee14dbf9e7719fd.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user jpountz commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232738909
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,157 @@ 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 point values. Timeout: "
++ queryTimeout.toString()
++ ", PointValues=" + in
+);
+ } else if (Thread.interrupted()) {
+throw new ExitingReaderException("Interrupted while iterating over
point values. PointValues=" + in);
+ }
+}
+
+@Override
+public void intersect(IntersectVisitor visitor) throws IOException {
+ checkAndThrow();
+ in.intersect(new ExitableIntersectVisitor(visitor, queryTimeout));
+}
+
+@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();
+}
+ }
+
+ 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()) {
--- End diff --
:+1: to sampling checks to reduce overhead.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user jpountz commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232735208
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,157 @@ 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 point values. Timeout: "
++ queryTimeout.toString()
++ ", PointValues=" + in
+);
+ } else if (Thread.interrupted()) {
+throw new ExitingReaderException("Interrupted while iterating over
point values. PointValues=" + in);
+ }
+}
+
+@Override
+public void intersect(IntersectVisitor visitor) throws IOException {
+ checkAndThrow();
+ in.intersect(new ExitableIntersectVisitor(visitor, queryTimeout));
+}
+
+@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();
+}
+ }
+
+ 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;
+}
--- End diff --
Let's not make the `maxCallsBeforeQueryTimeoutCheck` configurable and just
rely on the constant?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user jpountz commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232740066
--- Diff:
lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java
---
@@ -152,13 +150,130 @@ public void testExitableFilterIndexReader() 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);
reader.close();
directory.close();
}
+
+ /**
+ * Tests timing out of PointValues queries
+ *
+ * @throws Exception on error
+ */
+ 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,
inifiniteQueryTimeout());
+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,
immediateQueryTimeout());
+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,
inifiniteQueryTimeout());
+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,
disabledQueryTimeout());
+reader = new TestReader(getOnlyLeafReader(exitableDirectoryReader));
+searcher = new IndexSearcher(reader);
+searcher.search(query, 10);
+reader.close();
+
+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
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user jpountz commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232738340
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -100,13 +109,157 @@ 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 point values. Timeout: "
++ queryTimeout.toString()
++ ", PointValues=" + in
+);
+ } else if (Thread.interrupted()) {
+throw new ExitingReaderException("Interrupted while iterating over
point values. PointValues=" + in);
+ }
+}
+
+@Override
+public void intersect(IntersectVisitor visitor) throws IOException {
+ checkAndThrow();
+ in.intersect(new ExitableIntersectVisitor(visitor, queryTimeout));
+}
+
+@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();
+}
+ }
+
+ 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()) {
--- End diff --
should this also not be called everytime and moved under the `if (calls++ %
maxCallsBeforeQueryTimeoutCheck == 0)` test?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232681060
--- Diff:
lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java
---
@@ -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")
--- End diff --
Fixed in 2b3dfbcba8944dd48c22c341699da528c8525f91.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232681097
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -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);
--- End diff --
Fixed in 5587aa2c904ba135da11a89f47cfa035901b046d.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232681035
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -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);
--- End diff --
Fixed in 6c6e45be3b645814b9871db3b33b799a7c31296d.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232681011
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -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: "
--- End diff --
Fixed in 6c6e45be3b645814b9871db3b33b799a7c31296d.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232664251
--- Diff:
lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java
---
@@ -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")
--- End diff --
I'll try to do so, yes, thanks :+1:
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232664071
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -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);
--- End diff --
I think it would be better, yes. I'll suggest a change like this, thank you.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232663580
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -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);
--- End diff --
Same here.
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user cbismuth commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232663501
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -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: "
--- End diff --
I missed this one ... yes :+1:
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232661641
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -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);
--- End diff --
Same here?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232661519
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -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: "
--- End diff --
Maybe change `to iterate over terms` to `to iterate over points`?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232661927
--- Diff:
lucene/core/src/test/org/apache/lucene/index/TestExitableDirectoryReader.java
---
@@ -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")
--- End diff --
I wonder if we could make a mock clock and then have deterministic control
and be able to re-enable these tests?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
Github user mikemccand commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/497#discussion_r232662145
--- Diff:
lucene/core/src/java/org/apache/lucene/index/ExitableDirectoryReader.java ---
@@ -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);
--- End diff --
A lot of time/effort can be spent in this (recursive) intersect call --
should we also wrap the `IntersectVisitor` and sometimes check for timeout?
---
-
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
[GitHub] lucene-solr pull request #497: LUCENE-8026: ExitableDirectoryReader does not...
GitHub user cbismuth opened a pull request: https://github.com/apache/lucene-solr/pull/497 LUCENE-8026: ExitableDirectoryReader does not instrument points > This means it cannot interrupt range or geo queries. See [LUCENE-8026](https://issues.apache.org/jira/browse/LUCENE-8026). You can merge this pull request into a Git repository by running: $ git pull https://github.com/cbismuth/lucene-solr LUCENE-8026 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/lucene-solr/pull/497.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #497 commit 6aca2ab24f55604a74baa0cf7ec1459640098d59 Author: Christophe Bismuth Date: 2018-11-12T12:53:21Z LUCENE-8026: ExitableDirectoryReader does not instrument points --- - To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional commands, e-mail: dev-h...@lucene.apache.org
