Nice find. Look forward to seeing the PR as the IndexFetcher timeout regression has already bitten us. I guess my attempt to fix it (https://github.com/apache/solr/pull/3356) is doomed to fail as it currently stands?
From: dev@solr.apache.org At: 05/19/25 09:46:11 UTC-4:00To: dev@solr.apache.org Cc: Luke Kot-Zaniewski (BLOOMBERG/ 919 3RD A ) , stilla...@apache.org, gus.h...@gmail.com Subject: SolrJ timeouts This weekend I was harmonizing some of the timeouts between JDK & Jetty based HTTP SolrClient implementations. I saw some inconsistencies, duplication, and the need for javadocs to explain what these timeouts fundamentally mean. I have some nice WIP code I was looking forward to sharing by now. Then I noticed something insidious: If an Http2SolrClient (Jetty) is created using the builder's withHttpClient(...) method, then the idle (aka socket) timeout & connection timeout (and basically any other setting that's inside the Jetty HttpClient) cannot *actually* be customized henceforth. This should be pretty obvious, when you think about it (duh). But we don't do any checks in construction to alert you, plus we redundantly put some of these settings on the Http2SolrClient (incl. base class) which fools any onlooker into thinking it might do something. My WIP addresses this by throwing an IllegalArgumentException on build() if you try to set something that won't take effect. This revealed a number of prominent places in Solr that both call withHttpClient and also customize the timeouts (to no avail): Overseer, SolrClientCache (for streaming expressions), IndexFetcher, and some recovery stuff; maybe I missed something. This solution won't work. Perhaps withHttpClient should not reuse the underlying HttpClient? It's a shame to not reuse it but it's at least the safest choice. In my opinion, withHttpClient doesn't have an ideal name either; it should be withHttpSolrClient. Its current name is ambiguous; I hate confusing HttpSolrClient with HttpClient. And if we copy configuration from the client but don't actually use it, then maybe we should name this method copyConfigurationFrom or withConfigurationFrom. I'll throw up a draft PR today that does the cleanup/harmonization but not yet solve the above conundrum. (CC'ed some folks that worked on timeouts) ~ David Smiley Apache Lucene/Solr Search Developer http://www.linkedin.com/in/davidwsmiley
