dajac commented on a change in pull request #10024:
URL: https://github.com/apache/kafka/pull/10024#discussion_r574307648



##########
File path: core/src/main/scala/kafka/common/InterBrokerSendThread.scala
##########
@@ -78,6 +77,9 @@ abstract class InterBrokerSendThread(
       failExpiredRequests(now)
       unsentRequests.clean()
     } catch {
+      case _: DisconnectException if isShutdownInitiated =>
+        // DisconnectException is caused by NetworkClient#ensureActive
+        // this thread is closing so this error is acceptable

Review comment:
       `DisconnectException` is indeed expected after `initiateClose` is 
called. It seems OK to handle it here but I would rather use `client.active()` 
instead of `isShutdownInitiated`. This is something that we should have done in 
https://github.com/apache/kafka/pull/9999.
   
   @ijuma I think that your point is legitimate. It seems that we brutally stop 
the send thread without really draining the inflight requests. This maybe OK as 
we stop this thread only when the broker is shutdown anyway. In order to 
restore the CI, I suggest to proceed with the minor fix and to review/address 
this point in a followup PR if necessary.




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