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 2022/12/09 07:32:42 UTC
[kudu-CR] [tools] fix mistake in LeaderStepDown()
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19332
Change subject: [tools] fix mistake in LeaderStepDown()
......................................................................
[tools] fix mistake in LeaderStepDown()
This patch fixes mistake in LeaderStepDown() that lead to SIGSEGV in
some scenarios when using the 'kudu tablet leader_step_down' CLI tool.
This is a follow-up to 7f7e2cf4611e9fb6fe07a57df8762f1ce2e71b8f.
Change-Id: I3ee51c2b99c37f1eec96d94a6538c0fd78f39be2
---
M src/kudu/tools/tool_action_tablet.cc
1 file changed, 6 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/19332/1
--
To view, visit http://gerrit.cloudera.org:8080/19332
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3ee51c2b99c37f1eec96d94a6538c0fd78f39be2
Gerrit-Change-Number: 19332
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
[kudu-CR] [tools] fix mistake in LeaderStepDown()
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/19332 )
Change subject: [tools] fix mistake in LeaderStepDown()
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19332
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ee51c2b99c37f1eec96d94a6538c0fd78f39be2
Gerrit-Change-Number: 19332
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Sun, 11 Dec 2022 10:24:50 +0000
Gerrit-HasComments: No
[kudu-CR] [tools] fix mistake in LeaderStepDown()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19332 )
Change subject: [tools] fix mistake in LeaderStepDown()
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19332/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19332/1//COMMIT_MSG@10
PS1, Line 10: scenarios
> nit: If possible, could you please add the scenario that hit SIGSEGV .
I don't know what's the exact list (that's the issue), but at least the following failed when I ran dist-test last time:
* AdminCliTest.TestLeaderTransferToEvictedPeer
* AdminCliTest.TestGracefulLeaderStepDown
* AdminCliTest.TestLeaderTransferToNonVoter
http://dist-test.cloudera.org/job?job_id=aserbin.1670568093.132444
http://gerrit.cloudera.org:8080/#/c/19332/1/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/19332/1/src/kudu/tools/tool_action_tablet.cc@196
PS1, Line 196: if (new_leader_uuid && leader_uuid == *new_leader_uuid) {
> Thanks for fixing this.
I'm not sure I understand why the same condition applies for new_leader_uuid and current_leader_uuid since their semantics is different. The UUID provided by new_leader_uuid is the target, while current_leader_uuid shows where to send the request for current_leader_uuid to step down, so current_leader_uuid doesn't have semantics of "a new leader to establish", as I understand.
--
To view, visit http://gerrit.cloudera.org:8080/19332
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ee51c2b99c37f1eec96d94a6538c0fd78f39be2
Gerrit-Change-Number: 19332
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 09 Dec 2022 15:52:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] fix mistake in LeaderStepDown()
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/19332 )
Change subject: [tools] fix mistake in LeaderStepDown()
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19332/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19332/1//COMMIT_MSG@10
PS1, Line 10: scenarios
nit: If possible, could you please add the scenario that hit SIGSEGV .
--
To view, visit http://gerrit.cloudera.org:8080/19332
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ee51c2b99c37f1eec96d94a6538c0fd78f39be2
Gerrit-Change-Number: 19332
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 09 Dec 2022 12:15:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] fix mistake in LeaderStepDown()
Posted by "Ashwani Raina (Code Review)" <ge...@cloudera.org>.
Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/19332 )
Change subject: [tools] fix mistake in LeaderStepDown()
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19332/1/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/19332/1/src/kudu/tools/tool_action_tablet.cc@196
PS1, Line 196: if (new_leader_uuid && leader_uuid == *new_leader_uuid) {
Thanks for fixing this.
nit: Not necessary to address in this patch - I am thinking if it would make sense to compare "current_leader_uuid "and "new_leader_uuid". If both are equal, same condition would apply i.e., no need to call DoLeaderStepDown.
Something like this:
if ((new_leader_uuid && leader_uuid == *new_leader_uuid) ||
(current_leader_uuid && new_leader_uuid &&
*current_leader_uuid == *new_leader_uuid) {
cout << Substitute("Requested new leader $0 is already the leader",leader_uuid) << endl;
return Status::OK();
}
--
To view, visit http://gerrit.cloudera.org:8080/19332
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3ee51c2b99c37f1eec96d94a6538c0fd78f39be2
Gerrit-Change-Number: 19332
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Comment-Date: Fri, 09 Dec 2022 11:12:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] [tools] fix mistake in LeaderStepDown()
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19332 )
Change subject: [tools] fix mistake in LeaderStepDown()
......................................................................
[tools] fix mistake in LeaderStepDown()
This patch fixes mistake in LeaderStepDown() that lead to SIGSEGV in
some scenarios when using the 'kudu tablet leader_step_down' CLI tool.
This is a follow-up to 7f7e2cf4611e9fb6fe07a57df8762f1ce2e71b8f.
Change-Id: I3ee51c2b99c37f1eec96d94a6538c0fd78f39be2
Reviewed-on: http://gerrit.cloudera.org:8080/19332
Tested-by: Kudu Jenkins
Reviewed-by: Ashwani Raina <ar...@cloudera.com>
Reviewed-by: Attila Bukor <ab...@apache.org>
---
M src/kudu/tools/tool_action_tablet.cc
1 file changed, 6 insertions(+), 3 deletions(-)
Approvals:
Kudu Jenkins: Verified
Ashwani Raina: Looks good to me, but someone else must approve
Attila Bukor: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/19332
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3ee51c2b99c37f1eec96d94a6538c0fd78f39be2
Gerrit-Change-Number: 19332
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>