hachikuji commented on a change in pull request #9564:
URL: https://github.com/apache/kafka/pull/9564#discussion_r525428214



##########
File path: 
clients/src/main/java/org/apache/kafka/clients/RequestCompletionHandler.java
##########
@@ -24,4 +24,11 @@
 
     void onComplete(ClientResponse response);
 
+    /**
+     * Fire when the request transmission hits a fatal exception.
+     *
+     * @param exception the thrown exception
+     */
+    default void onFailure(RuntimeException exception) {

Review comment:
       I am not so sure about adding this to `RequestCompletionHandler`. This 
interface is mainly serving `NetworkClient`, but there are no changes to 
`NetworkClient` here.

##########
File path: 
core/src/main/scala/kafka/server/BrokerToControllerChannelManagerImpl.scala
##########
@@ -178,6 +183,11 @@ class BrokerToControllerRequestThread(networkClient: 
KafkaClient,
     }
   }
 
+  private def isTimedOut(response: ClientResponse): Boolean = {
+    val requestCreatedTime = response.receivedTimeMs() - 
response.requestLatencyMs()

Review comment:
       The time we need to track should begin when the request is submitted to 
the manager. This looks like it is just checking how long an individual request 
is taking, which the request timeout in `NetworkClient` already takes care of.




----------------------------------------------------------------
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


Reply via email to