Adar Dembo has posted comments on this change. Change subject: Add tablet state summary metrics and fix KUDU-2044 ......................................................................
Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/7618/2//COMMIT_MSG 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? http://gerrit.cloudera.org:8080/#/c/7618/2/src/kudu/util/metrics.h 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 http://gerrit.cloudera.org:8080/7618 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8c82987ffe4f37e8af95972bde97841e44c521d9 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley <wdberke...@gmail.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-HasComments: Yes
