ZOOKEEPER-4284: Add metrics for observer sync time #1691
Conversation
|
@eolivelli @ztzg @hanm @anmolnar would you mind taking a quick look at this PR? It's simple but would be very useful when enabling the follower hosting observers feature. Thanks. |
| completedSync = true; | ||
| } finally { | ||
| final long syncTime = Time.currentElapsedTime() - startTime; | ||
| ServerMetrics.getMetrics().OBSERVER_SYNC_TIME.add(syncTime); |
There was a problem hiding this comment.
I think we should calculate this time iff completedSync is true. Otherwise, we will get noisy data such as 0 sync time in cases where sync immediately failed (due to network partition for example).
For the same reason the existing follower sync time needs to apply the same fix, which can be done separately.
There was a problem hiding this comment.
Agreed. Changed.
I have also opened a JIRA for fix the follower sync time and will submit the fix shortly. https://issues.apache.org/jira/browse/ZOOKEEPER-4318
@hanm Would you mind taking a quick look at the change?
There was a problem hiding this comment.
@hanm thanks for reviewing and approving the follower_sync_time. Can you approve this one, so the change can be merged?
Author: Li Wang <li4wang@gmail.com>
|
@hanm can you help approving the PR so the changes can be merged? Thanks. |
|
Merged into |
Motivation - With enabling the feature of followers hosting observers, we would need a metric to measure the observer sync time just like what we have for the follower sync time. Changes - Added the "observer_sync_time" metrics - Added unit test on the metrics Author: liwang <liwang@apple.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Michael Han <hanm@apache.org>, Damien Diederen <ddiederen@apache.org> Closes #1691 from li4wang/ZOOKEEPER-4284 (cherry picked from commit 2d30656) Signed-off-by: Damien Diederen <ddiederen@apache.org>
Motivation - With enabling the feature of followers hosting observers, we would need a metric to measure the observer sync time just like what we have for the follower sync time. Changes - Added the "observer_sync_time" metrics - Added unit test on the metrics Author: liwang <liwang@apple.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Michael Han <hanm@apache.org>, Damien Diederen <ddiederen@apache.org> Closes apache#1691 from li4wang/ZOOKEEPER-4284
Motivation - With enabling the feature of followers hosting observers, we would need a metric to measure the observer sync time just like what we have for the follower sync time. Changes - Added the "observer_sync_time" metrics - Added unit test on the metrics Author: liwang <liwang@apple.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Michael Han <hanm@apache.org>, Damien Diederen <ddiederen@apache.org> Closes apache#1691 from li4wang/ZOOKEEPER-4284 Co-authored-by: liwang <liwang@apple.com>
Motivation
Changes