[kudu-CR] [clock] auto-config of built-in NTP client in cloud

2020-01-22 Thread Alexey Serbin (Code Review) 
Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15070 )

Change subject: [clock] auto-config of built-in NTP client in cloud
..

[clock] auto-config of built-in NTP client in cloud

This patch introduces auto-configuration of the built-in NTP client
in public cloud environment.  Currently, AWS and GCE public cloud types
are supported: Kudu masters and tablet servers are now capable of
auto-detecting per-instance NTP server and using it as reference server
for the built-in NTP client.

The auto-configuration is controlled by the
--builtin_ntp_client_enable_auto_config_in_cloud boolean flag and gated
by the --time_source flag (i.e. the latter should be set to 'builtin'
to allow the auto-configuration to work).

Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb
Reviewed-on: http://gerrit.cloudera.org:8080/15070
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo 
---
M src/kudu/clock/CMakeLists.txt
M src/kudu/clock/builtin_ntp.cc
M src/kudu/clock/builtin_ntp.h
M src/kudu/clock/ntp-test.cc
M src/kudu/mini-cluster/external_mini_cluster-test.cc
5 files changed, 121 insertions(+), 11 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Adar Dembo: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/15070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb
Gerrit-Change-Number: 15070
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [clock] auto-config of built-in NTP client in cloud

2020-01-22 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15070 )

Change subject: [clock] auto-config of built-in NTP client in cloud
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15070/1/src/kudu/clock/builtin_ntp.cc
File src/kudu/clock/builtin_ntp.cc:

http://gerrit.cloudera.org:8080/#/c/15070/1/src/kudu/clock/builtin_ntp.cc@593
PS1, Line 593: "could not parse 
--builtin_ntp_servers flag");
 : }
 : RETURN_NOT_OK(PopulateServers(std::move(hps)));
> I'm not sure these are needed by default if using the NTP server provided b
We could probably skip for now.



--
To view, visit http://gerrit.cloudera.org:8080/15070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb
Gerrit-Change-Number: 15070
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 22 Jan 2020 17:33:14 +
Gerrit-HasComments: Yes

[kudu-CR] [clock] auto-config of built-in NTP client in cloud

2020-01-21 Thread Alexey Serbin (Code Review) 
Hello Kudu Jenkins, Adar Dembo, Grant Henke,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/15070

to look at the new patch set (#2).

Change subject: [clock] auto-config of built-in NTP client in cloud
..

[clock] auto-config of built-in NTP client in cloud

This patch introduces auto-configuration of the built-in NTP client
in public cloud environment.  Currently, AWS and GCE public cloud types
are supported: Kudu masters and tablet servers are now capable of
auto-detecting per-instance NTP server and using it as reference server
for the built-in NTP client.

The auto-configuration is controlled by the
--builtin_ntp_client_enable_auto_config_in_cloud boolean flag and gated
by the --time_source flag (i.e. the latter should be set to 'builtin'
to allow the auto-configuration to work).

Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb
---
M src/kudu/clock/CMakeLists.txt
M src/kudu/clock/builtin_ntp.cc
M src/kudu/clock/builtin_ntp.h
M src/kudu/clock/ntp-test.cc
M src/kudu/mini-cluster/external_mini_cluster-test.cc
5 files changed, 121 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/15070/2
--
To view, visit http://gerrit.cloudera.org:8080/15070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb
Gerrit-Change-Number: 15070
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [clock] auto-config of built-in NTP client in cloud

2020-01-21 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15070 )

Change subject: [clock] auto-config of built-in NTP client in cloud
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15070/1/src/kudu/clock/builtin_ntp.cc
File src/kudu/clock/builtin_ntp.cc:

http://gerrit.cloudera.org:8080/#/c/15070/1/src/kudu/clock/builtin_ntp.cc@593
PS1, Line 593:   
RETURN_NOT_OK_PREPEND(HostPort::ParseStrings(FLAGS_builtin_ntp_servers,
 :
kStandardNtpPort, ),
 : "could not parse 
--builtin_ntp_servers flag");
> Do you think we should also use these servers in addition to the cloud-prov
I'm not sure these are needed by default if using the NTP server provided by 
the cloud infrastructure itself, because these
  * might be blocked by firewall
  * might be too far in terms of RTT and thus wouldn't constitute a set of 
good-enough candidates for time synchronization

Maybe, we should add a flag to add those if really needed?  Not sure what might 
be a use-case that justifies for adding those in addition to the NTP server 
provided from within a cloud instance.



--
To view, visit http://gerrit.cloudera.org:8080/15070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb
Gerrit-Change-Number: 15070
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 21 Jan 2020 20:42:16 +
Gerrit-HasComments: Yes

[kudu-CR] [clock] auto-config of built-in NTP client in cloud

2020-01-18 Thread Adar Dembo (Code Review) 
Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15070 )

Change subject: [clock] auto-config of built-in NTP client in cloud
..


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15070/1/src/kudu/clock/builtin_ntp.cc
File src/kudu/clock/builtin_ntp.cc:

http://gerrit.cloudera.org:8080/#/c/15070/1/src/kudu/clock/builtin_ntp.cc@593
PS1, Line 593:   
RETURN_NOT_OK_PREPEND(HostPort::ParseStrings(FLAGS_builtin_ntp_servers,
 :
kStandardNtpPort, ),
 : "could not parse 
--builtin_ntp_servers flag");
Do you think we should also use these servers in addition to the cloud-provided 
ones?



--
To view, visit http://gerrit.cloudera.org:8080/15070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb
Gerrit-Change-Number: 15070
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Adar Dembo 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 18 Jan 2020 22:24:59 +
Gerrit-HasComments: Yes

[kudu-CR] [clock] auto-config of built-in NTP client in cloud

2020-01-18 Thread Alexey Serbin (Code Review) 
Alexey Serbin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15070


Change subject: [clock] auto-config of built-in NTP client in cloud
..

[clock] auto-config of built-in NTP client in cloud

This patch introduces auto-configuration of the built-in NTP client
in public cloud environment.  Currently, AWS and GCE public cloud types
are supported: Kudu masters and tablet servers are now capable of
auto-detecting per-instance NTP server and using it as reference server
for the built-in NTP client.

The auto-configuration is controlled by the
--builtin_ntp_client_enable_auto_config_in_cloud boolean flag and gated
by the --time_source flag (i.e. the latter should be set to 'builtin'
to allow the auto-configuration to work).

Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb
---
M src/kudu/clock/CMakeLists.txt
M src/kudu/clock/builtin_ntp.cc
M src/kudu/clock/ntp-test.cc
M src/kudu/mini-cluster/external_mini_cluster-test.cc
4 files changed, 118 insertions(+), 7 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/70/15070/1
--
To view, visit http://gerrit.cloudera.org:8080/15070
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0590c0b731a4da2f968e720dea0410d46ab62beb
Gerrit-Change-Number: 15070
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin