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 2019/10/07 02:37:49 UTC
[kudu-CR] WIP [tests] use system unsync time source for tests by default
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14379
Change subject: WIP [tests] use system_unsync time source for tests by default
......................................................................
WIP [tests] use system_unsync time source for tests by default
This patch switches all KuduTest-derived tests to use 'system_unsync'
time source by default.
There is no need to require NTP-synchronized clock for a generic test
derived from KuduTest. All the participating Kudu masters and tablet
servers are run at the same node, and using the same local clock for
each component makes the synchronisation of clocks unnecessary.
WIP:
* Collect some feedback on this approach.
* Add dedicated test scenarios to cover the specific functionality
attributed to the use of the 'system' (system clock synchronized
with NTP) and the 'builtin' (built-in NTP client) time sources,
correspondingly.
Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
---
M src/kudu/client/client_examples-test.sh
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
7 files changed, 42 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/14379/1
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 4:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/clock/hybrid_clock.cc@61
PS3, Line 61:
: DEFINE_string(time_source, "system",
: "The time source that HybridClock should use. Must be one of "
: "'builtin', 'system', 'system_unsync', or 'mock' "
: "('system_unsync' and 'mock' are for tests only)");
: TAG_FLAG(time_source, evolving);
> Is this part of the comment relevant to this line of code?
Indeed: this information should have been put into test_util.cc.
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/mini-cluster/external_mini_cluster.cc@191
PS3, Line 191: flags->emplace_back(Substitute("--ntp_initial_sync_wait_secs=10"));
> Don't we want to call this when NO_CHRONY is defined?
Yes, I need to re-haul that NO_CHRONY stuff once a new universal time source for tests has appeared.
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/scripts/start_kudu.sh
File src/kudu/scripts/start_kudu.sh:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/scripts/start_kudu.sh@29
PS3, Line 29: Usage:
> Update this.
Done
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/CMakeLists.txt@334
PS3, Line 334: clock
> Not necessary given the way in which the gflag is manipulated.
Good catch: it seems this is a left-over from earlier revisions.
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/test_util.cc@115
PS3, Line 115: component
> components
Done
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/test_util.cc@122
PS3, Line 122: isn't
> gflags
Done
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 4
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: Tue, 03 Dec 2019 22:49:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] use system unsync time source for tests by default
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/14379
to look at the new patch set (#4).
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
[tests] use system_unsync time source for tests by default
This patch switches all KuduTest-derived tests to use 'system_unsync'
time source by default (note that becomes so for test based on both
internal and external mini-clusters). The rationale is explained below.
Essentially, there is no need to require NTP-synchronized clock for
a generic test derived from KuduTest. All the participating Kudu
masters and tablet servers (and their sub-components in case of unit
tests) are run at the same node using the same local wallclock. Using
the same wallclock makes the synchronisation of clocks unnecessary.
The scenarios that verify the functioning of particular Kudu subsystems
when using the 'system' and the 'builtin' clock sources should be put
into a set of dedicated scenarios. That's partially so even now: the
scenarios which require verification of the built-in NTP client
functionality are separated into a dedicated test set. However, it's
necessary to add dedicated test scenarios for the 'system' clock source
and add more coverage for the 'built-in' clock source.
That's exactly what follow-up changelists will do, i.e. add dedicated
test scenarios to cover the specific functionality attributed to the use
of the following clock sources:
* system : local machine's wallclock clock synchronized with NTP
* builtin : independently tracked true time by the built-in NTP client
Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
---
M src/kudu/client/client_examples-test.sh
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/scripts/start_kudu.sh
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
9 files changed, 60 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/14379/4
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 4
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] [tests] use system unsync time source for tests by default
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 2:
Test failures look relevant.
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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, 27 Nov 2019 19:39:47 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14379/4/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/14379/4/src/kudu/mini-cluster/external_mini_cluster.cc@170
PS4, Line 170: Status ExternalMiniCluster::AddTimeSourceFlags(std::vector<std::string>* flags) {
> This is pretty tough to read, and is it even correct? Seems like if NO_CHRO
Whoops, it's certainly a typo.
http://gerrit.cloudera.org:8080/#/c/14379/4/src/kudu/mini-cluster/external_mini_cluster.cc@170
PS4, Line 170: Status ExternalMiniCluster::AddTimeSourceFlags(std::vector<std::string>* flags) {
> This is pretty tough to read, and is it even correct? Seems like if NO_CHRO
Whoops, it's certainly a typo. Good catch.
It should have been
#if defined(NO_CHRONY)
flags->emplace_back("--time_source=system_unsync");
#else
...
#end
I think I need to update the NO_CHRONY piece in a separate patch. Probably, compiling no more compiling out any of the stuff or at least renaming NO_CHRONY to HAVE_CHRONY to make it easier to read.
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 4
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: Tue, 03 Dec 2019 23:51:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: WIP [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 1: Verified+1
Unrelated test failure in Java test for TSAN build:
* org.apache.kudu.client.TestSecurity
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 07 Oct 2019 04:10:04 +0000
Gerrit-HasComments: No
[kudu-CR] WIP [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: WIP [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc@133
PS1, Line 133: FLAGS_time_source = UseSystemNtp() ? "system" : "system_unsync";
> If you set the default value for FLAGS_time_source programmatically, you ca
Yep, I changed the default value for the flag, not current value. Of course, setting current value is not what we want here.
My question was more about whether we want to have control over the default value for FLAGS_time_source via environment variable.
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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: Tue, 08 Oct 2019 15:05:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: WIP [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/clock/hybrid_clock-test.cc
File src/kudu/clock/hybrid_clock-test.cc:
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/clock/hybrid_clock-test.cc@355
PS1, Line 355: FLAGS_time_source = "system";
: clock_.reset(new HybridClock);
: ASSERT_OK(clock_->Init());
> This is in anticipation of making 'builtin' the default value?
Nope: this test scenario implicitly assumed it's run against 'system' time source. It expects ntp-related diagnostics to be output. Once the default time source is switched from 'system' (in this patch it's 'system_unsync'), it's necessary to explicitly turn enable the 'system' time source for this scenario.
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc@133
PS1, Line 133: FLAGS_time_source = UseSystemNtp() ? "system" : "system_unsync";
> Sorta weird to do this in KuduTest (and thus incur the odd "up the tree" de
I tried IsGTest() to set the default value for time_source, and it works if getting rid of probing for the env variable (because the code for the latter lives in kudu_test_util as of now).
However, I thought we might want to allow overriding the time source for our tests via environment variable or alike. At least that could allow running the whole bunch of tests using the specified time source.
Or we don't need that at all?
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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: Tue, 08 Oct 2019 02:16:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 5: Verified+1
The failure in HybridClockTest.TestWaitUntilAfter_TestCase2 (TSAN build) has been addressed in https://gerrit.cloudera.org/#/c/14823/
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 5
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, 04 Dec 2019 05:58:12 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 2:
> Test failures look relevant.
Yep, I'm planning to look at them today. The funny thing is that on CentOS6.6 the failure happens only in release build, in debug build it doesn't happen.
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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, 27 Nov 2019 20:51:30 +0000
Gerrit-HasComments: No
[kudu-CR] WIP [tests] use system unsync time source for tests by default
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: WIP [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/clock/hybrid_clock-test.cc
File src/kudu/clock/hybrid_clock-test.cc:
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/clock/hybrid_clock-test.cc@355
PS1, Line 355: FLAGS_time_source = "system";
: clock_.reset(new HybridClock);
: ASSERT_OK(clock_->Init());
This is in anticipation of making 'builtin' the default value?
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc@133
PS1, Line 133: FLAGS_time_source = UseSystemNtp() ? "system" : "system_unsync";
Sorta weird to do this in KuduTest (and thus incur the odd "up the tree" dependency). Perhaps you can use IsGtest() and do it in HybridClock itself?
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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: Mon, 07 Oct 2019 04:28:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP [tests] use system unsync time source for tests by default
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: WIP [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc@133
PS1, Line 133: FLAGS_time_source = UseSystemNtp() ? "system" : "system_unsync";
> Yep, I changed the default value for the flag, not current value. Of cours
I don't see the need. The environment variable exists so that the user can control the time source; I don't think it matters whether that's done by changing the gflag's "current value" or the "default value".
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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: Tue, 08 Oct 2019 17:35:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] use system unsync time source for tests by default
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/14379
to look at the new patch set (#3).
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
[tests] use system_unsync time source for tests by default
This patch switches all KuduTest-derived tests to use 'system_unsync'
time source by default (note that becomes so for test based on both
internal and external mini-clusters). The rationale is explained below.
Essentially, there is no need to require NTP-synchronized clock for
a generic test derived from KuduTest. All the participating Kudu
masters and tablet servers (and their sub-components in case of unit
tests) are run at the same node using the same local wallclock. Using
the same wallclock makes the synchronisation of clocks unnecessary.
The scenarios that verify the functioning of particular Kudu subsystems
when using the 'system' and the 'builtin' clock sources should be put
into a set of dedicated scenarios. That's partially so even now: the
scenarios which require verification of the built-in NTP client
functionality are separated into a dedicated test set. However, it's
necessary to add dedicated test scenarios for the 'system' clock source
and add more coverage for the 'built-in' clock source.
That's exactly what follow-up changelists will do, i.e. add dedicated
test scenarios to cover the specific functionality attributed to the use
of the following clock sources:
* system : local machine's wallclock clock synchronized with NTP
* builtin : independently tracked true time by the built-in NTP client
Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
---
M src/kudu/client/client_examples-test.sh
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/scripts/start_kudu.sh
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
9 files changed, 57 insertions(+), 35 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/14379/3
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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] [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 5:
> Still not sure if the NO_CHRONY case is handled properly, but the
> !NO_CHRONY case looks correct.
In the NO_CHRONY case tests are using the system_unsync by default, so probably I should remove some cruft.
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 5
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, 04 Dec 2019 00:44:04 +0000
Gerrit-HasComments: No
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 5:
> > Still not sure if the NO_CHRONY case is handled properly, but the
> > !NO_CHRONY case looks correct.
>
> In the NO_CHRONY case tests are using the system_unsync by default,
> so probably I should remove some cruft.
Ah, scratch that -- for tablet server and masters as the components of the external mini-cluster we should specify 'system_unsync' time source explicitly because masters and tablet servers are not derived from KuduTest
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 5
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, 04 Dec 2019 00:48:17 +0000
Gerrit-HasComments: No
[kudu-CR] WIP [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: WIP [tests] use system_unsync time source for tests by default
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tests] use system unsync time source for tests by default
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/14379
to look at the new patch set (#2).
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
[tests] use system_unsync time source for tests by default
This patch switches all KuduTest-derived tests to use 'system_unsync'
time source by default (note that becomes so for test based on both
internal and external mini-clusters). The rationale is explained below.
Essentially, there is no need to require NTP-synchronized clock for
a generic test derived from KuduTest. All the participating Kudu
masters and tablet servers (and their sub-components in case of unit
tests) are run at the same node using the same local wallclock. Using
the same wallclock makes the synchronisation of clocks unnecessary.
The scenarios that verify the functioning of particular Kudu subsystems
when using the 'system' and the 'builtin' clock sources should be put
into a set of dedicated scenarios. That's partially so even now: the
scenarios which require verification of the built-in NTP client
functionality are separated into a dedicated test set. However, it's
necessary to add dedicated test scenarios for the 'system' clock source
and add more coverage for the 'built-in' clock source.
That's exactly what follow-up changelists will do, i.e. add dedicated
test scenarios to cover the specific functionality attributed to the use
of the following clock sources:
* system : local machine's wallclock clock synchronized with NTP
* builtin : independently tracked true time by the built-in NTP client
Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
---
M src/kudu/client/client_examples-test.sh
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/scripts/start_kudu.sh
M src/kudu/util/CMakeLists.txt
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
9 files changed, 49 insertions(+), 31 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/14379/2
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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)
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc@133
PS1, Line 133: FLAGS_time_source = UseSystemNtp() ? "system" : "system_unsync";
> I don't see the need. The environment variable exists so that the user can
I removed the control over the test time source by env variables. Indeed, if adding dedicated test scenarios for the 'system' and 'builtin' time sources, there is no need for overriding the new default value.
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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: Tue, 26 Nov 2019 04:21:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/clock/hybrid_clock.cc
File src/kudu/clock/hybrid_clock.cc:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/clock/hybrid_clock.cc@61
PS3, Line 61: In case of test
: // environment, there is no need to require NTP-synchronized clock to run
: // a generic Kudu test. All the participating test components are run at the
: // same node, and using the same local clock for every component makes the
: // synchronisation of clocks unnecessary, so in tests the time source is set
: // to 'system_unsync' by default: see KuduTest::SetUp() for details.
Is this part of the comment relevant to this line of code?
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/mini-cluster/external_mini_cluster.cc@191
PS3, Line 191: flags->emplace_back(Substitute("--time_source=$0", "system_unsync"));
Don't we want to call this when NO_CHRONY is defined?
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/scripts/start_kudu.sh
File src/kudu/scripts/start_kudu.sh:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/scripts/start_kudu.sh@29
PS3, Line 29: Usage:
Update this.
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/CMakeLists.txt
File src/kudu/util/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/CMakeLists.txt@334
PS3, Line 334: clock
Not necessary given the way in which the gflag is manipulated.
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/test_util.cc@69
PS3, Line 69: DECLARE_string(time_source);
Not used?
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/test_util.cc@115
PS3, Line 115: compoents
components
http://gerrit.cloudera.org:8080/#/c/14379/3/src/kudu/util/test_util.cc@122
PS3, Line 122: glags
gflags
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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)
Gerrit-Comment-Date: Mon, 02 Dec 2019 05:00:09 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 5
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] [tests] use system unsync time source for tests by default
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/14379
to look at the new patch set (#5).
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
[tests] use system_unsync time source for tests by default
This patch switches all KuduTest-derived tests to use 'system_unsync'
time source by default (note that becomes so for test based on both
internal and external mini-clusters). The rationale is explained below.
Essentially, there is no need to require NTP-synchronized clock for
a generic test derived from KuduTest. All the participating Kudu
masters and tablet servers (and their sub-components in case of unit
tests) are run at the same node using the same local wallclock. Using
the same wallclock makes the synchronisation of clocks unnecessary.
The scenarios that verify the functioning of particular Kudu subsystems
when using the 'system' and the 'builtin' clock sources should be put
into a set of dedicated scenarios. That's partially so even now: the
scenarios which require verification of the built-in NTP client
functionality are separated into a dedicated test set. However, it's
necessary to add dedicated test scenarios for the 'system' clock source
and add more coverage for the 'built-in' clock source.
That's exactly what follow-up changelists will do, i.e. add dedicated
test scenarios to cover the specific functionality attributed to the use
of the following clock sources:
* system : local machine's wallclock clock synchronized with NTP
* builtin : independently tracked true time by the built-in NTP client
Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
---
M src/kudu/client/client_examples-test.sh
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/scripts/start_kudu.sh
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
8 files changed, 59 insertions(+), 42 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/79/14379/5
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 5
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] [tests] use system unsync time source for tests by default
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
[tests] use system_unsync time source for tests by default
This patch switches all KuduTest-derived tests to use 'system_unsync'
time source by default (note that becomes so for test based on both
internal and external mini-clusters). The rationale is explained below.
Essentially, there is no need to require NTP-synchronized clock for
a generic test derived from KuduTest. All the participating Kudu
masters and tablet servers (and their sub-components in case of unit
tests) are run at the same node using the same local wallclock. Using
the same wallclock makes the synchronisation of clocks unnecessary.
The scenarios that verify the functioning of particular Kudu subsystems
when using the 'system' and the 'builtin' clock sources should be put
into a set of dedicated scenarios. That's partially so even now: the
scenarios which require verification of the built-in NTP client
functionality are separated into a dedicated test set. However, it's
necessary to add dedicated test scenarios for the 'system' clock source
and add more coverage for the 'built-in' clock source.
That's exactly what follow-up changelists will do, i.e. add dedicated
test scenarios to cover the specific functionality attributed to the use
of the following clock sources:
* system : local machine's wallclock clock synchronized with NTP
* builtin : independently tracked true time by the built-in NTP client
Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Reviewed-on: http://gerrit.cloudera.org:8080/14379
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Tested-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/client/client_examples-test.sh
M src/kudu/clock/hybrid_clock-test.cc
M src/kudu/clock/hybrid_clock.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
M src/kudu/scripts/start_kudu.sh
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
8 files changed, 59 insertions(+), 42 deletions(-)
Approvals:
Adar Dembo: Looks good to me, approved
Alexey Serbin: Verified
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 6
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] WIP [tests] use system unsync time source for tests by default
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: WIP [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/14379/1/src/kudu/util/test_util.cc@133
PS1, Line 133: FLAGS_time_source = UseSystemNtp() ? "system" : "system_unsync";
> I tried IsGTest() to set the default value for time_source, and it works if
If you set the default value for FLAGS_time_source programmatically, you can tell gflags just to change its default value rather than its current value. That'll allow users to continue to set --time_source on the command line for tests and override whatever we stipulate in the IsGTest() block.
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
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: Tue, 08 Oct 2019 03:41:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14379/4/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/14379/4/src/kudu/mini-cluster/external_mini_cluster.cc@170
PS4, Line 170: Status ExternalMiniCluster::AddTimeSourceFlags(std::vector<std::string>* flags) {
This is pretty tough to read, and is it even correct? Seems like if NO_CHRONY is _not_ defined, we'll always use system_unsync and the code to look at num_ntp_servers and configure the built-in servers is compiled out.
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 4
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: Tue, 03 Dec 2019 23:18:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tests] use system unsync time source for tests by default
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14379 )
Change subject: [tests] use system_unsync time source for tests by default
......................................................................
Patch Set 5: Code-Review+2
Still not sure if the NO_CHRONY case is handled properly, but the !NO_CHRONY case looks correct.
--
To view, visit http://gerrit.cloudera.org:8080/14379
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7edbf884afaa19121aa92a4ce93c8a7eeb2d937
Gerrit-Change-Number: 14379
Gerrit-PatchSet: 5
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, 04 Dec 2019 00:06:28 +0000
Gerrit-HasComments: No