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_
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 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: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
