Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15215 )
Change subject: [clock] update on Clock interface ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/15215/1/src/kudu/clock/logical_clock.h File src/kudu/clock/logical_clock.h: http://gerrit.cloudera.org:8080/#/c/15215/1/src/kudu/clock/logical_clock.h@51 PS1, Line 51: const scoped_refptr<MetricEntity>& metric_entity = {}); > Why is it optional in the LogicalClock but required in the HybridClock? Cer Because with the newly introduced metrics HybridClock updates them very often, and doing additional 'if (metric)' seems prohibitive. LogicalClock doesn't update any metrics: it only reports on some if requested via the metrics registry. Yes, there are many more tests which don't set up an entity since they don't need to extract metrics from LogicalClock. I'm not sure it's worth updating those pursuing the noble goal of semantics unification, but let me know if you feel strongly otherwise. -- To view, visit http://gerrit.cloudera.org:8080/15215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic4c1944d54bf50e54c06c12e2fb9e57fc452b877 Gerrit-Change-Number: 15215 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin <[email protected]> Gerrit-Comment-Date: Fri, 14 Feb 2020 04:16:15 +0000 Gerrit-HasComments: Yes
