clolov commented on code in PR #18109:
URL: https://github.com/apache/kafka/pull/18109#discussion_r1877888237
##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/CoordinatorRequestManagerTest.java:
##########
@@ -75,6 +83,65 @@ public void testSuccessfulResponse() {
assertEquals(Collections.emptyList(), pollResult.unsentRequests);
}
+ /**
+ * This test mimics a client that has been disconnected from the
coordinator. When the client remains disconnected
+ * from the coordinator for 60 seconds, the client will begin to emit a
warning log every minute thereafter to
+ * alert the user about the ongoing disconnect status. The warning log
includes the length of time of the ongoing
+ * disconnect.
+ *
+ * <p/>
+ *
+ * However, the logic used to calculate the length of the disconnect was
not correct. This test exercises the
+ * disconnect logic, controlling the logging and system time, to ensure
the warning message is correct.
+ *
+ * @see CoordinatorRequestManager#markCoordinatorUnknown(String, long)
+ */
+ @Test
+ public void testMarkCoordinatorUnknownLoggingAccuracy() {
+ long oneMinute = 60000;
+
+ try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister()) {
+ appender.setClassLogger(CoordinatorRequestManager.class,
Level.DEBUG);
Review Comment:
Also for my understanding, if the message is at INFO level why do you set
the level to DEBUG here?
##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/CoordinatorRequestManagerTest.java:
##########
@@ -75,6 +83,65 @@ public void testSuccessfulResponse() {
assertEquals(Collections.emptyList(), pollResult.unsentRequests);
}
+ /**
+ * This test mimics a client that has been disconnected from the
coordinator. When the client remains disconnected
+ * from the coordinator for 60 seconds, the client will begin to emit a
warning log every minute thereafter to
+ * alert the user about the ongoing disconnect status. The warning log
includes the length of time of the ongoing
+ * disconnect.
+ *
+ * <p/>
+ *
+ * However, the logic used to calculate the length of the disconnect was
not correct. This test exercises the
+ * disconnect logic, controlling the logging and system time, to ensure
the warning message is correct.
+ *
+ * @see CoordinatorRequestManager#markCoordinatorUnknown(String, long)
+ */
+ @Test
+ public void testMarkCoordinatorUnknownLoggingAccuracy() {
+ long oneMinute = 60000;
+
+ try (final LogCaptureAppender appender =
LogCaptureAppender.createAndRegister()) {
+ appender.setClassLogger(CoordinatorRequestManager.class,
Level.DEBUG);
+ CoordinatorRequestManager coordinatorRequestManager =
setupCoordinatorManager(GROUP_ID);
+ assertFalse(coordinatorRequestManager.coordinator().isPresent());
+
+ // Step 1: mark the coordinator as disconnected right after
creation of the CoordinatorRequestManager.
+ // Because the disconnect occurred immediately, no warning should
be logged.
+ coordinatorRequestManager.markCoordinatorUnknown("test",
time.milliseconds());
+ List<Long> ms = millisFromLog(appender);
+ assertEquals(0, ms.size());
+
+ // Step 2: sleep for one minute and mark the coordinator unknown
again. Then verify that the warning was
+ // logged and the reported time is accurate.
+ time.sleep(oneMinute);
+ coordinatorRequestManager.markCoordinatorUnknown("test",
time.milliseconds());
+ assertMillisecondEquals(appender, oneMinute);
+
+ // Step 3: sleep for *another* minute, mark the coordinator
unknown again, and verify the accuracy.
+ time.sleep(oneMinute);
+ coordinatorRequestManager.markCoordinatorUnknown("test",
time.milliseconds());
+ assertMillisecondEquals(appender, oneMinute * 2);
+ }
+ }
+
+ private void assertMillisecondEquals(LogCaptureAppender appender, long
expected) {
+ List<Long> ms = millisFromLog(appender);
+ assertFalse(ms.isEmpty());
+ long actual = ms.get(ms.size() - 1);
+ assertEquals(expected, actual, "The ");
Review Comment:
For my understanding, is the message upon assertion failure meant to be just
"The"?
--
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]
