[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-28 Thread Mike Percy (Code Review) 
Mike Percy has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8645 )

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..

KUDU-1097 (patch 5b): kudu tablet config_change move should use 3-4-3

This patch adds support for 3-4-3 re-replication to the "replica move"
tool.

Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Reviewed-on: http://gerrit.cloudera.org:8080/8645
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin 
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_config_change-itest.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_tablet.cc
5 files changed, 185 insertions(+), 29 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 7
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-28 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8645 )

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..


Patch Set 6: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 6
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 29 Nov 2017 07:07:04 +
Gerrit-HasComments: No

[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-28 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8645 )

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..


Patch Set 6:

rev 5 addresses feedback from rev 4; rev 6 is just a rebase


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 6
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 29 Nov 2017 05:00:27 +
Gerrit-HasComments: No

[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-28 Thread Mike Percy (Code Review) 
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..

KUDU-1097 (patch 5b): kudu tablet config_change move should use 3-4-3

This patch adds support for 3-4-3 re-replication to the "replica move"
tool.

Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_config_change-itest.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_tablet.cc
5 files changed, 185 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/8645/5
--
To view, visit http://gerrit.cloudera.org:8080/8645
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 5
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-28 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8645 )

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8645/4/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

http://gerrit.cloudera.org:8080/#/c/8645/4/src/kudu/tools/tool_action_tablet.cc@496
PS4, Line 496: }
> nit: add brake once it's clear the peer is still in the config?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Wed, 29 Nov 2017 04:58:25 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-28 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8645 )

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..


Patch Set 4: Code-Review+2

(2 comments)

Just a tiny nit, feel free to ignore.

http://gerrit.cloudera.org:8080/#/c/8645/4/src/kudu/integration-tests/cluster_itest_util.h
File src/kudu/integration-tests/cluster_itest_util.h:

http://gerrit.cloudera.org:8080/#/c/8645/4/src/kudu/integration-tests/cluster_itest_util.h@166
PS4, Line 166: num_voters
That's a good one.


http://gerrit.cloudera.org:8080/#/c/8645/4/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

http://gerrit.cloudera.org:8080/#/c/8645/4/src/kudu/tools/tool_action_tablet.cc@496
PS4, Line 496: }
nit: add brake once it's clear the peer is still in the config?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 28 Nov 2017 23:11:18 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-28 Thread Mike Percy (Code Review) 
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..

KUDU-1097 (patch 5b): kudu tablet config_change move should use 3-4-3

This patch adds support for 3-4-3 re-replication to the "replica move"
tool.

Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_config_change-itest.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_tablet.cc
5 files changed, 184 insertions(+), 29 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/45/8645/4
--
To view, visit http://gerrit.cloudera.org:8080/8645
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 4
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-28 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8645 )

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/kudu-admin-test.cc@216
PS3, Line 216: bool enable_kudu_1097
> nit: consider introducing an enum for that.
ok let me give that a try


http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/kudu-admin-test.cc@265
PS3, Line 265: "--unlock_experimental_flags"
> nit: maybe, instead of adding this regarding the mode, have a pair of comma
Done


http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/kudu-admin-test.cc@266
PS3, Line 266: "--v=0",
> why is it necessary?
i'll see if i can get rid of it


http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/tool_action_tablet.cc@467
PS3, Line 467: RETURN_NOT_OK(proxy->BulkChangeConfig(bulk_req, , ));
> I think it would be more user-friendly if the error in the case of the sour
I don't think that is an issue because it's possible to set "replace" on the 
leader.


http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/tool_action_tablet.cc@487
PS3, Line 487:   for (const auto& peer : cstate.committed_config().peers()) 
{
 : if (peer.attrs().replace()) {
 :   num_marked_replace++;
 :   if (peer.permanent_uuid() == leader_uuid) {
 : // The leader is marked for replacement; Make it 
step down.
 : ignore_result(DoLeaderStepDown(client, tablet_id, 
leader_uuid, leader_hp));
 :   }
 : }
 :   }
> Would it make sense to limit this only to the replica which was marked for
good idea



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Tue, 28 Nov 2017 10:44:19 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-27 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8645 )

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/kudu-admin-test.cc@216
PS3, Line 216: bool enable_kudu_1097
nit: consider introducing an enum for that.


http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/kudu-admin-test.cc@265
PS3, Line 265: "--unlock_experimental_flags"
nit: maybe, instead of adding this regarding the mode, have a pair of command 
flag vectors depending on enable_kudu_1097 and use corresponding one for 
RunKuduTool invocations?


http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/kudu-admin-test.cc@266
PS3, Line 266: "--v=0",
why is it necessary?


http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:

http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/tool_action_tablet.cc@467
PS3, Line 467: RETURN_NOT_OK(proxy->BulkChangeConfig(bulk_req, , ));
I think it would be more user-friendly if the error in the case of the source 
replica being the current leader produced  some actionable output.  I.e., some 
message with the recommendation of first using 'kudu tablet leader_stepdown' 
and then trying to run this action again.

What do you think?


http://gerrit.cloudera.org:8080/#/c/8645/3/src/kudu/tools/tool_action_tablet.cc@487
PS3, Line 487:   for (const auto& peer : cstate.committed_config().peers()) 
{
 : if (peer.attrs().replace()) {
 :   num_marked_replace++;
 :   if (peer.permanent_uuid() == leader_uuid) {
 : // The leader is marked for replacement; Make it 
step down.
 : ignore_result(DoLeaderStepDown(client, tablet_id, 
leader_uuid, leader_hp));
 :   }
 : }
 :   }
Would it make sense to limit this only to the replica which was marked for 
replacement by this session?  Otherwise, it might be possible to get into an 
endless loop of re-elections in case of some mishap or misconfiguration.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Comment-Date: Tue, 28 Nov 2017 00:21:00 +
Gerrit-HasComments: Yes

[kudu-CR] KUDU-1097 (patch 5b): kudu tablet config change move should use 3-4-3

2017-11-27 Thread Mike Percy (Code Review) 
Hello Alexey Serbin, Kudu Jenkins,

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

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

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

Change subject: KUDU-1097 (patch 5b): kudu tablet config_change move should use 
3-4-3
..

KUDU-1097 (patch 5b): kudu tablet config_change move should use 3-4-3

This patch adds support for 3-4-3 re-replication to the "replica move"
tool.

Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
---
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/raft_config_change-itest.cc
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_tablet.cc
5 files changed, 163 insertions(+), 26 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I259aeaf24f60a05b88c5c3e0e11d603aa75646a5
Gerrit-Change-Number: 8645
Gerrit-PatchSet: 3
Gerrit-Owner: Mike Percy 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins