[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-12 Thread Mike Percy (Code Review) 
Mike Percy has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8989 )

Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735
..

[raft_consensus-itest] proper fix for Test_KUDU_1735

Adapted the logic of the Test_KUDU_1735 scenario to work for both
3-2-3 and 3-4-3 replication schemes.

This is a follow-up for 8bcc80eec075321faef26b2a0ccac12ac9d7.

Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
Reviewed-on: http://gerrit.cloudera.org:8080/8989
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy 
---
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/util/fault_injection.cc
2 files changed, 91 insertions(+), 25 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Mike Percy: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
Gerrit-Change-Number: 8989
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-12 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8989 )

Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735
..


Patch Set 4: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
Gerrit-Change-Number: 8989
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Sat, 13 Jan 2018 03:18:48 +
Gerrit-HasComments: No

[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-12 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8989 )

Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735
..


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2428
PS3, Line 2428:   vector tservers;
> nit: this vector isn't really needed anymore
Done


http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2430
PS3, Line 2430:   ASSERT_GE(tservers.size(), 3);
> nit: I don't think this is really required but if so it can just be ASSERT_
Done


http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2432
PS3, Line 2432: tservers[0]
> nit: how about:
Good idea!


http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2477
PS3, Line 2477: attemp
> typo: attempt
Done


http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2497
PS3, Line 2497:   ASSERT_EVENTUALLY([&] {
> nit: this ASSERT_EVENTUALLY can go outside the loop
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
Gerrit-Change-Number: 8989
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Fri, 12 Jan 2018 23:21:42 +
Gerrit-HasComments: Yes

[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-12 Thread Alexey Serbin (Code Review) 
Hello Mike Percy, Kudu Jenkins,

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

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

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

Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735
..

[raft_consensus-itest] proper fix for Test_KUDU_1735

Adapted the logic of the Test_KUDU_1735 scenario to work for both
3-2-3 and 3-4-3 replication schemes.

This is a follow-up for 8bcc80eec075321faef26b2a0ccac12ac9d7.

Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
---
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/util/fault_injection.cc
2 files changed, 91 insertions(+), 25 deletions(-)


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

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

[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-12 Thread Mike Percy (Code Review) 
Mike Percy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8989 )

Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735
..


Patch Set 3:

(5 comments)

looks good, just some small nits

http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc
File src/kudu/integration-tests/raft_consensus-itest.cc:

http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2428
PS3, Line 2428:   vector tservers;
nit: this vector isn't really needed anymore


http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2430
PS3, Line 2430:   ASSERT_GE(tservers.size(), 3);
nit: I don't think this is really required but if so it can just be ASSERT_EQ(3


http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2432
PS3, Line 2432: tservers[0]
nit: how about:

  tablet_servers_[cluster_->tablet_server(0)->uuid()]

here and on the line below?


http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2477
PS3, Line 2477: attemp
typo: attempt


http://gerrit.cloudera.org:8080/#/c/8989/3/src/kudu/integration-tests/raft_consensus-itest.cc@2497
PS3, Line 2497:   ASSERT_EVENTUALLY([&] {
nit: this ASSERT_EVENTUALLY can go outside the loop



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
Gerrit-Change-Number: 8989
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Fri, 12 Jan 2018 21:53:12 +
Gerrit-HasComments: Yes

[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-12 Thread Alexey Serbin (Code Review) 
Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8989 )

Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735
..


Patch Set 3:

> I posted a rough sketch of a fix and an explanation in
 > https://gerrit.cloudera.org/c/9013/

Great, thanks.  This patch already works with the specific of 3-4-3, I just 
wanted to make sure that's this is the legit behavior.

I integrated more perks from the 9013 item, and added explanation on why it 
works the way it's written.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
Gerrit-Change-Number: 8989
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Comment-Date: Fri, 12 Jan 2018 19:02:40 +
Gerrit-HasComments: No

[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-12 Thread Alexey Serbin (Code Review) 
Hello Mike Percy, Kudu Jenkins,

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

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

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

Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735
..

[raft_consensus-itest] proper fix for Test_KUDU_1735

Adapted the logic of the Test_KUDU_1735 scenario to work for both
3-2-3 and 3-4-3 replication schemes.

This is a follow-up for 8bcc80eec075321faef26b2a0ccac12ac9d7.

Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
---
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/util/fault_injection.cc
2 files changed, 89 insertions(+), 21 deletions(-)


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

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

[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-09 Thread Alexey Serbin (Code Review) 
Alexey Serbin has removed Adar Dembo from this change.  ( 
http://gerrit.cloudera.org:8080/8989 )

Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735
..


Removed reviewer Adar Dembo.
--
To view, visit http://gerrit.cloudera.org:8080/8989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
Gerrit-Change-Number: 8989
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy

[kudu-CR] [raft consensus-itest] proper fix for Test KUDU 1735

2018-01-09 Thread Alexey Serbin (Code Review) 
Alexey Serbin has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8989


Change subject: [raft_consensus-itest] proper fix for Test_KUDU_1735
..

[raft_consensus-itest] proper fix for Test_KUDU_1735

Adapted logic of the Test_KUDU_1735 scenario to work for both 3-2-3
and 3-4-3 replication schemes.

This is a follow-up for 8bcc80eec075321faef26b2a0ccac12ac9d7.

Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
---
M src/kudu/integration-tests/raft_consensus-itest.cc
1 file changed, 71 insertions(+), 20 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibd7695f2b78da9755eb109940b40bda2bc6ec02e
Gerrit-Change-Number: 8989
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin