Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10548 )
Change subject: KUDU-2242 Wait for NTP synchronization on startup ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/10548/3/src/kudu/clock/system_ntp.cc File src/kudu/clock/system_ntp.cc: http://gerrit.cloudera.org:8080/#/c/10548/3/src/kudu/clock/system_ntp.cc@113 PS3, Line 113: WaitForNtp BTW, how do you know whether chronyd or ntpd is used to synchronize the clock if both binaries are present? Or we just rely on the way how the packaging for ubiquitous Linux distros build and assume they conflict with each other? http://gerrit.cloudera.org:8080/#/c/10548/3/src/kudu/clock/system_ntp.cc@122 PS3, Line 122: . nit: drop? http://gerrit.cloudera.org:8080/#/c/10548/3/src/kudu/clock/system_ntp.cc@133 PS3, Line 133: "/sbin", "/usr/sbin" It seems the 'chronyc' is usually in /usr/bin for Linux distros: https://pkgs.org/download/chrony Consider adding '/usr/bin' into the list of search paths. http://gerrit.cloudera.org:8080/#/c/10548/3/src/kudu/clock/system_ntp.cc@185 PS3, Line 185: s = WaitForNtp(); : if (s.ok()) { : s = CallAdjTime(&timex); : } nit: consider using Status::AndThen() -- To view, visit http://gerrit.cloudera.org:8080/10548 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6082c5d35ed0d230d91c61734e7b5a351e50933b Gerrit-Change-Number: 10548 Gerrit-PatchSet: 3 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Fri, 01 Jun 2018 18:57:58 +0000 Gerrit-HasComments: Yes
