[GitHub] [solr] dsmiley commented on a diff in pull request #1360: SOLR 16642 : upgrade Solr to use Lucene 9.5.0

["dsmiley (via GitHub)" <gi...@apache.org>]

dsmiley commented on code in PR #1360:
URL: https://github.com/apache/solr/pull/1360#discussion_r1118137048


##########
solr/core/src/java/org/apache/solr/search/SolrQueryTimeoutImpl.java:
##########
@@ -45,7 +45,6 @@ public static Long getTimeoutAtNs() {
     return timeoutAt.get();
   }
 
-  @Override

Review Comment:
   This innocuous looking removal was very curious to me and I went digging into Lucene's changes that led to this.  Based on what I learned, I predict a performance degradation for any query that isn't using timeouts.  The Lucene issue is https://github.com/apache/lucene/issues/11914 and it means that if Solr does nothing about this, then ExitableDirectoryReader.wrap (used in SolrIndexSearcher) will wrap all underlying low level index components (Terms, ...) because Lucene's ExitableDirectoryReader no longer guards against this with a "isTimeoutEnabled" check as it no longer exists.  Solr could fork the old ExitableDirectoryReader, which would be sad but pretty easy.  Alternatively, maybe SolrIndexSearcher could only wrap for the execution of a query -- not sure as I haven't explored this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org