laminelam commented on code in PR #2412:
URL: https://github.com/apache/solr/pull/2412#discussion_r1572678088
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudHttp2SolrClient.java:
##########
@@ -85,14 +79,62 @@ protected CloudHttp2SolrClient(Builder builder) {
this.lbClient = new LBHttp2SolrClient.Builder(myClient).build();
}
+ private Http2SolrClient createOrGetHttpClientFromBuilder(Builder builder) {
+ return builder.httpClient != null
+ ? builder.httpClient
+ : builder.internalClientBuilder != null
+ ? builder.internalClientBuilder.build()
+ : new Http2SolrClient.Builder().build();
+ }
+
+ private ClusterStateProvider createZkClusterStateProvider(Builder builder) {
+ try {
+ ClusterStateProvider stateProvider =
+ ClusterStateProvider.newZkClusterStateProvider(
+ builder.zkHosts, builder.zkChroot, builder.canUseZkACLs);
+ if (stateProvider instanceof SolrZkClientTimeoutAware) {
+ var timeoutAware = (SolrZkClientTimeoutAware) stateProvider;
+ timeoutAware.setZkClientTimeout(builder.zkClientTimeout);
+ timeoutAware.setZkConnectTimeout(builder.zkConnectTimeout);
+ }
+ return stateProvider;
+ } catch (Exception e) {
+ closeMyClientIfNeeded();
+ throw (e);
+ }
+ }
+
+ private ClusterStateProvider createHttp2ClusterStateProvider(
+ List<String> solrUrls, Http2SolrClient httpClient) {
+ try {
+ return new Http2ClusterStateProvider(solrUrls, httpClient);
+ } catch (Exception e) {
+ closeMyClientIfNeeded();
+ throw new RuntimeException(
+ "Couldn't initialize a HttpClusterStateProvider (is/are the "
+ + "Solr server(s), "
+ + solrUrls
+ + ", down?)",
+ e);
+ }
+ }
+
+ private void closeMyClientIfNeeded() {
+ try {
+ if (clientIsInternal && myClient != null) {
+ myClient.close();
+ }
+ } catch (Exception e) {
+ throw new RuntimeException("Exception on closing myClient", e);
+ }
+ }
+
@Override
public void close() throws IOException {
stateProvider.close();
lbClient.close();
- if (clientIsInternal && myClient != null) {
- myClient.close();
- }
+ closeMyClientIfNeeded();
Review Comment:
Yeah I did see this pattern in lot of places in Solr code.
_myClient_ is assigned in the constructor (depending on the configured
builder). If the builder didn't have a _httpClient_ we create one, and this is
the one (the one we created) we want to close. We don't close the "external"
one.
This is the purpose of the _closeMyClientIfNeeded_ method. Close it only if
it's internal (meaning WE created it)
--
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
