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 2018/05/17 02:37:10 UTC
[kudu-CR] KUDU-2443 moving single-replica tablets is broken
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10438
Change subject: KUDU-2443 moving single-replica tablets is broken
......................................................................
KUDU-2443 moving single-replica tablets is broken
Fixed bug in the consensus::ShouldEvictReplica() function for the case
when the leader replica of a tablet with replication factor of 1
(i.e. a non-replicated tablet) is marked with the REPLACE attribute
and there is an extra voter replica in the tablet's Raft config.
Prior to this fix, the master would try to evict the new extra voter
from the configuration and then it would add a new non-voter because
of the consensus::ShouldAddReplica() method's behavior. After the
newly added non-voter replica catches up and is promoted to voter,
that would happen again and again, until the REPLACE attribute
is removed.
Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
---
M src/kudu/consensus/quorum_util-test.cc
M src/kudu/consensus/quorum_util.cc
2 files changed, 85 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/10438/1
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10438 )
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
Patch Set 4: Code-Review+2
> The kudu move tool would not work without http://gerrit.cloudera.org:8080/10439 for a single-replica tablets, so I think the integration test you mentioned is better fit for 10439, not this changelist.
OK, let's do that.
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 18 May 2018 02:20:38 +0000
Gerrit-HasComments: No
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10438 )
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
Patch Set 3: Code-Review+1
LGTM but I think we should add a regression itest for KUDU-2443
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 17 May 2018 21:09:52 +0000
Gerrit-HasComments: No
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10438 )
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
Patch Set 4:
> > LGTM but I think we should add a regression itest for KUDU-2443
>
> the new scenarios in src/kudu/quorum_util-test.cc is
> the regression tests for KUDU-2443.
>
> Particularly, this one is the test I think you mentioned:
Yes but shouldn't we also test the kudu move tool in that scenario as well?
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 17 May 2018 23:47:06 +0000
Gerrit-HasComments: No
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10438 )
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
Patch Set 3:
> LGTM but I think we should add a regression itest for KUDU-2443
That's done -- the new scenarios in src/kudu/quorum_util-test.cc is the regression tests for KUDU-2443.
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 17 May 2018 21:32:44 +0000
Gerrit-HasComments: No
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Mike Percy, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10438
to look at the new patch set (#4).
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
[consensus] KUDU-2443 fix replica replacement of RF=1
Fixed bug in the consensus::ShouldEvictReplica() function for the case
when the leader replica of a tablet with replication factor of 1
is marked with the REPLACE attribute and there is an extra voter
replica in the tablet's Raft config.
Prior to this fix, the master would evict the extra voter from the
configuration and then it would add a new non-voter because of the
consensus::ShouldAddReplica() method's behavior. After the newly
added non-voter replica catches up and becomes a voter, that would
happen again and again, until the REPLACE attribute is removed.
This changelist also includes regression tests to cover the
corresponding functionality. Also, additional test scenarios added
to extend the coverage for the single-replica edge case.
Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
---
M src/kudu/consensus/quorum_util-test.cc
M src/kudu/consensus/quorum_util.cc
2 files changed, 86 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/10438/4
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10438 )
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
Patch Set 5:
> > The kudu move tool would not work without http://gerrit.cloudera.org:8080/10439
> for a single-replica tablets, so I think the integration test you
> mentioned is better fit for 10439, not this changelist.
>
> OK, let's do that.
OK, thanks -- I'll update 10439 with the test then. Or, if it makes sense, I can publish that integration test a separate changelist.
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 18 May 2018 02:42:19 +0000
Gerrit-HasComments: No
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10438 )
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
Patch Set 3:
> > LGTM but I think we should add a regression itest for KUDU-2443
>
> That's done -- the new scenarios in src/kudu/quorum_util-test.cc is
> the regression tests for KUDU-2443.
Particularly, this one is the test I think you mentioned:
{
RaftConfigPB config;
AddPeer(&config, "A", V, '+', {{"REPLACE", true}});
AddPeer(&config, "B", V, '+');
EXPECT_FALSE(ShouldAddReplica(config, 1, policy));
EXPECT_FALSE(ShouldEvictReplica(config, "A", 1, policy));
string to_evict;
ASSERT_TRUE(ShouldEvictReplica(config, "B", 1, policy, &to_evict));
EXPECT_EQ("A", to_evict);
}
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Thu, 17 May 2018 21:34:09 +0000
Gerrit-HasComments: No
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10438 )
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
[consensus] KUDU-2443 fix replica replacement of RF=1
Fixed bug in the consensus::ShouldEvictReplica() function for the case
when the leader replica of a tablet with replication factor of 1
is marked with the REPLACE attribute and there is an extra voter
replica in the tablet's Raft config.
Prior to this fix, the master would evict the extra voter from the
configuration and then it would add a new non-voter because of the
consensus::ShouldAddReplica() method's behavior. After the newly
added non-voter replica catches up and becomes a voter, that would
happen again and again, until the REPLACE attribute is removed.
This changelist also includes regression tests to cover the
corresponding functionality. Also, additional test scenarios added
to extend the coverage for the single-replica edge case.
Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Reviewed-on: http://gerrit.cloudera.org:8080/10438
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/consensus/quorum_util-test.cc
M src/kudu/consensus/quorum_util.cc
2 files changed, 86 insertions(+), 1 deletion(-)
Approvals:
Kudu Jenkins: Verified
Mike Percy: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10438 )
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
Patch Set 4:
> > > LGTM but I think we should add a regression itest for KUDU-2443
> >
> > the new scenarios in src/kudu/quorum_util-test.cc is
> > the regression tests for KUDU-2443.
> >
> > Particularly, this one is the test I think you mentioned:
>
> Yes but shouldn't we also test the kudu move tool in that scenario
> as well?
The kudu move tool would not work without http://gerrit.cloudera.org:8080/10439 for a single-replica tablets, so I think the integration test you mentioned is better fit for 10439, not this changelist.
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 18 May 2018 00:19:04 +0000
Gerrit-HasComments: No
[kudu-CR] [consensus] KUDU-2443 fix replica replacement of RF=1
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Will Berkeley, Mike Percy, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10438
to look at the new patch set (#2).
Change subject: [consensus] KUDU-2443 fix replica replacement of RF=1
......................................................................
[consensus] KUDU-2443 fix replica replacement of RF=1
Fixed bug in the consensus::ShouldEvictReplica() function for the case
when the leader replica of a tablet with replication factor of 1
is marked with the REPLACE attribute and there is an extra voter
replica in the tablet's Raft config.
Prior to this fix, the master would evict the extra voter from the
configuration and then it would add a new non-voter because of the
consensus::ShouldAddReplica() method's behavior. After the newly
added non-voter replica catches up and becomes a voter, that would
happen again and again, until the REPLACE attribute is removed.
Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
---
M src/kudu/consensus/quorum_util-test.cc
M src/kudu/consensus/quorum_util.cc
2 files changed, 85 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/10438/2
--
To view, visit http://gerrit.cloudera.org:8080/10438
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9da9fe6788f28b40f7adc53e23540bcdf103c1ea
Gerrit-Change-Number: 10438
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>