sjvanrossum commented on PR #37705:
URL: https://github.com/apache/beam/pull/37705#issuecomment-4091459868

   FYI, the original comment is somewhat outdated. This was somewhat noticeable 
under special circumstances back when there was a separate function running 
poll in a loop with its own timer, but it's not that much of a deal now. When 
we have fetches queued up on the client then `poll()` returns almost instantly 
and you'd measure the latency of calling `System.nanoTime()`.
   
   The Kafka client metric measures fetch latency instead of poll latency so 
extracting that from the client would remove that last bit of inaccuracy, but 
there's no mechanism to tap into external metrics right now. If you're 
interested it might be worth fleshing out a design and sending it for comment 
on the dev list.
   
   As noted above, Guava's `Stopwatch` is just used as a convenience wrapper 
over `System.nanoTime()`. Both `System.currentTimeMillis()` and 
`System.nanoTime()` may derive from the same source (e.g., `gettimeofday`) if 
the OS does not provide a monotonic clock (e.g., `clock_gettime`). Monotonicity 
is guaranteed on the relevant platforms so that makes `System.nanoTime()` the 
right choice for accurate measurement of durations.
   
   I think you can close this PR since it does not address the issue I had 
(poorly) described in that comment. Solving it requires some design work and 
changes to the metrics subsystem. I hope you feel motivated to tackle that, 
because it would improve the accuracy of that metric. 😃 
   
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to