This is an automated email from the ASF dual-hosted git repository. granthenke pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 4bbe2c07b7334a6d657f3ca17d3abb60d3e83ebb Author: Alexey Serbin <[email protected]> AuthorDate: Tue Feb 11 16:20:37 2020 -0800 [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 <[email protected]> Tested-by: Kudu Jenkins --- src/kudu/clock/builtin_ntp.cc | 25 +++++++++++----------- .../mini-cluster/external_mini_cluster-test.cc | 2 +- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/kudu/clock/builtin_ntp.cc b/src/kudu/clock/builtin_ntp.cc index 2f5922c..13e8d94 100644 --- a/src/kudu/clock/builtin_ntp.cc +++ b/src/kudu/clock/builtin_ntp.cc @@ -70,7 +70,7 @@ DEFINE_string(builtin_ntp_servers, "3.pool.ntp.org", "The NTP servers used by the built-in NTP client, in format " "<FQDN|IP>[:PORT]. These will only be used if the built-in NTP " - "client is enabled."); + "client is enabled (i.e. if setting --time_source=builtin)."); // In the 'Best practices' section, RFC 4330 states that 15 seconds is the // minimum allowed polling interval. @@ -107,12 +107,14 @@ DEFINE_uint32(builtin_ntp_true_time_refresh_max_interval_s, 3600, TAG_FLAG(builtin_ntp_true_time_refresh_max_interval_s, experimental); TAG_FLAG(builtin_ntp_true_time_refresh_max_interval_s, runtime); -DEFINE_bool(builtin_ntp_client_enable_auto_config_in_cloud, false, - "Whether to attempt the automatic configuration of the built-in " - "NTP client, pointing it to the internal NTP server accessible " - "from within a cloud instance"); -TAG_FLAG(builtin_ntp_client_enable_auto_config_in_cloud, advanced); -TAG_FLAG(builtin_ntp_client_enable_auto_config_in_cloud, experimental); +DEFINE_bool(builtin_ntp_client_enable_auto_config, false, + "Whether to try auto-discovery of servers for the built-in NTP " + "client. E.g., in case of AWS and GCE cloud instances, use the " + "dedicated NTP server provided by the cloud instance. If " + "auto-discovery fails, the built-in NTP client falls back to using " + "servers specified by the --builtin_ntp_servers flag."); +TAG_FLAG(builtin_ntp_client_enable_auto_config, advanced); +TAG_FLAG(builtin_ntp_client_enable_auto_config, experimental); using kudu::clock::internal::Interval; using kudu::clock::internal::kIntervalNone; @@ -570,7 +572,7 @@ Status BuiltInNtp::InitImpl() { // That's the case when this object has been created using the default // constructor. vector<HostPort> hps; - if (FLAGS_builtin_ntp_client_enable_auto_config_in_cloud) { + if (FLAGS_builtin_ntp_client_enable_auto_config) { // Try to find the instance-only NTP server and configure the built-in // NTP client with it. InstanceDetector detector; @@ -582,10 +584,9 @@ Status BuiltInNtp::InitImpl() { hps.emplace_back(ntp_server, 123); return Status::OK(); }); - WARN_NOT_OK(s, Substitute("auto-configuration of built-in NTP client " - "for cloud instance failed, switching to " - "the default set of NTP servers provided by " - "the --builtin_ntp_servers flag")); + WARN_NOT_OK(s, Substitute("auto-configuration of the built-in NTP client " + "failed: falling back to the set of servers " + "provided by the --builtin_ntp_servers flag")); } if (hps.empty()) { RETURN_NOT_OK_PREPEND(HostPort::ParseStrings(FLAGS_builtin_ntp_servers, diff --git a/src/kudu/mini-cluster/external_mini_cluster-test.cc b/src/kudu/mini-cluster/external_mini_cluster-test.cc index 66aaebe..ab2ba07 100644 --- a/src/kudu/mini-cluster/external_mini_cluster-test.cc +++ b/src/kudu/mini-cluster/external_mini_cluster-test.cc @@ -282,7 +282,7 @@ TEST_P(ExternalMiniClusterTest, TestBasicOperation) { } else { opts.num_ntp_servers = 1; opts.extra_master_flags.emplace_back( - "--builtin_ntp_client_enable_auto_config_in_cloud=true"); + "--builtin_ntp_client_enable_auto_config=true"); } #endif
