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/12/24 05:13:27 UTC
[kudu-CR] [consensus] KUDU-2839 fix assert in GetParticipantRole()
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14946
Change subject: [consensus] KUDU-2839 fix assert in GetParticipantRole()
......................................................................
[consensus] KUDU-2839 fix assert in GetParticipantRole()
Before this patch, the consistency check in GetParticipantRole() was
incorrect. It tried to search for the replica UUID against pending
config, but replica UUID comes from committed config at the only call
site of the function in CatalogManager::BuildLocationsForTablet().
The pending Raft config might have replica already removed in case if
there is move/replace replica operation in progress. The incorrect
assertion triggered here:
http://dist-test.cloudera.org/job?job_id=aserbin.1577145505.5646
I also did a minor clean up of the code around.
This is a follow-up to 586e957f76a547340f2ab93a7eebc3f116ff825e.
Change-Id: I1a490f5b61dda8cebb8ee9bd9171ef3aa733a148
---
M src/kudu/consensus/quorum_util.cc
M src/kudu/consensus/quorum_util.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
4 files changed, 12 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/14946/1
--
To view, visit http://gerrit.cloudera.org:8080/14946
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1a490f5b61dda8cebb8ee9bd9171ef3aa733a148
Gerrit-Change-Number: 14946
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [consensus] KUDU-2839 fix assert in GetParticipantRole()
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14946 )
Change subject: [consensus] KUDU-2839 fix assert in GetParticipantRole()
......................................................................
Patch Set 2: Code-Review+2
Thanks for fixing this; it was a pretty annoying source of test flakiness.
--
To view, visit http://gerrit.cloudera.org:8080/14946
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a490f5b61dda8cebb8ee9bd9171ef3aa733a148
Gerrit-Change-Number: 14946
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 29 Dec 2019 18:06:17 +0000
Gerrit-HasComments: No
[kudu-CR] [consensus] KUDU-2839 fix assert in GetParticipantRole()
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14946 )
Change subject: [consensus] KUDU-2839 fix assert in GetParticipantRole()
......................................................................
[consensus] KUDU-2839 fix assert in GetParticipantRole()
Before this patch, the consistency check in GetParticipantRole() was
incorrect. It tried to search for the replica UUID against pending
config, but replica UUID comes from committed config at the only call
site of the function in CatalogManager::BuildLocationsForTablet().
The pending Raft config might have replica already removed in case if
there is move/replace replica operation in progress. The incorrect
assertion triggered here:
http://dist-test.cloudera.org/job?job_id=aserbin.1577145505.5646
I also did a minor clean up of the code around.
This is a follow-up to 586e957f76a547340f2ab93a7eebc3f116ff825e.
Change-Id: I1a490f5b61dda8cebb8ee9bd9171ef3aa733a148
Reviewed-on: http://gerrit.cloudera.org:8080/14946
Tested-by: Kudu Jenkins
Reviewed-by: Bankim Bhavsar <ba...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/consensus/quorum_util.cc
M src/kudu/consensus/quorum_util.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
4 files changed, 12 insertions(+), 14 deletions(-)
Approvals:
Kudu Jenkins: Verified
Bankim Bhavsar: Looks good to me, but someone else must approve
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/14946
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I1a490f5b61dda8cebb8ee9bd9171ef3aa733a148
Gerrit-Change-Number: 14946
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [consensus] KUDU-2839 fix assert in GetParticipantRole()
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14946 )
Change subject: [consensus] KUDU-2839 fix assert in GetParticipantRole()
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/14946
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a490f5b61dda8cebb8ee9bd9171ef3aa733a148
Gerrit-Change-Number: 14946
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 26 Dec 2019 22:53:29 +0000
Gerrit-HasComments: No
[kudu-CR] [consensus] KUDU-2839 fix assert in GetParticipantRole()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14946
to look at the new patch set (#2).
Change subject: [consensus] KUDU-2839 fix assert in GetParticipantRole()
......................................................................
[consensus] KUDU-2839 fix assert in GetParticipantRole()
Before this patch, the consistency check in GetParticipantRole() was
incorrect. It tried to search for the replica UUID against pending
config, but replica UUID comes from committed config at the only call
site of the function in CatalogManager::BuildLocationsForTablet().
The pending Raft config might have replica already removed in case if
there is move/replace replica operation in progress. The incorrect
assertion triggered here:
http://dist-test.cloudera.org/job?job_id=aserbin.1577145505.5646
I also did a minor clean up of the code around.
This is a follow-up to 586e957f76a547340f2ab93a7eebc3f116ff825e.
Change-Id: I1a490f5b61dda8cebb8ee9bd9171ef3aa733a148
---
M src/kudu/consensus/quorum_util.cc
M src/kudu/consensus/quorum_util.h
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
4 files changed, 12 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/46/14946/2
--
To view, visit http://gerrit.cloudera.org:8080/14946
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I1a490f5b61dda8cebb8ee9bd9171ef3aa733a148
Gerrit-Change-Number: 14946
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)