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-unsubscr...@solr.apache.org

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


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

Reply via email to