My annotated draft PR: https://github.com/apache/solr/pull/3357 Doesn't solve the issue but its exception throwing means many tests fail since those places in Solr were trying to both use an existing HttpClient _and_ set timeouts. We can't do that and we *weren't* doing that! 'course if I'm missing something obvious; let me know!
> I guess my attempt to fix it (https://github.com/apache/solr/pull/3356) is doomed to fail as it currently stands? Agreed. But let's see: Maybe a test would be: configure a super low idle timeout on an Http2SolrClient that's constructed manually. Then create a new Http2SolrClient with the first using withHttpClient and set a reasonable timeout. We should be able to do a request that takes time (like a Solr request referencing "sleep(5000)" in a function query. See features-slow.json with such an excerpt. I believe we'll sadly have a timeout because the idle timeout customization isn't honored when withHttpClient is used. Such a test would go in Http2SolrClientTest. On Mon, May 19, 2025 at 9:45 AM David Smiley <dsmi...@apache.org> wrote: > 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 >
