jolshan commented on code in PR #19489:
URL: https://github.com/apache/kafka/pull/19489#discussion_r2641260972
##########
clients/src/test/java/org/apache/kafka/clients/producer/internals/TransactionManagerTest.java:
##########
@@ -783,16 +785,18 @@ MAX_REQUEST_SIZE, ACKS_ALL, MAX_RETRIES, new
SenderMetricsRegistry(metrics), thi
assertTrue(transactionManager.hasInflightBatches(tp0));
assertEquals(1, transactionManager.sequenceNumber(tp0));
- time.sleep(5000); // delivery time out
- sender.runOnce();
+ ProducerTestUtils.runUntil(sender, () -> {
+ time.sleep(5000); // advance the "now" so NetworkClient sees the
request as expired
+ return responseFuture1.isDone();
+ });
// The retried request will remain inflight until the request timeout
// is reached even though the delivery timeout has expired and the
// future has completed exceptionally.
assertTrue(responseFuture1.isDone());
-
TestUtils.assertFutureThrowsWithMessageContaining(TimeoutException.class,
responseFuture1, SENDER_TIMEOUT_MSG);
+
TestUtils.assertFutureThrowsWithMessageContaining(NetworkException.class,
responseFuture1, NETWORK_DISCONNECTED_MSG);
assertFalse(transactionManager.hasInFlightRequest());
- assertEquals(1, client.inFlightRequestCount());
+ assertEquals(0, client.inFlightRequestCount());
Review Comment:
I think for me, the main thing I'm not sure about is if this test is even
testing the original behavior. It is a bit hard to parse what the original
behavior was though. It says `testDuplicateSequenceAfterProducerReset` but I'm
not even sure where this duplicate sequence is supposed to happen.
The request remaining inflight also seems like it would matter, but now that
is no longer the case.
--
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]
