apoorvmittal10 commented on PR #20152: URL: https://github.com/apache/kafka/pull/20152#issuecomment-3357387238
> > @DL1231 I am sorry I still can't follow why we need new MetricConfig and can't do correct calculation in the method itself, [#20152 (comment)](https://github.com/apache/kafka/pull/20152#discussion_r2200852049)? > > @apoorvmittal10 Simply considering the `timeUnit` in the `windowSize` might not be sufficient, because the `timeWindowMs` in `MetricConfig` affects multiple parts of the logic, such as > > https://github.com/apache/kafka/blob/7527a8bac088ac551a34c7ef2447588f70d8ec7e/clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java#L148 > > If we only modify the logic of `windowSize`, consider the following scenario: > > 1. In `MetricConfig`, the default `timeWindowMs` is 30 seconds, and `DEFAULT_NUM_SAMPLES` = 2, meaning it can retain data for at most 60 seconds. > 2. If rebalances occur twice—once at the 1st minute and again at the 59th minute—the expected value for rebalance-rate-per-hour should be 2. > 3. At the 59th minute, when calculating the rebalance rate, since only 60 seconds of data can be retained, the rebalance count = 1. If we consider `timeUnit` in `windowSize`, the time window would become 1 hour. As a result, the calculated rebalance-rate-per-hour = rebalance count / window size = 1 / 1h = 1, which is not equal to 2. Complete missed the PR, apologies for the delay. I still find the approach of having another MetricConfig a workaround. The reason I say that is, fundamentally `timeWindowMs` is defined in MetricConfig which is common for all metrics. But there should be a way to supply the `timeWindowMs` on specific metric, as on override from MetricConfig. Perhaps that might be difficult but what about having an explicit timeWindow detail in Rate class i.e. add another constructor which should take the time window details. If supplied then use that else use from config, will that work? ``` public Rate(TimeUnit unit, SampledStat stat, long timeWindowMs) ``` -- 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]
