Adar Dembo has posted comments on this change.

Change subject: Add tablet state summary metrics and fix KUDU-2044

Patch Set 2:

Commit Message:

PS2, Line 11: The numbers are computed by the heartbeater. There's two reasons
            : for this:
            : 1. the heartbeater was already computing the number of RUNNING
            : and BOOTSTRAPPING tablets by holding the appropriate lock and
            : iterating over the tablet map
            : 2. the alternative to having some thread periodically iterate
            : over the tablet map is to increment and decrement the metrics
            : when tablets transition states. This is error-prone,
            : particularly if new states are added, and mistakes will
            : accumulate until the metric is worse than useless.
I appreciate the justification, but I don't think the heartbeater is a great 
place either, since there's one heartbeating thread per master. So your current 
approach instantiates each metric multiple times (not to mention the 
unnecessary updates incurred from having more than one  heartbeating thread).

PS2, Line 23: A metric entity can now be
            : marked as hidden, so it will not print to /metrics, and
            : tablet metric entities are marked as hidden when the tablet is
            : tombstoned, and un-hidden if and when the tablet is revived.
Why hide them and not remove them outright? Don't "hidden" entities still add 
ever-growing load given how we do tombstoning?
File src/kudu/util/metrics.h:

Line 533:   std::atomic<bool> hidden_;
The rest of the file uses util/atomic.h for atomics; please do the same here.

Also, are hidden/set_hidden accessed often? If not, could also make this a 
simple bool and protect with lock_.

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I8c82987ffe4f37e8af95972bde97841e44c521d9
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <>
Gerrit-Reviewer: Adar Dembo <>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <>
Gerrit-Reviewer: Will Berkeley <>
Gerrit-HasComments: Yes

Reply via email to