lhotari commented on pull request #11352: URL: https://github.com/apache/pulsar/pull/11352#issuecomment-885665292
@danielsinai I think there's a good intent in this PR, but currently it doesn't provide much value. The high level problem of the changes is that it adds more dependencies to low level details of the rate limiter implementation at the concept / type level. Usually the goal of software development is to reduce dependencies in general. In this case, it would mean that we should go towards extracting some interface (or abstract base class) that defines the minimal interface for a rate limiter. This PR currently takes it to the opposite direction. Each usage now depends directly on either LeakyBucketRateLimiter or FixedWindowRateLimiter. This is the high level problem of the proposed changes in this PR. Usually it helps to iterate and describe the problems of the current implementation once again. I think #11351 is a good start. It could be improved by providing more examples and possible solutions. Repro cases for problems with existing implementation is also useful for creating a better understanding. If there isn't a way to reproduce the issue, it's very hard for others to understand what the exact problem is. In Pulsar, the main way to propose larger design changes is to create a PIP document and bring it up to discussion on the mailing list and in community meetings. That is necessary when larger changes are planned. The current PIPs are listed in the wiki: https://github.com/apache/pulsar/wiki . Anyone can propose a new one by starting a discussion on the mailing list. -- 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]
