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 _httClient_s.
In my PR, I pushed most of the code into the CloudHttp2SolrClient
constructor. 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: [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]
