jolshan commented on code in PR #13391:
URL: https://github.com/apache/kafka/pull/13391#discussion_r1163421308
##########
core/src/main/scala/kafka/server/KafkaRequestHandler.scala:
##########
@@ -69,20 +108,52 @@ class KafkaRequestHandler(id: Int,
completeShutdown()
return
+ case callback: RequestChannel.CallbackRequest =>
+ try {
+ val originalRequest = callback.originalRequest
+
+ // If we've already executed a callback for this request, reset
the times and subtract the callback time from the
+ // new dequeue time. This will allow calculation of multiple
callback times.
+ // Otherwise, set dequeue time to now.
+ if (originalRequest.callbackRequestDequeueTimeNanos.isDefined) {
+ val prevCallbacksTimeNanos =
originalRequest.callbackRequestCompleteTimeNanos.getOrElse(0L) -
originalRequest.callbackRequestDequeueTimeNanos.getOrElse(0L)
+ originalRequest.callbackRequestCompleteTimeNanos = None
+ originalRequest.callbackRequestDequeueTimeNanos =
Some(time.nanoseconds() - prevCallbacksTimeNanos)
+ } else {
+ originalRequest.callbackRequestDequeueTimeNanos =
Some(time.nanoseconds())
+ }
+
+ currentRequest.set(originalRequest)
+ callback.fun()
+ if (originalRequest.callbackRequestCompleteTimeNanos.isEmpty)
Review Comment:
There are two cases according to artem --
one where we send a response via network client and one where we don't (and
we have more callbacks)
in case a, we set in the network client and that is the end of handling of
the request. This is also when the metric is updated. In case b, we don't set
it there so we must do it here. I have the check to match the same sort of
protocol we see in kafka apis where we check if the value is -1. Maybe it's
fine to set it twice in case a as we won't update the metrics again, but I did
it for consistency.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
