-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[improve][broker] Make dispatch rate limiter more precise #18553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[improve][broker] Make dispatch rate limiter more precise #18553
Conversation
9f74a0b to
be27cbd
Compare
5db855a to
fccd477
Compare
|
first of all adding precise terminology in dispatch path is not correct and really misguiding. also have you considered performance impact due to this change? we should not take any performance impact because of this requirement. |
|
Hi @rdhabalia
This patch has almost no performance penalty, except that the first time a consumer is created it retrieves a property of another consumer directly from memory |
a37e875 to
ffd378c
Compare
ffd378c to
529a928
Compare
|
The pr had no activity for 30 days, mark with Stale label. |
|
please rebase |
Motivation
The Dispatch rate limiter serves two purposes:
But the current design has the following problems:
available permitsindicates how many messages the client needs, and then uses the rate limiter to determine how many entries to read. If batch production is not enabled, it is working. When batch production is enabled, the rate limiter counts the number of entries as the number of messages, resulting in excessive entries to be read.In PR #6719,
preciseDispatcherFlowControlwas added to optimize the above problem: If enabledpreciseDispatcherFlowControl, we callavailablePermits / avgMessagesPerEntryto calculate how many entries to read, but this did not solve the problem because that:avgMessagesPerEntry, randomly take a consumer from the subscription, and get attributestat.avgMessagesPerEntry, it is possible to null the property. We should look for the consumer that meets conditionstat.avgMessagesPerEntry != null.Modifications
rate limiter, and then calculate how many entries to read byavgMessagesPerEntryavgMessagesPerEntryfrom the consumers under subscription, if all consumersavgMessagesPerEntryis zero, look up under the topic.calculateToReadofSingle consumer dispatcherandMulti consumer dispatcherinto the parent classAbstractDispatcher, this will save codes.Follow-up PR
After this PR, there are still such scenarios: failing to get
avgMessagesPerEntryresulting in reading excessive messages:avgMessagesPerEntry.The PR below will fix it.
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: