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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
