jeffkbkim commented on code in PR #17221:
URL: https://github.com/apache/kafka/pull/17221#discussion_r1769334386
##########
coordinator-common/src/test/java/org/apache/kafka/coordinator/common/runtime/HdrHistogramTest.java:
##########
@@ -172,4 +177,39 @@ public void testHistogramDataReset() {
assertEquals(numEventsInFirstCycle, hdrHistogram.count(now +
maxSnapshotAgeMs));
assertEquals(numEventsInSecondCycle, hdrHistogram.count(now + 1 +
maxSnapshotAgeMs));
}
+
+ @Test
+ public void testLatestHistogramRace() throws InterruptedException,
ExecutionException {
+ long maxSnapshotAgeMs = 10L;
+ long now = System.currentTimeMillis();
+ HdrHistogram hdrHistogram = new HdrHistogram(maxSnapshotAgeMs,
MAX_VALUE, 1);
+ ExecutorService countExecutor = Executors.newFixedThreadPool(2);
+ for (int i = 1; i < 10000; i++) {
+ int numEvents = 2;
+ for (int j = 0; j < numEvents; j++) {
+ hdrHistogram.record(i);
+ }
+ final long moreThanMaxAge = now + maxSnapshotAgeMs + 1;
+ now = moreThanMaxAge;
+ CountDownLatch latch = new CountDownLatch(1);
+ Callable<Long> countTask = () -> {
+ try {
+ assertTrue(latch.await(500, TimeUnit.MILLISECONDS));
+ return hdrHistogram.count(moreThanMaxAge);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ };
+ Future<Long> t1Future = countExecutor.submit(countTask);
+ Future<Long> t2Future = countExecutor.submit(countTask);
+ latch.countDown();
+ long t1Count = t1Future.get();
+ long t2Count = t2Future.get();
+ assertTrue(
+ numEvents == t1Count && numEvents == t2Count,
Review Comment:
actually, is this what we want? my understanding is that if we have two
threads concurrently calling latestHistogram, we want the first thread to reset
the recorder and return the previous histogram where the other thread should
return the reset-ed histogram.
i may be wrong but both thread counts returning 2 doesn't seem right
##########
coordinator-common/src/test/java/org/apache/kafka/coordinator/common/runtime/HdrHistogramTest.java:
##########
@@ -172,4 +177,39 @@ public void testHistogramDataReset() {
assertEquals(numEventsInFirstCycle, hdrHistogram.count(now +
maxSnapshotAgeMs));
assertEquals(numEventsInSecondCycle, hdrHistogram.count(now + 1 +
maxSnapshotAgeMs));
}
+
+ @Test
+ public void testLatestHistogramRace() throws InterruptedException,
ExecutionException {
+ long maxSnapshotAgeMs = 10L;
+ long now = System.currentTimeMillis();
+ HdrHistogram hdrHistogram = new HdrHistogram(maxSnapshotAgeMs,
MAX_VALUE, 1);
+ ExecutorService countExecutor = Executors.newFixedThreadPool(2);
+ for (int i = 1; i < 10000; i++) {
+ int numEvents = 2;
+ for (int j = 0; j < numEvents; j++) {
+ hdrHistogram.record(i);
+ }
+ final long moreThanMaxAge = now + maxSnapshotAgeMs + 1;
+ now = moreThanMaxAge;
+ CountDownLatch latch = new CountDownLatch(1);
+ Callable<Long> countTask = () -> {
+ try {
+ assertTrue(latch.await(500, TimeUnit.MILLISECONDS));
Review Comment:
if we want both threads to arrive here before proceeding, i think we can use
CyclicBarrier instead
--
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
