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 2021/06/11 06:44:27 UTC

[kudu-CR] WIP [clock] change clock source selection for 'auto'

Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17582


Change subject: WIP [clock] change clock source selection for 'auto'
......................................................................

WIP [clock] change clock source selection for 'auto'

This patch changes the logic to select particular time source the
pseudo-source 'auto' used for --time_source.

Before this patch, the 'system' time source would be selected
if a Kudu server --time_source=auto is run on a non-cloud VM
or other environment where the cloud instance detector isn't
aware of a dedicated NTP server (which is usually available
via the host-only interface).

After this patch, the 'builtin' time source would be selected
if a Kudu server runs with --time_source=auto on a non-cloud VM
or other environment where the cloud instance detector isn't
aware of dedicated NTP servers AND the --builtin_ntp_servers flag
is set to a valid value.

WIP:
  * collect initial feedback
  * update/add tests to cover the newly introduced functionality

Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
---
M src/kudu/clock/builtin_ntp.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/clock/ntp-test.cc
3 files changed, 28 insertions(+), 9 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: [clock] change clock source selection for 'auto'
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc@476
PS3, Line 476:       // Switch to the built-in NTP client unless the set of reference servers
In the case the built-in NTP client can't reach the configured ntp servers, should we continue falling back to the system ntp? Is that something we can easily detect?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 21 Jun 2021 19:15:04 +0000
Gerrit-HasComments: Yes

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: [clock] change clock source selection for 'auto'
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc@260
PS3, Line 260:   uint64_t now_latest = GetPhysicalValueMicros(now) + error;
> warning: The right operand of '+' is a garbage value [clang-analyzer-core.U
This is a garbage warning: NowWithErrorOrDie() cannot return not setting the value for 'error'.

Ignored.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Sat, 12 Jun 2021 00:27:09 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP [clock] change clock source selection for 'auto'

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: WIP [clock] change clock source selection for 'auto'
......................................................................


Patch Set 2:

(1 comment)

The overall change/approach makes sense to me.

http://gerrit.cloudera.org:8080/#/c/17582/2/src/kudu/clock/hybrid_clock.h
File src/kudu/clock/hybrid_clock.h:

http://gerrit.cloudera.org:8080/#/c/17582/2/src/kudu/clock/hybrid_clock.h@206
PS2, Line 206:   // The 'builtin_ntp_servers' is used in case of TimeSource::BUILTIN_NTP_SYNC.
this line is no longer needed.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 11 Jun 2021 12:56:27 +0000
Gerrit-HasComments: Yes

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: [clock] change clock source selection for 'auto'
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc@476
PS3, Line 476:       // Switch to the built-in NTP client unless the set of reference servers
> In the case the built-in NTP client can't reach the configured ntp servers,
Sure: it's possible to detect that, but it will require extra time waiting for the built-in client to sync-up and then bail if it could not.

The risk here is that due to transient networking issue some nodes might fallback to the system, while others might be able to get it working with the built-in NTP client.  With that, do you think we need some sort of flag to gate that probing behavior?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 21 Jun 2021 19:20:09 +0000
Gerrit-HasComments: Yes

[kudu-CR] WIP [clock] change clock source selection for 'auto'

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

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

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

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

Change subject: WIP [clock] change clock source selection for 'auto'
......................................................................

WIP [clock] change clock source selection for 'auto'

This patch changes the logic to select particular time source the
pseudo-source 'auto' used for --time_source.

Before this patch, the 'system' time source would be selected
if a Kudu server --time_source=auto is run on a non-cloud VM
or other environment where the cloud instance detector isn't
aware of a dedicated NTP server (which is usually available
via the host-only interface).

After this patch, the 'builtin' time source would be selected
if a Kudu server runs with --time_source=auto on a non-cloud VM
or other environment where the cloud instance detector isn't
aware of dedicated NTP servers AND the --builtin_ntp_servers flag
is set to a valid value.

WIP:
  * collect initial feedback
  * update/add tests to cover the newly introduced functionality

Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
---
M src/kudu/clock/builtin_ntp.cc
M src/kudu/clock/builtin_ntp.h
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/clock/ntp-test.cc
M src/kudu/server/default_path_handlers.cc
6 files changed, 83 insertions(+), 54 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: [clock] change clock source selection for 'auto'
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc@476
PS3, Line 476:       // Switch to the built-in NTP client unless the set of reference servers
> One of the nice things about the current iteration of this is that the sele
Yep, this iteration is deterministic in the terms of current configuration and doesn't depend much on network and other runtime issues.

The expectation is that the built-in client has less strict requirements to synchronize because it's not needed to sync the system clock (assuming the set of the NTP servers is the same for ntpd/chronyd and the built-in client).  With that, it might happen that the built-in NTP client is able to synchronize quite fast, but the system clock cannot synchronized for a long time even if NTP server is latched with the reference NTP servers (e.g., the ntpd is running with '-x' option and the original offset of the local time was several seconds).



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 09 Jul 2021 22:59:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Grant Henke, 

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

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

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

Change subject: [clock] change clock source selection for 'auto'
......................................................................

[clock] change clock source selection for 'auto'

This patch changes the logic to select particular time source when
the pseudo-source 'auto' used for --time_source.

Before this patch, the 'system' time source would be auto-selected if
a Kudu server with --time_source=auto is run in an environment where the
instance detector isn't aware of a dedicated NTP server (those are
usually available via the host-only interface, at least for EC2 and
GCE instances).

After this patch, the 'builtin' time source would be auto-selected
if a Kudu server runs with --time_source=auto in environment where the
instance detector isn't aware of dedicated NTP servers AND the
--builtin_ntp_servers flag is set to a valid value.  Otherwise, if
--builtin_ntp_servers flag is set to an empty/invalid value, 'system'
becomes the auto-selected time source for platforms supporting
get_ntptime() API, otherwise the catch-all case is used to auto-select
'system_unsync' as the time source.

I also added a new test scenario to cover the updated functionality.

Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
---
M src/kudu/clock/builtin_ntp.cc
M src/kudu/clock/builtin_ntp.h
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/clock/ntp-test.cc
M src/kudu/server/default_path_handlers.cc
7 files changed, 146 insertions(+), 60 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/82/17582/3
-- 
To view, visit http://gerrit.cloudera.org:8080/17582
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: [clock] change clock source selection for 'auto'
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 13 Jul 2021 18:22:54 +0000
Gerrit-HasComments: No

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: [clock] change clock source selection for 'auto'
......................................................................


Patch Set 3:

> Patch Set 3: Code-Review+2

Thank you for review, Andrew!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 13 Jul 2021 18:29:48 +0000
Gerrit-HasComments: No

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: [clock] change clock source selection for 'auto'
......................................................................

[clock] change clock source selection for 'auto'

This patch changes the logic to select particular time source when
the pseudo-source 'auto' used for --time_source.

Before this patch, the 'system' time source would be auto-selected if
a Kudu server with --time_source=auto is run in an environment where the
instance detector isn't aware of a dedicated NTP server (those are
usually available via the host-only interface, at least for EC2 and
GCE instances).

After this patch, the 'builtin' time source would be auto-selected
if a Kudu server runs with --time_source=auto in environment where the
instance detector isn't aware of dedicated NTP servers AND the
--builtin_ntp_servers flag is set to a valid value.  Otherwise, if
--builtin_ntp_servers flag is set to an empty/invalid value, 'system'
becomes the auto-selected time source for platforms supporting
get_ntptime() API, otherwise the catch-all case is used to auto-select
'system_unsync' as the time source.

I also added a new test scenario to cover the updated functionality.

Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Reviewed-on: http://gerrit.cloudera.org:8080/17582
Tested-by: Kudu Jenkins
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/clock/builtin_ntp.cc
M src/kudu/clock/builtin_ntp.h
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/clock/hybrid_clock.h
M src/kudu/clock/ntp-test.cc
M src/kudu/server/default_path_handlers.cc
7 files changed, 146 insertions(+), 60 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Andrew Wong: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: [clock] change clock source selection for 'auto'
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/17582/2/src/kudu/clock/hybrid_clock.h
File src/kudu/clock/hybrid_clock.h:

http://gerrit.cloudera.org:8080/#/c/17582/2/src/kudu/clock/hybrid_clock.h@206
PS2, Line 206:   // The 'builtin_ntp_servers' is used in case of TimeSource::BUILTIN_NTP_SYNC.
> this line is no longer needed.
Done


http://gerrit.cloudera.org:8080/#/c/17582/2/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:

http://gerrit.cloudera.org:8080/#/c/17582/2/src/kudu/clock/hybrid_clock.cc@260
PS2, Line 260:   uint64_t now_latest = GetPhysicalValueMicros(now) + error;
> warning: The right operand of '+' is a garbage value [clang-analyzer-core.U
This is a garbage warning: NowWithErrorOrDie() cannot return not setting the value for 'error'.

Ignored.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 11 Jun 2021 22:52:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] [clock] change clock source selection for 'auto'

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17582 )

Change subject: [clock] change clock source selection for 'auto'
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:

http://gerrit.cloudera.org:8080/#/c/17582/3/src/kudu/clock/hybrid_clock.cc@476
PS3, Line 476:       // Switch to the built-in NTP client unless the set of reference servers
> Sure: it's possible to detect that, but it will require extra time waiting 
One of the nice things about the current iteration of this is that the selected time source is entirely dependent on the configured environment, regardless of any networking woes.

Depending on what we want "auto" to be, that might be good enough. If it is meant to pick the best clock for the current environment and current configs, we seem to already be doing the right thing. If the goal is to reduce the number of times there are instances where an unsynchronized clock can crash a server, I suppose that's another story, and that would bring into question the robustness of the built-in clock vs the system clock for initial synchronization. What are our expectations of the builtin's ability to synchronize vs typical "system" deployments? With default deployments, would we expect that if the system client can't synchronize, the built-in client would also have issues synchronizing?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72cb54c488b2aa2c73acd6e6e5f6e50dd5811175
Gerrit-Change-Number: 17582
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 06 Jul 2021 22:09:25 +0000
Gerrit-HasComments: Yes