You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/05/02 13:22:41 UTC

[GitHub] [kafka] mimaison commented on a diff in pull request #12045: KAFKA-12319: Change calculation of window size used to calculate `Rate`

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


##########
clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java:
##########
@@ -110,25 +127,40 @@ public String toString() {
     protected void purgeObsoleteSamples(MetricConfig config, long now) {
         long expireAge = config.samples() * config.timeWindowMs();
         for (Sample sample : samples) {
-            if (now - sample.lastWindowMs >= expireAge)
+            if (now - sample.getLastWindowMs() >= expireAge)
                 sample.reset(now);
         }
     }
 
     protected static class Sample {
-        public double initialValue;
-        public long eventCount;
-        public long lastWindowMs;
-        public double value;
+        private double initialValue;

Review Comment:
   This is also part of the public API, so we shouldn't be changing these fields



##########
clients/src/main/java/org/apache/kafka/common/metrics/stats/SampledStat.java:
##########
@@ -138,6 +170,46 @@ public boolean isComplete(long timeMs, MetricConfig config) {
             return timeMs - lastWindowMs >= config.timeWindowMs() || eventCount >= config.eventWindow();
         }
 
+        public boolean isActive() {

Review Comment:
   Again because it's public API we can't add new public methods without having a KIP



##########
clients/src/main/java/org/apache/kafka/common/metrics/stats/Rate.java:
##########
@@ -52,10 +51,6 @@ public Rate(TimeUnit unit, SampledStat stat) {
         this.unit = unit;
     }
 
-    public String unitName() {

Review Comment:
   `Rate` is part of the public API (https://kafka.apache.org/31/javadoc/org/apache/kafka/common/metrics/stats/Rate.html) so we don't want to remove this method.



-- 
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-unsubscribe@kafka.apache.org

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