-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
