-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[improve][pip] PIP-406: Introduce metrics related to dispatch throttled events #23945
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
Conversation
…led_msgs and bytes metrics
…a schema and transaction Discussion email: Implementation PR: [improve][pip] PIP-406: Introduce pulsar_subscription_dispatch_throttled_msgs and bytes metrics apache#23945
…a schema and transaction Discussion email: Implementation PR: [improve][pip] PIP-406: Introduce pulsar_subscription_dispatch_throttled_msgs and bytes metrics apache#23945
|
hi, @RobertIndie @tjiuming @poorbarcode The previous design made it difficult for users to identify where throttling was causing a high backlog. I have rewritten the PIP to separate the metrics at the broker, topic, and subscription levels. Please review it again. Thanks. |
RobertIndie
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need separate metrics for both messages throttled and bytes throttled. This could make it complex for users. Could we just name it pulsar_dispatch_throttled_events and only count the number of throttled events where the message count or bytes exceed the rate limits?
So the logic would be like:
private boolean applyDispatchRateLimitsToReadLimits(DispatchRateLimiter rateLimiter,
MutablePair<Integer, Long> readLimits,
DispatchRateLimiter.Type limiterType) {
+ int originalMessagesToRead = readLimits.getLeft();
+ long originalBytesToRead = readLimits.getRight();
// update messagesToRead according to available dispatch rate limit.
int availablePermitsOnMsg = (int) rateLimiter.getAvailableDispatchRateLimitOnMsg();
if (availablePermitsOnMsg >= 0) {
@@ -414,6 +416,12 @@ private boolean applyDispatchRateLimitsToReadLimits(DispatchRateLimiter rateLimi
if (availablePermitsOnByte >= 0) {
readLimits.setRight(Math.min(readLimits.getRight(), availablePermitsOnByte));
}
+ if (readLimits.getLeft() < originalMessagesToRead || readLimits.getRight() < originalBytesToRead) {
+ rateLimiter.increaseDispatchThrottleMsgCount();
+ }
lhotari
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @shibd
|
https://lists.apache.org/thread/pmgkf67ph98k5l3lr60rtg96octb397q The vote passed with 3 +1 binding.
|
Motivation
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: