emitskevich-blp commented on code in PR #15889:
URL: https://github.com/apache/kafka/pull/15889#discussion_r1610223223
##########
clients/src/test/java/org/apache/kafka/common/metrics/stats/RateTest.java:
##########
@@ -64,4 +69,30 @@ public void testRateWithNoPriorAvailableSamples(int
numSample, int sampleWindowS
double expectedRatePerSec = sampleValue / windowSize;
assertEquals(expectedRatePerSec, observedRate, EPS);
}
+
+ // Record an event every 100 ms on average, moving some 1 ms back or forth
for fine-grained
+ // window control. The expected rate, hence, is 10-11 events/sec depending
on the moment of
+ // measurement. Start assertions from the second window.
+ @Test
+ public void testRateIsConsistentAfterTheFirstWindow() {
+ MetricConfig config = new MetricConfig().timeWindow(1,
SECONDS).samples(2);
+ List<Integer> steps = Arrays.asList(0, 99, 100, 100, 100, 100, 100,
100, 100, 100, 100);
+
+ // start the first window and record events at 0,99,199,...,999 ms
+ for (int stepMs : steps) {
+ time.sleep(stepMs);
+ rate.record(config, 1, time.milliseconds());
+ }
+
+ // making a gap of 100 ms between windows
+ time.sleep(101);
+
+ // start the second window and record events at 0,99,199,...,999 ms
+ for (int stepMs : steps) {
+ time.sleep(stepMs);
+ rate.record(config, 1, time.milliseconds());
+ double observedRate = rate.measure(config, time.milliseconds());
Review Comment:
I’m not feeling strongly, could you please advise?
My thoughts:
Now Rate.measure() provides repeatable results when called at/with the same
timestamp. And it did so before. That non-repeatable state, previously
discovered in current PR, was never committed to trunk. So it wouldn’t be clear
for future readers, what’s the rationale behind this double-checking in the
test.
--
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
