Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15249 )
Change subject: [clock] KUDU-3048 add builtin-NTP-specific metrics ...................................................................... Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc File src/kudu/clock/builtin_ntp.cc: http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@117 PS2, Line 117: 2^63-1 when true ti > maybe 2^64-1 would be more clear? Done http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@506 PS2, Line 506: RegisterMetrics(metric_entity); > Isn't metric_entity always non-null? Indeed: in this revision it's always non-null. http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@1105 PS2, Line 1105: s<int64_t > clang-tidy Done http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@1107 PS2, Line 1107: > sched_yield Done http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@1109 PS2, Line 1109: // is reported in milliseconds: don't take into account the timing of the > Bear in mind that function metrics are called en masse in response to an ac It's a good call. Especially given the fact WalltimeWithError() might be waiting a bit on a shared lock. I removed sched_yield() and added a comment. http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@1110 PS2, Line 1110: > rename to indicate that this is ignored? Done -- To view, visit http://gerrit.cloudera.org:8080/15249 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia0a13d58c6e5da50ef7a98ae73de267ec4eab491 Gerrit-Change-Number: 15249 Gerrit-PatchSet: 4 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: Tidy Bot (241) Gerrit-Reviewer: Volodymyr Verovkin <[email protected]> Gerrit-Comment-Date: Thu, 20 Feb 2020 02:41:06 +0000 Gerrit-HasComments: Yes
