zheguang commented on code in PR #21770:
URL: https://github.com/apache/kafka/pull/21770#discussion_r2939641518
##########
coordinator-common/src/test/java/org/apache/kafka/coordinator/common/runtime/CoordinatorBackgroundThreadPoolExecutorTest.java:
##########
@@ -80,28 +81,32 @@ public void testMetrics() throws ExecutionException,
InterruptedException, Timeo
task1Unblocked.countDown();
task1.get(5, TimeUnit.SECONDS);
- // Task 3 starts.
+ // Task 3 starts after task 1's metrics are recorded.
task3Started.await();
// Task 2 takes 500 ms.
mockTime.sleep(400);
task2Unblocked.countDown();
task2.get(5, TimeUnit.SECONDS);
+ // Wait until the metrics are recorded before advancing the clock.
+ verify(metrics,
timeout(5000).times(1)).recordBackgroundProcessingTime(500L);
+ verify(metrics,
timeout(5000).times(1)).recordBackgroundThreadBusyTime(250.0);
+
// Task 3 takes 500 ms.
mockTime.sleep(100);
task3Unblocked.countDown();
task3.get(5, TimeUnit.SECONDS);
-
- verify(metrics, times(2)).recordBackgroundQueueTime(0);
- verify(metrics, times(1)).recordBackgroundQueueTime(100);
- verify(metrics, times(1)).recordBackgroundProcessingTime(100);
- verify(metrics, times(2)).recordBackgroundProcessingTime(500);
- verify(metrics, times(1)).recordBackgroundThreadBusyTime(50.0);
- verify(metrics, times(2)).recordBackgroundThreadBusyTime(250.0);
Review Comment:
Hm... if we decided to go with the
`CoordinatorBackgroundThreadPoolExecutor.execute` as is, then in this test we'd
indeed need to contend with the Time that's advanced concurrently between (1)
the `execute()`'s `finally` block where the metrics are updated, and (2) the
the `verify` of metrics.
I wondered though if the right fix should be adding the
`timeout(X).times(Y)` here, to further reduce the effect of unfortunate CPU
scheduling that (2) gets executed before (1).
--
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]
