eolivelli commented on a change in pull request #9802:
URL: https://github.com/apache/pulsar/pull/9802#discussion_r601357328



##########
File path: 
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java
##########
@@ -119,6 +123,9 @@ public ProxyConnection(ProxyService proxyService, 
Supplier<SslHandler> sslHandle
         this.service = proxyService;
         this.state = State.Init;
         this.sslHandlerSupplier = sslHandlerSupplier;
+        if (proxyService.getConfiguration().isEnableShareTimer()) {

Review comment:
       I am not sure we need to make this implementation configurable.
   it is a good thing to share this timer.

##########
File path: 
pulsar-proxy/src/main/java/org/apache/pulsar/proxy/server/ProxyConnection.java
##########
@@ -68,6 +71,7 @@
  */
 public class ProxyConnection extends PulsarHandler implements 
FutureListener<Void> {
     // ConnectionPool is used by the proxy to issue lookup requests
+    private static volatile Timer shareTimer = null;

Review comment:
       can we add this variable as regular field inside ProxyService ?
   this way:
   - you do not need to use a static variable
   - you have a clean lifecycle for the sharedTime (you will shutdown it when 
you shutdown the ProxyService)
   




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to