You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yifan Zhang (Code Review)" <ge...@cloudera.org> on 2019/11/22 10:37:03 UTC
[kudu-CR] KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
Yifan Zhang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14784
Change subject: KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
......................................................................
KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
When starting a cluster and create a table, some replicas may be
unhealthy, if we run the reblancing, these replicas would not be
moved and the test failed. This patch fixed it by waiting for the
cluster become healthy, and then run the rebalancing.
Change-Id: Icf4b248a5fc91dac0dd6303da08f3b304878bfa2
---
M src/kudu/rebalance/placement_policy_util.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/rebalancer_tool.cc
3 files changed, 32 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/14784/1
--
To view, visit http://gerrit.cloudera.org:8080/14784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icf4b248a5fc91dac0dd6303da08f3b304878bfa2
Gerrit-Change-Number: 14784
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14784 )
Change subject: KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
......................................................................
Patch Set 1:
(5 comments)
Looks good overall, a couple of nits and one question.
http://gerrit.cloudera.org:8080/#/c/14784/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14784/1//COMMIT_MSG@10
PS1, Line 10: if we run the reblancing
If running the rebalancing prior to that
http://gerrit.cloudera.org:8080/#/c/14784/1//COMMIT_MSG@10
PS1, Line 10: reblancing
rebalancing
http://gerrit.cloudera.org:8080/#/c/14784/1//COMMIT_MSG@9
PS1, Line 9: When starting a cluster and create a table, some replicas may be
: unhealthy,
It takes some time for the newly created replicas to bootstrap and get their status reported as 'healthy' in the catalog manager.
http://gerrit.cloudera.org:8080/#/c/14784/1/src/kudu/tools/rebalancer_tool-test.cc
File src/kudu/tools/rebalancer_tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/14784/1/src/kudu/tools/rebalancer_tool-test.cc@309
PS1, Line 309: an
nit: a
http://gerrit.cloudera.org:8080/#/c/14784/1/src/kudu/tools/rebalancer_tool-test.cc@372
PS1, Line 372: ASSERT_EVENTUALLY([&]() {
: ASSERT_TOOL_OK(
: "cluster",
: "ksck",
: cluster_->master()->bound_rpc_addr().ToString()
: );
: });
Is it really necessary? If so, please add a comment explaining the reasoning.
--
To view, visit http://gerrit.cloudera.org:8080/14784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4b248a5fc91dac0dd6303da08f3b304878bfa2
Gerrit-Change-Number: 14784
Gerrit-PatchSet: 1
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 27 Nov 2019 19:58:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14784
to look at the new patch set (#2).
Change subject: KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
......................................................................
KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
It takes some time for the newly created replicas to bootstrap
and get their status reported as 'healthy' in the catalog manager,
if running the rebalancing prior to that, some unhealthy replicas
would not be moved and the test would fail. So we should wait for
the cluster become healthy and then run the rebalancing.
Change-Id: Icf4b248a5fc91dac0dd6303da08f3b304878bfa2
---
M src/kudu/rebalance/placement_policy_util.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/rebalancer_tool.cc
3 files changed, 26 insertions(+), 10 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/14784/2
--
To view, visit http://gerrit.cloudera.org:8080/14784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icf4b248a5fc91dac0dd6303da08f3b304878bfa2
Gerrit-Change-Number: 14784
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14784 )
Change subject: KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
......................................................................
KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
It takes some time for the newly created replicas to bootstrap
and get their status reported as 'healthy' in the catalog manager,
if running the rebalancing prior to that, some unhealthy replicas
would not be moved and the test would fail. So we should wait for
the cluster become healthy and then run the rebalancing.
Change-Id: Icf4b248a5fc91dac0dd6303da08f3b304878bfa2
Reviewed-on: http://gerrit.cloudera.org:8080/14784
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/rebalance/placement_policy_util.cc
M src/kudu/tools/rebalancer_tool-test.cc
M src/kudu/tools/rebalancer_tool.cc
3 files changed, 26 insertions(+), 10 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icf4b248a5fc91dac0dd6303da08f3b304878bfa2
Gerrit-Change-Number: 14784
Gerrit-PatchSet: 3
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
[kudu-CR] KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14784 )
Change subject: KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14784/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14784/1//COMMIT_MSG@9
PS1, Line 9: It takes some time for the newly created replicas to bootstrap
: and get th
> It takes some time for the newly created replicas to bootstrap and get thei
Done
http://gerrit.cloudera.org:8080/#/c/14784/1//COMMIT_MSG@10
PS1, Line 10: ir status reported as 'h
> If running the rebalancing prior to that
Done
http://gerrit.cloudera.org:8080/#/c/14784/1//COMMIT_MSG@10
PS1, Line 10: rted as 'h
> rebalancing
Done
http://gerrit.cloudera.org:8080/#/c/14784/1/src/kudu/tools/rebalancer_tool-test.cc
File src/kudu/tools/rebalancer_tool-test.cc:
http://gerrit.cloudera.org:8080/#/c/14784/1/src/kudu/tools/rebalancer_tool-test.cc@309
PS1, Line 309: a
> nit: a
Done
http://gerrit.cloudera.org:8080/#/c/14784/1/src/kudu/tools/rebalancer_tool-test.cc@372
PS1, Line 372: // Assign two ignored tservers, one of which is unhealthy,
: // '-move_replicas_from_ignored_tservers' and
: // '-output_replica_distribution_details' are both enabled.
: auto* ts = cluster_->tablet_server(1);
: ASSERT_NE(nullptr, ts);
: ts->Shutdown();
: {
> Is it really necessary? If so, please add a comment explaining the reasoni
I thought it's not necessary here and removed it.
--
To view, visit http://gerrit.cloudera.org:8080/14784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4b248a5fc91dac0dd6303da08f3b304878bfa2
Gerrit-Change-Number: 14784
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Thu, 28 Nov 2019 15:30:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/14784 )
Change subject: KUDU-3006: deflake RebalanceIgnoredTserversTest.Basic
......................................................................
Patch Set 2: Code-Review+2
Thank you for the fix!
--
To view, visit http://gerrit.cloudera.org:8080/14784
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf4b248a5fc91dac0dd6303da08f3b304878bfa2
Gerrit-Change-Number: 14784
Gerrit-PatchSet: 2
Gerrit-Owner: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Sun, 01 Dec 2019 01:51:34 +0000
Gerrit-HasComments: No