lhotari commented on pull request #10384:
URL: https://github.com/apache/pulsar/pull/10384#issuecomment-885635317


   > * at the time of configuration change, if the connection was in a state 
where the 'read is disabled' (because of a previous rate limit), should the 
RateLimiter call autoReadResetFunction() in it's 'close' method? Otherwise, 
would the connection deadlock and forever remain in 'read disable' state?
   
   @bharanic-dev Thanks for the feedback. I have added the call to the close 
method.
   
   > * for PIP-82, one rate limiter will be shared by multiple topics. So, when 
a topic is unloaded, the ratelimiter should not be closed unless it was the 
last topic that was using it. So, instead of making 'PublishRateLimiter' 
'Autoclosable', can you please consider adding a 'detach' method that can be 
called when the topic is getting unloaded?
   
   @bharanic-dev I think it's better to handle this with composition instead of 
making a single class handle different use cases. With composition it becomes 
easy. You can have a wrapper that takes a PublishRateLimiter and handles the 
life cycle with reference counting. One possible solution is to use the 
org.apache.bookkeeper.mledger.util.AbstractCASReferenceCounted class as a base 
class for this kind of wrapper.


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