lhotari commented on code in PR #24893:
URL: https://github.com/apache/pulsar/pull/24893#discussion_r2465741367


##########
pulsar-client-admin/src/main/java/org/apache/pulsar/client/admin/internal/http/AsyncHttpConnector.java:
##########
@@ -112,33 +120,57 @@ public AsyncHttpConnector(Client client, 
ClientConfigurationData conf, int autoC
                 (int) 
client.getConfiguration().getProperty(ClientProperties.READ_TIMEOUT),
                 PulsarAdminImpl.DEFAULT_REQUEST_TIMEOUT_SECONDS * 1000,
                 autoCertRefreshTimeSeconds,
-                conf, acceptGzipCompression);
+                conf, acceptGzipCompression, null);
     }
 
     @SneakyThrows
     public AsyncHttpConnector(int connectTimeoutMs, int readTimeoutMs,
                               int requestTimeoutMs,
                               int autoCertRefreshTimeSeconds, 
ClientConfigurationData conf,
-                              boolean acceptGzipCompression) {
+                              boolean acceptGzipCompression,
+                              PulsarClientSharedResourcesImpl sharedResources) 
{
         Validate.notEmpty(conf.getServiceUrl(), "Service URL is not provided");
         serviceNameResolver = new PulsarServiceNameResolver();
         String serviceUrl = conf.getServiceUrl();
         serviceNameResolver.updateServiceUrl(serviceUrl);
         this.acceptGzipCompression = acceptGzipCompression;
         AsyncHttpClientConfig asyncHttpClientConfig =
                 createAsyncHttpClientConfig(conf, connectTimeoutMs, 
readTimeoutMs, requestTimeoutMs,
-                        autoCertRefreshTimeSeconds);
+                        autoCertRefreshTimeSeconds, sharedResources);
         httpClient = createAsyncHttpClient(asyncHttpClientConfig);
         this.requestTimeout = requestTimeoutMs > 0 ? 
Duration.ofMillis(requestTimeoutMs) : null;
         this.maxRetries = httpClient.getConfig().getMaxRequestRetry();
+        this.nameResolver = buildNameResolverIfConfigured(sharedResources);
+    }
+    private NameResolver<InetAddress> 
buildNameResolverIfConfigured(PulsarClientSharedResourcesImpl sharedResources) {
+        if (sharedResources != null && sharedResources.getDnsResolverGroup() 
!= null) {
+            EventLoopGroup eventLoopGroupReference;
+            if (sharedResources.getIoEventLoopGroup() != null) {
+                eventLoopGroupReference = 
sharedResources.getIoEventLoopGroup();
+            } else {
+                // build an EventLoopGroup with default value
+                eventLoopGroupReference = EventLoopUtil.newEventLoopGroup(
+                        Runtime.getRuntime().availableProcessors(), false,
+                        new 
ExecutorProvider.ExtendedThreadFactory("pulsar-admin-client-io",
+                                Thread.currentThread().isDaemon()));

Review Comment:
   In this case the EventLoopGroup wouldn't get closed and it would leak 
resources. It would be better to move this logic earlier so that the created 
event loop group is also used for AsyncHttpClient in this case and it would be 
closed when the instance is closed.
   
   Please also add a test for this case. Restricting sharing of resources can  
be achieved by using the `shareConfigured` method 
(org.apache.pulsar.client.api.PulsarClientSharedResourcesBuilder#shareConfigured
 ) so that the eventloopgroup isn't shared by not configuring it, but just 
calling `configureDnsResolver` to configure the shared dns resolver.



-- 
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]

Reply via email to