Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15249 )
Change subject: [clock] KUDU-3048 add builtin-NTP-specific metrics ...................................................................... Patch Set 3: (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: 9223372036854775807 maybe 2^64-1 would be more clear? http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@506 PS2, Line 506: if (metric_entity) { Isn't metric_entity always non-null? http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@1105 PS2, Line 1105: CLangTidy clang-tidy http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@1107 PS2, Line 1107: sched_yeild sched_yield http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@1109 PS2, Line 1109: sched_yield(); Bear in mind that function metrics are called en masse in response to an access to /metrics, regardless of query parameters. Does sched_yield() add noticeable latency to such calls, especially for the periodic collection into the diagnostics log? http://gerrit.cloudera.org:8080/#/c/15249/2/src/kudu/clock/builtin_ntp.cc@1110 PS2, Line 1110: err rename to indicate that this is ignored? -- 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: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[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 01:09:55 +0000 Gerrit-HasComments: Yes
