lhotari opened a new pull request #10384: URL: https://github.com/apache/pulsar/pull/10384
### Motivation When using `preciseTopicPublishRateLimiterEnable=true` (introduced by #7078) setting for rate limiting, there are various issues: - updating the limits doesn't set either boundary when changing the limits from a bounded limit to unbounded. - each topic will create a scheduler thread for each limiter instance - each topic will never release the scheduler thread when the topic gets unloaded / closed ### Modifications - Fix updating of the limits by cleaning up the previous limiter instances before creating new limiter instances - Use `brokerService.pulsar().getExecutor()` as the scheduler for the rate limiter instances - Add resource cleanup hooks for topic closing (unload) ### Open issue The existing code has a difference in passing the `rateLimitFunction`: https://github.com/apache/pulsar/blob/69a173a82c89893f54dbe5b6f422249f66ea5418/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PrecisPublishLimiter.java#L80-L86 It's passed to the `topicPublishRateLimiterOnMessage`, but not to `topicPublishRateLimiterOnByte` . It is unclear whether this is intentional. The `rateLimitFunction` is `() -> this.enableCnxAutoRead()` https://github.com/apache/pulsar/blob/69a173a82c89893f54dbe5b6f422249f66ea5418/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractTopic.java#L913 (This also raises a question whether rate limiting works consistently when multiple topics share the same connection.) -- 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: [email protected]
