Github user jtuple commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/580#discussion_r216049921
--- Diff: src/java/main/org/apache/zookeeper/server/ServerStats.java ---
@@ -33,17 +34,17 @@
public class ServerStats {
private static final Logger LOG =
LoggerFactory.getLogger(ServerStats.class);
- private long packetsSent;
- private long packetsReceived;
- private long maxLatency;
- private long minLatency = Long.MAX_VALUE;
- private long totalLatency = 0;
- private long count = 0;
+ private final AtomicLong packetsSent = new AtomicLong();
--- End diff --
When making this change, we kept all existing metrics "as-is" and only
added new metrics to `ServerMetrics`. The packet sent/receive and
request_latency are both examples of metrics existing prior to this
pull-request which are directly exposed in both in the `/metrics` admin command
(where `ServerMetrics` metrics also report) as well as the `mntr` four letter
command (where `ServerMetrics` metrics do not report).
The only changes to these existing metrics in this pull-request was
converting some of them to `AtomicLong` for minor performance wins.
I'm happy to move both of these metrics to `ServerMetrics` if we want. I'm
not sure if the names will 100% match the current values though. We'd also need
to decide if we're happy losing these metrics in `mntr` or if I should port the
existing `mntr` reporting logic to still query these from `ServerMetrics`.
This is also an open question for all other pre-existing metrics in
`ServerStats`.
---
