michaeljmarshall commented on code in PR #17686:
URL: https://github.com/apache/pulsar/pull/17686#discussion_r973247757
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/AbstractBaseDispatcher.java:
##########
@@ -220,6 +229,11 @@ public int
filterEntriesForConsumer(Optional<MessageMetadata[]> optMetadataArray
}
+ if (serviceConfig.isDispatchThrottlingForFilteredEntriesEnabled()) {
Review Comment:
> one side effect of putting this here is that this affects the
"analize-backlog" command.
Good point, I think that might be good though in order to prevent that
command from consuming too many resources at a time in the broker.
> also, we are blocking the dispatch thread.
The call to `tryDispatchPermit` is non-blocking, other than needing to
acquire a lock on the subscription's rate limiter. Here is the relevant code
path:
https://github.com/apache/pulsar/blob/ad6213828c1f2e6b736657d3745522ac1e2b1bb1/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/DispatchRateLimiter.java#L100-L108
https://github.com/apache/pulsar/blob/84b65598481fd9bbb6e06e2deb335222a04b9c6b/pulsar-common/src/main/java/org/apache/pulsar/common/util/RateLimiter.java#L175-L202
It's interesting that the current solution acquires permits after sending
messages. In looking at the `RateLimiter` code above, we completely ignore
whether or not the specific messages actually get the permit to dispatch
messages when `isDispatchOrPrecisePublishRateLimiter` is false. That is
probably the intention of the setting though, and since this PR does not change
those semantics though, I think it is fine to leave it as is for this PR.
--
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]
