[ISSUE #11351] separate Rate Limiter Implementation to LeakyBucket and FixedWindow

[danielsinai]

Master Issue: #11351

Motivation

In order to fix multiple issues with preciseRateLimiting this PR separates RateLimiter into 2 implementations

1. FixedWindowRateLimiter
2. LeakeyBucketRateLimiter

Modifications

1. Created an abstract class of the current RateLimiter functionalities and added 2 extends classes that override the renew and tryAcquire methods

2. Fixed precis Typo

3. removed IsDispatchRateLimiter in RateLimiter and used the new LeakyBucket algorithm

Verifying this change

LeakyBucketRateLimiterTest
FixedWindowRateLimiterTest

I want to add more tests, but before I would love to get some feedback instead of implementing a lot of tests without reason

Does this pull request potentially affect one of the following parts:

• Dependencies no
• The public API: no
• The schema: no
• The default values of configurations: no
• The wire protocol: no
• The rest endpoints: no
• The admin cli options: no
• Anything that affects deployment: no

Documentation

For contributor

For this PR, do we need to update docs? Probably not, it is just fixing the current implementation

BTW, That's my first PR into pulsar and I am not a strong JAVA programmer - would love to get some feedback and learn from this PR :)

[eolivelli]

Overall the approach makes sense to me

@merlimat @rdhabalia @lhotari @codelipenghui

[danielsinai]

I see that some UT failed - I will fix that soon

[Anonymitaet]

@danielsinai thanks for providing doc-related info!

[danielsinai]

Is it possible to merge this and ron's PR to 2.8 .1 release? @sijie
I think this is pretty big bug fixes

[lhotari]

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

[danielsinai]

@lhotari Ok I understand that this is a big change.

I will close this PR and open a new one that includes a patch for the current solution like have been done in the dispatchRateLimiter (#8599) and it will contain smaller changes.