laminelam commented on code in PR #2378:
URL: https://github.com/apache/solr/pull/2378#discussion_r1569571691
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -422,7 +422,10 @@ public CloudHttp2SolrClient build() {
}
} else if (!solrUrls.isEmpty()) {
try {
- stateProvider = new Http2ClusterStateProvider(solrUrls,
httpClient);
+ if (this.internalClientBuilder != null) {
+ this.httpClient = internalClientBuilder.build();
Review Comment:
That's funny, I pushed 2 weeks ago an internal PR (at my work) for this
exact same bug, as I faced it myself.
@stillalex is correct, the resource won't be released if an exception is
raised when instantiating the _Http2ClusterStateProvider_. It also interferes
with the _clientIsInternal_ logic and prevents _myClient_ from being released.
Plus we end up creating two _httpCient_ instances. CloudHttp2SolrClient and
stateProvider would use 2 different _httClients_.
In my PR, I pushed most of the code into the CloudHttp2SolrClient
constructor. The idea is to first assign/build the _myClient_ and then pass it
to _Http2ClusterStateProvider_. This guaranties that one and unique httClient
instance is used (and released).
I also added a test case to test if the same _httpClient_ is preserved in
both _CloudHttp2SolrClient_ and _Http2ClusterStateProvider_.
All test cases passed, including _TestExportTool_.
Will see if I can push the PR here (upstream).
--
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]
