Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15014 )
Change subject: [metrics] Fix bugs when metrics do merge ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/15014/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15014/1//COMMIT_MSG@8 PS1, Line 8: : 1. METRIC_merged_entities_count_of_tablet/table/server should never : retire because their value will never change. What happens when the tablet or table is deleted though, and its underlying MetricEntity is destroyed? Shouldn't we allow the metric to retire then? I'm not sure I understand the value of the "never retire" thing anyway. It came from from a time before MetricEntities, when all metrics were in a flat namespace. Now that entities come and go and metrics are bound to entities, what's the point of it? http://gerrit.cloudera.org:8080/#/c/15014/1/src/kudu/util/metrics.h File src/kudu/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/15014/1/src/kudu/util/metrics.h@1106 PS1, Line 1106: virtual void set_value(const T& value) { This shouldn't be virtual; there are no subclasses of AtomicGauge. http://gerrit.cloudera.org:8080/#/c/15014/1/src/kudu/util/metrics.h@1114 PS1, Line 1114: virtual void IncrementBy(int64_t amount) { Likewise. http://gerrit.cloudera.org:8080/#/c/15014/1/src/kudu/util/metrics.h@1251 PS1, Line 1251: UpdateModificationEpoch(); Since the value of a FunctionGauge isn't stored anywhere, we can't track whether it changed or not. So we don't call UpdateModificationEpoch for it at all, and all FunctionGauge metrics are naturally included when metrics are collected, even if MetricJsonOptions.opts.only_modified_in_or_after_epoch has been set. So why should we update the epoch when we detach? That's not even a guarantee that the value has changed; it could be the same as it was pre-detach. -- To view, visit http://gerrit.cloudera.org:8080/15014 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36ee6244964820e3326742c6902a578bf23041d1 Gerrit-Change-Number: 15014 Gerrit-PatchSet: 1 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 13 Jan 2020 23:31:04 +0000 Gerrit-HasComments: Yes
