Github user hanm commented on the issue:

    https://github.com/apache/zookeeper/pull/307
  
    I think we should consolidate the latency check in 
`zks.serverStats().updateLatency`. It's odd to have two (or in future even 
more) types of latency checks scattered around which creates fragmentation 
w.r.t. the definition of what a request latency means. The existing latency 
measurement in ServerStats measures the time between a request creation and a 
request landing at final request processor; the patch instead measures end to 
end time of a request from its start to finish processing. I am fine with the 
end to end processing time, though I'd like to double check with a few folks 
around to make sure the regression and impact of this change is limited.
    
    I think ServerStats is a good place to put the DS Ted recommended. 
    
    I think it's a good idea to scope the JIRA so it's easier to get it 
reviewed and committed. What this patch is doing is a positive improvement to 
the operational aspects of ZK so that can be the scope of this PR. On top of 
that future improvements could be what Edward and Ted suggested (JMX, 
distribution of latencies / histogram etc). These work can be tracked by making 
them sub tasks under current JIRA.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to