Andrzej Bialecki created SOLR-17831:
---------------------------------------
Summary: ExitableDirectoryReader always initialized with
QueryLimits.NONE
Key: SOLR-17831
URL: https://issues.apache.org/jira/browse/SOLR-17831
Project: Solr
Issue Type: Bug
Affects Versions: 9.9
Reporter: Andrzej Bialecki
Assignee: Andrzej Bialecki
[~hossman] discovered that when the system property
{{solr.useExitableDirectoryReader}} is set to true then {{SolrIndexSearcher}}
is supposed to always wrap the current index reader with
{{ExitableDirectoryReader}} and make it use the
{{QueryLimits.getCurrentLimits()}} configuration, which should contain the
limits configuration taken from the current query context
({{{}SolrRequestInfo.getRequestInfo{}}}.
However, the code mistakenly invokes {{QueryLimits.getCurrentLimits()}}
directly in the wrapping method, when the query context is always empty. As a
result, the {{ExitableDirectoryReader}} was always configured with
{{QueryLimits.NONE}} which made its use pointless and misleading.
The key change necessary to fix this bug is a one-liner:
{code:java}
diff --git a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
index f4d0304ec3..e12d54eb9f 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrIndexSearcher.java
@@ -214,7 +214,7 @@ public class SolrIndexSearcher extends IndexSearcher
implements Closeable, SolrI
assert reader != null;
reader = UninvertingReader.wrap(reader,
core.getLatestSchema().getUninversionMapper());
if (useExitableDirectoryReader) { // SOLR-16693 legacy; may be removed.
Probably inefficient.
- reader = ExitableDirectoryReader.wrap(reader,
QueryLimits.getCurrentLimits());
+ reader = ExitableDirectoryReader.wrap(reader, () ->
QueryLimits.getCurrentLimits().shouldExit());
}
return reader;
}
{code}
However, due to the way this property is initialized (statically during
class-loading of {{{}SolrIndexSearcher{}}}) it's nearly impossible to actually
test its impact. That's also the reason this bug survived so long :)
So in addition to the fix above I propose to move the initialization of this
property to a different object (e.g. SolrCore?) which can then be reinitialized
without exiting the JVM. There are helper classes
(\{{CallerSpecificQueryLimit}} that can be used to test exactly where the
limits were tripped.
Also, I'd like to reword the comment referring to SOLR-16693 - until we come up
with a better alternative, using {{ExitableDirectoryReader}} is sometimes the
only way to prevent Solr from getting stuck in a tight loop, either due to
buggy components or due to the actual complexity of the query. Higher-level
limit checks in SearchHandler or in individual SearchComponents can't break
these lower-level loops and thus cannot prevent runaway queries. Also,
performance impact of using {{ExitableDirectoryReader}} hasn't been quantified.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
