You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2020/02/12 00:28:14 UTC
[kudu-CR] [clock] small cleanup on built-in NTP client flags
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15209
Change subject: [clock] small cleanup on built-in NTP client flags
......................................................................
[clock] small cleanup on built-in NTP client flags
This patch renames --builtin_ntp_client_enable_auto_config_in_cloud
flag into --builtin_ntp_client_enable_auto_config and updates the
flag's description to contain more details on the system behavior
behind the flag. I also updated the warning message output upon
falling back to the specified set of NTP servers when automatic
discovery of NTP servers yields no result.
The rationale for this change is to shorten the name of the flag
and be able to use the flag in future for the auto-discovery of
NTP servers provided by other means (e.g., DHCP, etc.).
Change-Id: Iaa419013f047d8ec7803092f8225199e9e6f985b
---
M src/kudu/clock/builtin_ntp.cc
M src/kudu/mini-cluster/external_mini_cluster-test.cc
2 files changed, 16 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/15209/1
--
To view, visit http://gerrit.cloudera.org:8080/15209
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iaa419013f047d8ec7803092f8225199e9e6f985b
Gerrit-Change-Number: 15209
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [clock] small cleanup on built-in NTP client flags
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15209 )
Change subject: [clock] small cleanup on built-in NTP client flags
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/15209
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa419013f047d8ec7803092f8225199e9e6f985b
Gerrit-Change-Number: 15209
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 12 Feb 2020 01:39:13 +0000
Gerrit-HasComments: No
[kudu-CR] [clock] small cleanup on built-in NTP client flags
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15209 )
Change subject: [clock] small cleanup on built-in NTP client flags
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15209/1/src/kudu/clock/builtin_ntp.cc
File src/kudu/clock/builtin_ntp.cc:
http://gerrit.cloudera.org:8080/#/c/15209/1/src/kudu/clock/builtin_ntp.cc@113
PS1, Line 113: the
> don't need
Done
http://gerrit.cloudera.org:8080/#/c/15209/1/src/kudu/clock/builtin_ntp.cc@115
PS1, Line 115: This flag is "
: "gated by setting --time_source=builtin (i.e. it does not affect "
: "anything if --time_source is not set to 'builtin').
> I don't think this is necessary, and it's especially confusing given that i
Yup, was thinking to update it in the follow-up changelist.
But I agree it's better to get rid of this extra detail.
Done.
--
To view, visit http://gerrit.cloudera.org:8080/15209
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa419013f047d8ec7803092f8225199e9e6f985b
Gerrit-Change-Number: 15209
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 12 Feb 2020 01:32:30 +0000
Gerrit-HasComments: Yes
[kudu-CR] [clock] small cleanup on built-in NTP client flags
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15209 )
Change subject: [clock] small cleanup on built-in NTP client flags
......................................................................
[clock] small cleanup on built-in NTP client flags
This patch renames --builtin_ntp_client_enable_auto_config_in_cloud
flag into --builtin_ntp_client_enable_auto_config and updates the
flag's description to contain more details on the system behavior
behind the flag. I also updated the warning message output upon
falling back to the specified set of NTP servers when automatic
discovery of NTP servers yields no result.
The rationale for this change is to shorten the name of the flag
and be able to use the flag in future for the auto-discovery of
NTP servers provided by other means (e.g., DHCP, etc.).
Change-Id: Iaa419013f047d8ec7803092f8225199e9e6f985b
Reviewed-on: http://gerrit.cloudera.org:8080/15209
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/clock/builtin_ntp.cc
M src/kudu/mini-cluster/external_mini_cluster-test.cc
2 files changed, 14 insertions(+), 13 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/15209
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iaa419013f047d8ec7803092f8225199e9e6f985b
Gerrit-Change-Number: 15209
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [clock] small cleanup on built-in NTP client flags
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15209 )
Change subject: [clock] small cleanup on built-in NTP client flags
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/15209/1/src/kudu/clock/builtin_ntp.cc
File src/kudu/clock/builtin_ntp.cc:
http://gerrit.cloudera.org:8080/#/c/15209/1/src/kudu/clock/builtin_ntp.cc@113
PS1, Line 113: the
don't need
http://gerrit.cloudera.org:8080/#/c/15209/1/src/kudu/clock/builtin_ntp.cc@115
PS1, Line 115: This flag is "
: "gated by setting --time_source=builtin (i.e. it does not affect "
: "anything if --time_source is not set to 'builtin').
I don't think this is necessary, and it's especially confusing given that it may also take effect if the time source is 'auto'.
--
To view, visit http://gerrit.cloudera.org:8080/15209
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa419013f047d8ec7803092f8225199e9e6f985b
Gerrit-Change-Number: 15209
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 12 Feb 2020 00:55:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [clock] small cleanup on built-in NTP client flags
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/15209
to look at the new patch set (#2).
Change subject: [clock] small cleanup on built-in NTP client flags
......................................................................
[clock] small cleanup on built-in NTP client flags
This patch renames --builtin_ntp_client_enable_auto_config_in_cloud
flag into --builtin_ntp_client_enable_auto_config and updates the
flag's description to contain more details on the system behavior
behind the flag. I also updated the warning message output upon
falling back to the specified set of NTP servers when automatic
discovery of NTP servers yields no result.
The rationale for this change is to shorten the name of the flag
and be able to use the flag in future for the auto-discovery of
NTP servers provided by other means (e.g., DHCP, etc.).
Change-Id: Iaa419013f047d8ec7803092f8225199e9e6f985b
---
M src/kudu/clock/builtin_ntp.cc
M src/kudu/mini-cluster/external_mini_cluster-test.cc
2 files changed, 14 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/15209/2
--
To view, visit http://gerrit.cloudera.org:8080/15209
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iaa419013f047d8ec7803092f8225199e9e6f985b
Gerrit-Change-Number: 15209
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)