dajac commented on a change in pull request #9344: URL: https://github.com/apache/kafka/pull/9344#discussion_r496473270
########## File path: clients/src/test/java/org/apache/kafka/clients/admin/KafkaAdminClientTest.java ########## @@ -733,7 +733,9 @@ public void testCreateTopicsRetryThrottlingExceptionWhenEnabledUntilRequestTimeO time.sleep(defaultApiTimeout + 1); assertNull(result.values().get("topic1").get()); - TestUtils.assertFutureThrows(result.values().get("topic2"), TimeoutException.class); + ThrottlingQuotaExceededException e = TestUtils.assertFutureThrows(result.values().get("topic2"), + ThrottlingQuotaExceededException.class); + assertEquals(0, e.throttleTimeMs()); Review comment: Yeah, I do agree but this could happen even if this should be rare. This test case is a bit stretched to verify that throttle time does not go below zero. The reasoning of doing this is that a client could be throttled for longer than `default.api.timeout.ms`. When this happens, I believe that we should return an adjusted throttle time such that the client does not have to re-wait for the time that it has already waited for. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org