Thanks, Baodi. You found a great solution! I provided feedback in https://github.com/apache/pulsar/pull/23945#discussion_r1975535331 about 2 remaining issues to resolve. I hope that's helpful.
-Lari On 2025/02/28 07:25:25 Baodi Shi wrote: > thanks, Lari, I replied to the PIP comment: > https://github.com/apache/pulsar/pull/23945#discussion_r1974929601 > > I applied your suggestion, and you can take another look. The > corresponding draft code has also been updated for your reference. > > Thanks, > Baodi Shi > > Lari Hotari <lhot...@apache.org> 于2025年2月28日周五 01:01写道： > > > > -1 (binding) > > > > I cannot approve this proposal in its current form due to a design > > issue with the metric implementation that I previously raised in the > > PR discussion. > > > > Issue: > > The current implementation tracks messages that have already been > > successfully dispatched to consumers, rather than counting messages > > that were actually throttled. This means `dispatch_throttled_msg_count > > `/`dispatch_throttled_bytes_count` would not measure throttled > > messages/bytes as intended, but instead track messages and bytes that > > were already successfully sent. > > > > The dispatch rate limiter throttling logic was recently consolidated > > to a single point in the codebase through PR #24012. This is where the > > metric should be implemented, rather than at the token consumption > > point. > > > > When addressing this, the design needs to account for re-throttled > > messages, as Baodi brought up in the PR discussion. It seems that we > > have two options: > > 1. Count each throttle event independently (simpler, but may count the > > same message multiple times) > > 2. Implement stateful tracking to count unique throttled messages > > (more accurate, but more complex) > > > > I recommend we: > > 1. Address this design issue in the implementation > > 2. Update the PR accordingly > > 3. Resume the vote once these corrections are made > > > > -Lari > > > > On Thu, 27 Feb 2025 at 15:37, Baodi Shi <ba...@apache.org> wrote: > > > > > > I would like to start voting thread for [improve][pip] PIP-406: > > > Introduce metrics related to dispatch_throttled_count > > > > > > - PR: https://github.com/apache/pulsar/pull/23945 > > > > > > - Discuss thread: > > > https://lists.apache.org/thread/w9pq1jp3f22prsr97mxzj1yhdoo1cnc4 > > > > > > > > > Thanks, > > > Baodi Shi >
