[jira] [Updated] (SOLR-17831) ExitableDirectoryReader always initialized with QueryLimits.NONE

["Andrzej Bialecki (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SOLR-17831?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andrzej Bialecki updated SOLR-17831:
------------------------------------
    Fix Version/s: 9.9.1

> 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 20m
>  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: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org