Skip to content

Commit 679854c

Browse files
authored
KAFKA-19896: Use MockTime::sleep in ShareConsumeRequestManagerTest. (#20922)
*What* - Currently, when there is a backoff wait period, we are retrying the acknowledgements until the backoff period completes and then these acks are sent. - But as this is a unit test, we can use time.sleep() to forward the `currentTime`, which will allow the backoff period to be over. - PR fixes the 4 tests in `ShareConsumeRequestManagerTest` to use `MockTime::sleep`. This makes the tests faster as we do not actually need to wait for the backoff. We can just update the value. Reviewers: Andrew Schofield <[email protected]>
1 parent 31330e2 commit 679854c

File tree

1 file changed

+19
-12
lines changed

1 file changed

+19
-12
lines changed

clients/src/test/java/org/apache/kafka/clients/consumer/internals/ShareConsumeRequestManagerTest.java

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@
7777
import org.apache.kafka.common.utils.MockTime;
7878
import org.apache.kafka.common.utils.Time;
7979
import org.apache.kafka.common.utils.Timer;
80-
import org.apache.kafka.test.TestUtils;
8180

8281
import org.junit.jupiter.api.AfterEach;
8382
import org.junit.jupiter.api.BeforeEach;
@@ -366,14 +365,15 @@ public void testServerDisconnectedOnShareAcknowledge() throws InterruptedExcepti
366365

367366
assertEquals(1, shareConsumeRequestManager.requestStates(0).getAsyncRequest().getAcknowledgementsToSendCount(tip0));
368367

369-
TestUtils.retryOnExceptionWithTimeout(() -> {
370-
assertEquals(0, shareConsumeRequestManager.sendAcknowledgements());
371-
// We expect the remaining acknowledgements to be cleared due to share session epoch being set to 0.
372-
assertNull(shareConsumeRequestManager.requestStates(0));
373-
// The callback for these unsent acknowledgements will be invoked with an error code.
374-
assertEquals(Map.of(tip0, acknowledgements2), completedAcknowledgements.get(0));
375-
assertInstanceOf(ShareSessionNotFoundException.class, completedAcknowledgements.get(0).get(tip0).getAcknowledgeException());
376-
});
368+
// Wait for backoff time before sending the next request.
369+
time.sleep(retryBackoffMs);
370+
371+
assertEquals(0, shareConsumeRequestManager.sendAcknowledgements());
372+
// We expect the remaining acknowledgements to be cleared due to share session epoch being set to 0.
373+
assertNull(shareConsumeRequestManager.requestStates(0));
374+
// The callback for these unsent acknowledgements will be invoked with an error code.
375+
assertEquals(Map.of(tip0, acknowledgements2), completedAcknowledgements.get(0));
376+
assertInstanceOf(ShareSessionNotFoundException.class, completedAcknowledgements.get(0).get(tip0).getAcknowledgeException());
377377

378378
// Attempt a normal fetch to check if nodesWithPendingRequests is empty.
379379
assertEquals(1, sendFetches());
@@ -653,7 +653,10 @@ public void testRetryAcknowledgements() throws InterruptedException {
653653
assertEquals(6, shareConsumeRequestManager.requestStates(0).getSyncRequestQueue().peek().getIncompleteAcknowledgementsCount(tip0));
654654
assertEquals(0, shareConsumeRequestManager.requestStates(0).getSyncRequestQueue().peek().getInFlightAcknowledgementsCount(tip0));
655655

656-
TestUtils.retryOnExceptionWithTimeout(() -> assertEquals(1, shareConsumeRequestManager.sendAcknowledgements()));
656+
// Wait for backoff time before sending the next request.
657+
// After the first attempt, it can maximum be 1.2x of the configured backoff when acknowledge fails. (jitter = 0.2)
658+
time.sleep((long) (1.5 * retryBackoffMs));
659+
assertEquals(1, shareConsumeRequestManager.sendAcknowledgements());
657660

658661
assertEquals(6, shareConsumeRequestManager.requestStates(0).getSyncRequestQueue().peek().getInFlightAcknowledgementsCount(tip0));
659662

@@ -1232,7 +1235,9 @@ public void testCallbackHandlerConfig() throws InterruptedException {
12321235
shareConsumeRequestManager.commitAsync(Map.of(tip0, new NodeAcknowledgements(0, acknowledgements2)),
12331236
calculateDeadlineMs(time.timer(defaultApiTimeoutMs)));
12341237

1235-
TestUtils.retryOnExceptionWithTimeout(() -> assertEquals(1, shareConsumeRequestManager.sendAcknowledgements()));
1238+
// Wait for backoff time before sending the next request.
1239+
time.sleep(retryBackoffMs);
1240+
assertEquals(1, shareConsumeRequestManager.sendAcknowledgements());
12361241

12371242
client.prepareResponse(fullAcknowledgeResponse(tip0, Errors.NONE));
12381243
networkClientDelegate.poll(time.timer(0));
@@ -1401,7 +1406,9 @@ public void testShareAcknowledgeInvalidResponse() throws InterruptedException {
14011406

14021407
assertEquals(1, shareConsumeRequestManager.requestStates(0).getAsyncRequest().getIncompleteAcknowledgementsCount(tip0));
14031408

1404-
TestUtils.retryOnExceptionWithTimeout(() -> assertEquals(1, shareConsumeRequestManager.sendAcknowledgements()));
1409+
// Wait for backoff time before sending the next request. (it can maximum be 1.2x of the configured backoff when acknowledge fails.)
1410+
time.sleep((long) (1.5 * retryBackoffMs));
1411+
assertEquals(1, shareConsumeRequestManager.sendAcknowledgements());
14051412

14061413
client.prepareResponse(fullAcknowledgeResponse(t2ip0, Errors.NONE));
14071414
networkClientDelegate.poll(time.timer(0));

0 commit comments

Comments
 (0)