[Broker] Add time based backlog quota #10093
Conversation
|
@codelipenghui @flowchartsman Please take a look. |
| /** | ||
| * @return determine if backlog quota enforcement needs to be done for topic based on time limit | ||
| */ | ||
| public boolean isTimeBacklogExceeded() { |
There was a problem hiding this comment.
A more efficient way to achieve this is based on the Ledger metadata, we can evaluated a time based on the Ledger create time, Ledger create time(next ledger create time) and the max entry Id of the ledger. So that we don't need to read an entry. Of course this will affect the accuracy of the limit, but the way to read the entry every time will make the bookkeeper's burden heavier in a large number of topic cluster
There was a problem hiding this comment.
@codelipenghui Yes, I agree, reading first entry in backlog of slowest cursor will mostly causing read from BK disk which could affect overall performance, but using metadata could also affect accuracy as you mentioned, how about we have both and make it configurable so user can make their own decision? We can have the metadata one as default.
There was a problem hiding this comment.
@codelipenghui PTAL
| * | ||
| * @return quota limit in bytes | ||
| */ | ||
| public long getLimit() { | ||
| return limit; | ||
| public long getLimitSize() { |
There was a problem hiding this comment.
This object is serialized, changing the name is a breaking change.
Are we handling compatibility with old configurations?
There was a problem hiding this comment.
Good catch, I actually missed that. Added compatibility conversion and a unit test to make sure it works.
|
/pulsarbot run-failure-checks |
|
/pulsarbot run-failure-checks |
1 similar comment
|
/pulsarbot run-failure-checks |
|
@MarvinCai Can you resolve the conflict? |
|
@sijie Merged latest mainline |
sijie
added
the
doc-required
Anonymitaet
removed
the
doc-required
Anonymitaet
added
the
doc-complete
Motivation
As discussed in issue #9680, although we can use message TTL to expire the backlogs of the subscription, but it'll still be good to have a time based backlog quota so broker can apply proper back pressure strategy when quota exceeded and stop creating more producer.
Modifications
In BacklogQuota add time based backlog, in BacklogQuotaManager add dropBacklogForTimeLimit to drop message when time based backlog quota exceeded.
Add isTimeBacklogExceeded in PersistentTopic to check whether time based backlog quota has exceeded for given topic.
Verifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
Documentation