divijvaidya commented on code in PR #12045:
URL: https://github.com/apache/kafka/pull/12045#discussion_r862906685


##########
clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java:
##########
@@ -68,28 +63,61 @@ public double measure(MetricConfig config, long now) {
     }
 
     public long windowSize(MetricConfig config, long now) {
-        // purge old samples before we compute the window size
+        // Purge obsolete samples. Obsolete samples are the ones which are not 
relevant to the current calculation
+        // because their creation time is outside (before) the duration of 
time window used to calculate rate.
         stat.purgeObsoleteSamples(config, now);
 
         /*
          * Here we check the total amount of time elapsed since the oldest 
non-obsolete window.
-         * This give the total windowSize of the batch which is the time used 
for Rate computation.
-         * However, there is an issue if we do not have sufficient data for 
e.g. if only 1 second has elapsed in a 30 second
-         * window, the measured rate will be very high.
-         * Hence we assume that the elapsed time is always N-1 complete 
windows plus whatever fraction of the final window is complete.
+         * This gives the duration of computation time window which used to 
calculate Rate.
+         *
+         * For scenarios when rate computation is performed after at least 
`config.samples` have completed,
+         * the duration of computation time window is determined by:
+         *      window duration = (now - start time of oldest non-obsolete 
window)
+         *
+         * ## Special case: First ever window
+         * A special scenario occurs when rate calculation is performed before 
at least `config.samples` have completed
+         * (e.g. if only 1 second has elapsed in a 30 second). In such a 
scenario, window duration would be equal to the
+         * time elapsed in the current window (since oldest non-obsolete 
window is current window). This leads to the
+         * following values for rate. Consider the following example:
+         *      config.timeWindowMs() = 1s
+         *      config.samples() = 2
+         *      Record events (E) at timestamps:
+         *          E1 = CurrentTimeStamp (T1)
+         *          E2 = T1 + 30ms
+         *          E2 = T1 + 60ms

Review Comment:
   Fixed. Thanks for point thing this out.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to