junrao commented on code in PR #13391:
URL: https://github.com/apache/kafka/pull/13391#discussion_r1164424805
##########
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:
For simplicity, let's just assume there is only one callback for now.
Currently, the code sets callbackRequestCompleteTimeNanos when the response is
being sent. If the callback has been started, but not completed at that point,
the measurement of the callback portion time is not accurate. If we delay the
cut off of callbackRequestCompleteTimeNanos at the time when request time
metric is updated (which currently is when the response send completes), we
allow for more accurate measurement of the callback portion time.
For your second question, yes, it's a bit weird to report the callback time
as 'local' after the response is being sent. However, 'local' really reflects
the amount of time a request handler thread is tied up for processing a
request. From that perspective, it could also make sense. If we cut off the
callback portion of the time by response send time, we could leave a portion of
the request thread time unaccounted 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.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
