Re: [PR] ZOOKEEPER-4767: New implementation of prometheus quantile metrics based on DataSketches [zookeeper]

["DCjanus (via GitHub)" <gi...@apache.org>]

DCjanus commented on PR #2086:
URL: https://github.com/apache/zookeeper/pull/2086#issuecomment-4341647410

   This PR has been inactive for a while, but I want to leave one data point.
   
   We recently hit the same Prometheus Summary performance issue in our internal ZooKeeper 3.9.2-based fork. We backported the approach from this PR, and it significantly improved our benchmark results. Even unmerged, this PR was very useful for us.
   
   One thing worth mentioning is the default 60s rotation interval. In our fork, we benchmarked the same implementation with 60s, 5s, and 1s intervals under the same high-read workload. We did not see a significant performance difference. With a 1s interval, the rotate path was still only `0.12%` in the leader CPU flame graph.
   
   From an operational perspective, 60s of quantile lag is quite visible in dashboards and incident debugging, while 1s seems acceptable for most practical cases. This might be useful if this PR is picked up again, or if someone tries to move a similar DataSketches-based Summary implementation into the Prometheus Java Client.
   
   Thanks again to the PR author and the reviewers for the work and discussion here. It helped us a lot.
   


-- 
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: notifications-unsubscribe@zookeeper.apache.org

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