[
https://issues.apache.org/jira/browse/SOLR-17831?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Andrzej Bialecki resolved SOLR-17831.
-------------------------------------
Resolution: Fixed
> 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
> Priority: Major
> Labels: pull-request-available
> Fix For: 9.9.1
>
> Time Spent: 1h 40m
> Remaining Estimate: 0h
>
> [~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]
