SOLR-18312: introduce dedicated thread pool executor for httpClientBu…

[renatoh]

see ticket https://issues.apache.org/jira/browse/SOLR-18312

[Copilot]

Pull request overview

This PR (SOLR-18312) updates HttpJdkSolrClient to use a dedicated executor for the JDK HttpClient, aiming to prevent deadlocks when request bodies are streamed (producer) while the JDK client consumes them (consumer), particularly under HTTP/1.1 with concurrent requests.

Changes:

• Introduce a separate httpClientExecutor that is always owned and shut down by HttpJdkSolrClient, instead of reusing the request-body streaming executor.
• Add a concurrency regression test that issues multiple concurrent JsonQueryRequest calls with large streamed bodies while forcing HTTP/1.1.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpJdkSolrClient.java	Adds a dedicated executor for the underlying JDK HttpClient and ensures it is shut down on close.
solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpJdkSolrClientTest.java	Adds a concurrent, streamed-body test intended to catch deadlocks under HTTP/1.1.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dsmiley]

I think it'd be more natural for the builder/configured executor to be for the httpClient usage itself, just as the jetty one.

[dsmiley]

I pushed an improvement to defer the use of the executor for body thread pushing to only start when it's requested (by the supplier). Maybe in practice won't be notice but I like the code for it much better now. CC @serhiy-bzhezytskyy

[serhiy-bzhezytskyy]

@dsmiley the deferral does more than tidy the code — it removes the leak by construction for one whole class of failure. I measured it with a counting executor on a refused connection: before e534b380e one body task is submitted and then has to be released; after it, zero (expected:<0> but was:<1> against the previous commit). So the original SOLR-17707 guard now only covers "body requested, then the connection dropped", which is the narrower case it should have been.

Two things I checked while I was in here, both fine:

• the mechanism sentence still holds. I mutated the fix on main to keep only cancel(true) and the SOLR-17707 test fails with content-writing thread leaked, still blocked after failure — so close(sink) is still load-bearing, and moving the comment along with the code was right.
• contentWritingSink is assigned before contentWritingFuture, so a releaseContentWriting() arriving between them finds a null future and an already-closed sink. Over 200 runs that is 200 writers exiting on IOException with the pool fully drained — log noise, not a leak. Not worth a change.

The one thing missing is a test for the new property. I have the one above if you want it, against this branch or as a follow-up.

[dsmiley]

Thanks for the review Serhiy. testStuckContentWritingThreadIsReleasedOnFailure that you added previously is good enough IMO. I just pushed a change to switch from volatile to synchronized, which avoids the sequence issue you pointed out -- thanks for that.