You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/06/30 23:07:03 UTC

[GitHub] [kafka] hachikuji opened a new pull request, #12372: KAFKA-14036; Set local time in `ControllerApis` when `handle` returns

hachikuji opened a new pull request, #12372:
URL: https://github.com/apache/kafka/pull/12372

   In `ControllerApis`, we are missing the logic to set the local processing end time after `handle` returns. As a consequence of this, the remote time ends up reported as the local time in the request level metrics.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] jsancio commented on a diff in pull request #12372: KAFKA-14036; Set local time in `ControllerApis` when `handle` returns

Posted by GitBox <gi...@apache.org>.
jsancio commented on code in PR #12372:
URL: https://github.com/apache/kafka/pull/12372#discussion_r911517339


##########
core/src/main/scala/kafka/server/ControllerApis.scala:
##########
@@ -117,6 +117,11 @@ class ControllerApis(val requestChannel: RequestChannel,
           s"with context ${request.context}", t)
         requestHelper.handleError(request, t)
       }
+    } finally {
+      // Only record local completion time if it is unset.
+      if (request.apiLocalCompleteTimeNanos < 0) {
+        request.apiLocalCompleteTimeNanos = time.nanoseconds

Review Comment:
   All the requests to the controller and kraft get queued for another thread to do the work. That means that for these RPCs the api local time is the time it took to authorize the request and queue the request for the controller or kraft, right?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] hachikuji merged pull request #12372: KAFKA-14036; Set local time in `ControllerApis` when `handle` returns

Posted by GitBox <gi...@apache.org>.
hachikuji merged PR #12372:
URL: https://github.com/apache/kafka/pull/12372


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [kafka] hachikuji commented on a diff in pull request #12372: KAFKA-14036; Set local time in `ControllerApis` when `handle` returns

Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12372:
URL: https://github.com/apache/kafka/pull/12372#discussion_r911532842


##########
core/src/main/scala/kafka/server/ControllerApis.scala:
##########
@@ -117,6 +117,11 @@ class ControllerApis(val requestChannel: RequestChannel,
           s"with context ${request.context}", t)
         requestHelper.handleError(request, t)
       }
+    } finally {
+      // Only record local completion time if it is unset.
+      if (request.apiLocalCompleteTimeNanos < 0) {
+        request.apiLocalCompleteTimeNanos = time.nanoseconds

Review Comment:
   Right. The asynchronous processing time is counted as "remote time." Most of the time, we are waiting for replication of some kind, so the meaning is mostly correct. The "local time" is mainly measuring how much time was spent on the request threads. Perhaps we could consider an additional level of granularity which accounts for the time spent on the raft/controller threads. Seems like that would be useful.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org