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/03 20:09:46 UTC
[kudu-CR] [test] use built-in NTP by default for ExternalMiniCluster
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14362
Change subject: [test] use built-in NTP by default for ExternalMiniCluster
......................................................................
[test] use built-in NTP by default for ExternalMiniCluster
This patch switches all ExternalMiniCluster-based tests to use
the built-in NTP client as the default source for HybridTime timestamps.
A new environment variable is introduced to override the default when
running tests: KUDU_USE_SYSTEM_NTP. Set it to true if it's desired
to run the ExternalMiniCluster-based scenarios using the machine's local
clock as the source for HybridTime (the clock should be synchronized by
the kernel NTP discipline).
Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
---
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
3 files changed, 26 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/14362/1
--
To view, visit http://gerrit.cloudera.org:8080/14362
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
Gerrit-Change-Number: 14362
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [test] use built-in NTP by default for ExternalMiniCluster
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14362 )
Change subject: [test] use built-in NTP by default for ExternalMiniCluster
......................................................................
Patch Set 3:
As per our Slack discussion, we have a rough consensus that we should default to the built-in NTP client in 1.12.0 (i.e. the release after the imminent release of 1.11.0). Doing that means changing the default value of --time_source and probably ensuring --time_source has an effect on HLC-using subprocesses (like EMCs).
How does this change factor into that plan? Is it a means of increasing built-in NTP client test coverage early? Then why only to EMC-based tests? What about InternalMiniClusters, or tests that set up fewer Kudu pieces but still use an HLC? And if not that, then what's the motivation?
--
To view, visit http://gerrit.cloudera.org:8080/14362
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
Gerrit-Change-Number: 14362
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 04 Oct 2019 21:40:30 +0000
Gerrit-HasComments: No
[kudu-CR] [test] use built-in NTP by default for ExternalMiniCluster
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [test] use built-in NTP by default for ExternalMiniCluster
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/14362
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
Gerrit-Change-Number: 14362
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [test] use built-in NTP by default for ExternalMiniCluster
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14362 )
Change subject: [test] use built-in NTP by default for ExternalMiniCluster
......................................................................
[test] use built-in NTP by default for ExternalMiniCluster
This patch switches all ExternalMiniCluster-based tests to use
the built-in NTP client as the default source for HybridTime timestamps.
A new environment variable is introduced to override the default when
running tests: KUDU_USE_SYSTEM_NTP. Set it to true if it's desired
to run the ExternalMiniCluster-based scenarios using the machine's local
clock as the source for HybridTime (the clock should be synchronized by
the kernel NTP discipline).
Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
Reviewed-on: http://gerrit.cloudera.org:8080/14362
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/util/test_util.cc
M src/kudu/util/test_util.h
3 files changed, 26 insertions(+), 6 deletions(-)
Approvals:
Alexey Serbin: Verified
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14362
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
Gerrit-Change-Number: 14362
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [test] use built-in NTP by default for ExternalMiniCluster
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14362 )
Change subject: [test] use built-in NTP by default for ExternalMiniCluster
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14362
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
Gerrit-Change-Number: 14362
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 04 Oct 2019 16:47:26 +0000
Gerrit-HasComments: No
[kudu-CR] [test] use built-in NTP by default for ExternalMiniCluster
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14362 )
Change subject: [test] use built-in NTP by default for ExternalMiniCluster
......................................................................
Patch Set 1:
> Build Failed
>
> http://jenkins.kudu.apache.org/job/kudu-gerrit/18992/ : FAILURE
It seems I need to remove the directory where chronyd puts its control Unix socket.
--
To view, visit http://gerrit.cloudera.org:8080/14362
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
Gerrit-Change-Number: 14362
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 03 Oct 2019 23:04:09 +0000
Gerrit-HasComments: No
[kudu-CR] [test] use built-in NTP by default for ExternalMiniCluster
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14362 )
Change subject: [test] use built-in NTP by default for ExternalMiniCluster
......................................................................
Patch Set 2: Verified+1
Unrelated test failures:
* org.apache.kudu.spark.kudu.DefaultSourceTest
--
To view, visit http://gerrit.cloudera.org:8080/14362
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
Gerrit-Change-Number: 14362
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 04 Oct 2019 00:26:36 +0000
Gerrit-HasComments: No
[kudu-CR] [test] use built-in NTP by default for ExternalMiniCluster
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14362 )
Change subject: [test] use built-in NTP by default for ExternalMiniCluster
......................................................................
Patch Set 3:
> As per our Slack discussion, we have a rough consensus that we
> should default to the built-in NTP client in 1.12.0 (i.e. the
> release after the imminent release of 1.11.0). Doing that means
> changing the default value of --time_source and probably ensuring
> --time_source has an effect on HLC-using subprocesses (like EMCs).
>
> How does this change factor into that plan? Is it a means of
> increasing built-in NTP client test coverage early? Then why only
> to EMC-based tests? What about InternalMiniClusters, or tests that
> set up fewer Kudu pieces but still use an HLC? And if not that,
> then what's the motivation?
This is the part of the plan to switch all the upstream tests to use built-in NTP client. My impression was that we were going to switch to the internal built-in NTP client in our tests even for 1.11 (is that correct?). I think this change doesn't do any harm" it should even help with some cases when clock cannot be synchronized on test machine within 60 seconds.
Is you question more about 'when do we switch to the built-in NTP client in internal minicluster and other tests', or I just misunderstood something and the actual idea was to switch to the built-in NTP client in our upstream tests only in version 1.12?
--
To view, visit http://gerrit.cloudera.org:8080/14362
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2a93bc3847ff5b81b40106e5074934e26136320a
Gerrit-Change-Number: 14362
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: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 04 Oct 2019 22:20:30 +0000
Gerrit-HasComments: No