[kudu-CR] [common] small optimisation on PartitionContainsRow
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19333 Change subject: [common] small optimisation on PartitionContainsRow .. [common] small optimisation on PartitionContainsRow This patch updates PartitionSchema::PartitionContainsRow() to avoid calling EncodeColumns() twice for the same data. That's to improve performance of Tablet::CheckRowInTablet() which is in a hot path when inserting data into a tablet. Change-Id: Iaf1d8a9ea533a859d218292a6a0e2a081878 --- M src/kudu/common/partition.cc M src/kudu/common/partition.h 2 files changed, 16 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/19333/1 -- To view, visit http://gerrit.cloudera.org:8080/19333 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iaf1d8a9ea533a859d218292a6a0e2a081878 Gerrit-Change-Number: 19333 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin
[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
[kudu-CR] KUDU-1945 Auto-Incrementing Column
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19097 ) Change subject: KUDU-1945 Auto-Incrementing Column .. Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/client/client-test.cc@9638 PS8, Line 9638: } add some negative test cases, like set value for auto_incrementing_id column when inserting rows, wrong ColumnSchema settings, etc. http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/common/schema.h File src/kudu/common/schema.h: http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/common/schema.h@221 PS8, Line 221: is_auto_incrementing: true if the column is auto-incrementing column. : //Auto-incrementing column cannot be updated but written to by a client and is auto : //filled in by Kudu by incrementing the previous highest written value to the tablet. : //There can only be a single auto-incrementing column per table. move this block to line #215 http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/tablet/tablet_auto_incrementing-test.cc File src/kudu/tablet/tablet_auto_incrementing-test.cc: http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/tablet/tablet_auto_incrementing-test.cc@81 PS8, Line 81: } add some negative test cases here. -- To view, visit http://gerrit.cloudera.org:8080/19097 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1dbde9095da78f6d1bd00adcc0a6e7dd63082bbc Gerrit-Change-Number: 19097 Gerrit-PatchSet: 8 Gerrit-Owner: Abhishek Chennaka Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Thu, 08 Dec 2022 22:23:38 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Auto-Incrementing Column
Wenzhe Zhou has posted comments on this change. (
http://gerrit.cloudera.org:8080/19097 )
Change subject: KUDU-1945 Auto-Incrementing Column
..
Patch Set 8:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/client/client-test.cc
File src/kudu/client/client-test.cc:
http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/client/client-test.cc@9585
PS8, Line 9585: ->NotNull()->AutoIncrementing();
set auto_incrementing column as primary key
http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/client/schema.cc
File src/kudu/client/schema.cc:
http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/client/schema.cc@488
PS8, Line 488: if (data_->auto_incrementing && internal_type != kudu::UINT64) {
auto_incrementing column should be primary_key, consolidate all sanity checks
for auto_incrementing as
if (data_->auto_incrementing) {
if (data_->default_val) {
...
} else if (!data_->primary_key) {
...
} else if (internal_type != kudu::UINT64) {
...
} else if (nullable) {
...
} else if ()
}
http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/common/schema.h
File src/kudu/common/schema.h:
http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/common/schema.h@231
PS8, Line 231: ColumnSchema col_c("c", INT32, false, false, &default_i32);
: // Slice default_str("Hello");
: // ColumnSchema col_d("d", STRING, false, false,
&default_str);
: // ColumnSchema col_e("d", STRING, false, false,
&default_str, false);
is_auto_incrementing should be added as 5-th parameter
http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:
http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/common/schema.cc@301
PS8, Line 301: TODO:(achennaka)
This should be enforced in KuduColumnSpec::ToColumnSchema()
http://gerrit.cloudera.org:8080/#/c/19097/8/src/kudu/common/schema.cc@653
PS8, Line 653: nullable or immutabl
nit: copy/paste error
--
To view, visit http://gerrit.cloudera.org:8080/19097
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1dbde9095da78f6d1bd00adcc0a6e7dd63082bbc
Gerrit-Change-Number: 19097
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Chennaka
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Comment-Date: Thu, 08 Dec 2022 22:05:01 +
Gerrit-HasComments: Yes
[kudu-CR] LookupRpc use the meta cache's client
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19321 ) Change subject: LookupRpc use the meta_cache's client .. LookupRpc use the meta_cache's client If kudu::internal::LookupRpc is consturcted it gets access to two clients, one from the table and one from the meta_cache. They can be different when the table was created with an other client. This can be problematic if KuduSession::Apply(KuduWriteOperation*) is used because the RPC timeout is extracted from the operation's table's client instead of the session's client. Change-Id: Ib5351b6d651af120b3f69a34ab1e40a05e9c535b Reviewed-on: http://gerrit.cloudera.org:8080/19321 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/client/meta_cache.cc 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/19321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ib5351b6d651af120b3f69a34ab1e40a05e9c535b Gerrit-Change-Number: 19321 Gerrit-PatchSet: 4 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] LookupRpc use the meta cache's client
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19321 ) Change subject: LookupRpc use the meta_cache's client .. Patch Set 3: Code-Review+2 Thank you for the fix! -- To view, visit http://gerrit.cloudera.org:8080/19321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5351b6d651af120b3f69a34ab1e40a05e9c535b Gerrit-Change-Number: 19321 Gerrit-PatchSet: 3 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Thu, 08 Dec 2022 20:38:27 + Gerrit-HasComments: No
[kudu-CR] KUDU-3403 Enable kudu CLI to accept specific leader to step down
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19024 ) Change subject: KUDU-3403 Enable kudu CLI to accept specific leader to step down .. KUDU-3403 Enable kudu CLI to accept specific leader to step down Accept UUID as an argument to identify the leader and deduce other information like host, port, etc. This can be useful in scenario when master is out of sync and user wants to choose a specific leader to step down instead of relying on information about leader in the tablet configuration known to master. Change-Id: I40569faa40a8173c51504c7567aa84a6ae1fb64a Reviewed-on: http://gerrit.cloudera.org:8080/19024 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 6 files changed, 144 insertions(+), 19 deletions(-) Approvals: Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/19024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I40569faa40a8173c51504c7567aa84a6ae1fb64a Gerrit-Change-Number: 19024 Gerrit-PatchSet: 11 Gerrit-Owner: Ashwani Raina Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yuqi Du
[kudu-CR] KUDU-3403 Enable kudu CLI to accept specific leader to step down
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19024 ) Change subject: KUDU-3403 Enable kudu CLI to accept specific leader to step down .. Patch Set 10: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/19024/8/src/kudu/tools/kudu-admin-test.cc File src/kudu/tools/kudu-admin-test.cc: http://gerrit.cloudera.org:8080/#/c/19024/8/src/kudu/tools/kudu-admin-test.cc@1397 PS8, Line 1397: string stderr; : > I guess that is just the convention tests follow in this suite. I have rem Cool! That also results in shorter test runtime, and that's good. -- To view, visit http://gerrit.cloudera.org:8080/19024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I40569faa40a8173c51504c7567aa84a6ae1fb64a Gerrit-Change-Number: 19024 Gerrit-PatchSet: 10 Gerrit-Owner: Ashwani Raina Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yuqi Du Gerrit-Comment-Date: Thu, 08 Dec 2022 18:55:34 + Gerrit-HasComments: Yes
[kudu-CR] rpc: plumb JWTs into the RPC layer
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18469 ) Change subject: rpc: plumb JWTs into the RPC layer .. rpc: plumb JWTs into the RPC layer This patch plumbs JWT verification into our C++ RPC negotiation. It is limited in the sense that JWTs can be sent over encrypted channels where the authenticity of the server is not verified yet -- this should be addressed before using in production. Co-authored-by: Zoltan Chovan Change-Id: I252f1e597d9df4408379c3b695f266dbd7f48dcc Reviewed-on: http://gerrit.cloudera.org:8080/18469 Reviewed-by: Wenzhe Zhou Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin --- M src/kudu/client/client.proto M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/client_negotiation.h M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/negotiation.cc M src/kudu/rpc/negotiation.h M src/kudu/rpc/rpc_header.proto M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/security/token.proto M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h A src/kudu/util/jwt.h 15 files changed, 236 insertions(+), 9 deletions(-) Approvals: Wenzhe Zhou: Looks good to me, but someone else must approve Kudu Jenkins: Verified Alexey Serbin: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/18469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I252f1e597d9df4408379c3b695f266dbd7f48dcc Gerrit-Change-Number: 18469 Gerrit-PatchSet: 18 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] rpc: plumb JWTs into the RPC layer
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18469 ) Change subject: rpc: plumb JWTs into the RPC layer .. Patch Set 17: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/18469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I252f1e597d9df4408379c3b695f266dbd7f48dcc Gerrit-Change-Number: 18469 Gerrit-PatchSet: 17 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Thu, 08 Dec 2022 18:50:13 + Gerrit-HasComments: No
[kudu-CR] rpc: plumb JWTs into the RPC layer
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/18469 ) Change subject: rpc: plumb JWTs into the RPC layer .. Patch Set 17: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/18469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I252f1e597d9df4408379c3b695f266dbd7f48dcc Gerrit-Change-Number: 18469 Gerrit-PatchSet: 17 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan Gerrit-Comment-Date: Thu, 08 Dec 2022 18:38:29 + Gerrit-HasComments: No
[kudu-CR] rpc: plumb JWTs into the RPC layer
Zoltan Chovan has posted comments on this change. (
http://gerrit.cloudera.org:8080/18469 )
Change subject: rpc: plumb JWTs into the RPC layer
..
Patch Set 17:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/18469/16/src/kudu/rpc/messenger.h
File src/kudu/rpc/messenger.h:
http://gerrit.cloudera.org:8080/#/c/18469/16/src/kudu/rpc/messenger.h@380
PS16, Line 380: jwt_verifier_.
> Once jwt_verifier_ might be a wrapper around nullptr, this might become an
Ack
http://gerrit.cloudera.org:8080/#/c/18469/16/src/kudu/rpc/server_negotiation.cc
File src/kudu/rpc/server_negotiation.cc:
http://gerrit.cloudera.org:8080/#/c/18469/16/src/kudu/rpc/server_negotiation.cc@504
PS16, Line 504: if (jwt_verifier_) {
> When JWT verifier can be nullptr, this shouldn't be added into the availabl
Done
http://gerrit.cloudera.org:8080/#/c/18469/16/src/kudu/rpc/server_negotiation.cc@727
PS16, Line 727: t_raw()) {
> This can be null if no JWT verifier is installed in the messenger.
Done
--
To view, visit http://gerrit.cloudera.org:8080/18469
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I252f1e597d9df4408379c3b695f266dbd7f48dcc
Gerrit-Change-Number: 18469
Gerrit-PatchSet: 17
Gerrit-Owner: Andrew Wong
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Wenzhe Zhou
Gerrit-Reviewer: Zoltan Chovan
Gerrit-Comment-Date: Thu, 08 Dec 2022 17:53:57 +
Gerrit-HasComments: Yes
[kudu-CR] rpc: plumb JWTs into the RPC layer
Zoltan Chovan has uploaded a new patch set (#17) to the change originally created by Andrew Wong. ( http://gerrit.cloudera.org:8080/18469 ) Change subject: rpc: plumb JWTs into the RPC layer .. rpc: plumb JWTs into the RPC layer This patch plumbs JWT verification into our C++ RPC negotiation. It is limited in the sense that JWTs can be sent over encrypted channels where the authenticity of the server is not verified yet -- this should be addressed before using in production. Co-authored-by: Zoltan Chovan Change-Id: I252f1e597d9df4408379c3b695f266dbd7f48dcc --- M src/kudu/client/client.proto M src/kudu/rpc/client_negotiation.cc M src/kudu/rpc/client_negotiation.h M src/kudu/rpc/messenger.cc M src/kudu/rpc/messenger.h M src/kudu/rpc/negotiation-test.cc M src/kudu/rpc/negotiation.cc M src/kudu/rpc/negotiation.h M src/kudu/rpc/rpc_header.proto M src/kudu/rpc/server_negotiation.cc M src/kudu/rpc/server_negotiation.h M src/kudu/security/token.proto M src/kudu/util/jwt-util.cc M src/kudu/util/jwt-util.h A src/kudu/util/jwt.h 15 files changed, 236 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/69/18469/17 -- To view, visit http://gerrit.cloudera.org:8080/18469 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I252f1e597d9df4408379c3b695f266dbd7f48dcc Gerrit-Change-Number: 18469 Gerrit-PatchSet: 17 Gerrit-Owner: Andrew Wong Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou Gerrit-Reviewer: Zoltan Chovan
[kudu-CR] KUDU-3403 Enable kudu CLI to accept specific leader to step down
Hello Alexey Serbin, Yuqi Du, Kudu Jenkins, Abhishek Chennaka, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19024 to look at the new patch set (#10). Change subject: KUDU-3403 Enable kudu CLI to accept specific leader to step down .. KUDU-3403 Enable kudu CLI to accept specific leader to step down Accept UUID as an argument to identify the leader and deduce other information like host, port, etc. This can be useful in scenario when master is out of sync and user wants to choose a specific leader to step down instead of relying on information about leader in the tablet configuration known to master. Change-Id: I40569faa40a8173c51504c7567aa84a6ae1fb64a --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 6 files changed, 144 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/19024/10 -- To view, visit http://gerrit.cloudera.org:8080/19024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I40569faa40a8173c51504c7567aa84a6ae1fb64a Gerrit-Change-Number: 19024 Gerrit-PatchSet: 10 Gerrit-Owner: Ashwani Raina Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yuqi Du
[kudu-CR] KUDU-3403 Enable kudu CLI to accept specific leader to step down
Ashwani Raina has posted comments on this change. (
http://gerrit.cloudera.org:8080/19024 )
Change subject: KUDU-3403 Enable kudu CLI to accept specific leader to step down
..
Patch Set 9:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/19024/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19024/7//COMMIT_MSG@7
PS7, Line 7: CLI
> nit: CLI
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/kudu-admin-test.cc@1362
PS7, Line 1362: an argum
> an argument
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/kudu-admin-test.cc@1362
PS7, Line 1362: nt to flag
> nit: this becomes non-relevant if the text is read outside of this patch (e
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/kudu-admin-test.cc@1370
PS7, Line 1370:
: // For deterministic control over manual leader selection
: co
> Would be great to have a comment to explain why to add this option.
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/kudu-admin-test.cc@1394
PS7, Line 1394: ASSERT_EQ(leader->uuid(), master_observed_leader->uuid());
> Why to have this step if the ultimate goal is to check how --current_leader
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/kudu-admin-test.cc@1404
PS7, Line 1404: // where master configuration is out
> nit for here and below: use ASSERT_OK(s) instead
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/kudu-admin-test.cc@1422
PS7, Line 1422:
> a leader
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/kudu-admin-test.cc@1422
PS7, Line 1422:
> a replica at other node
Done
http://gerrit.cloudera.org:8080/#/c/19024/8/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:
http://gerrit.cloudera.org:8080/#/c/19024/8/src/kudu/tools/kudu-admin-test.cc@1397
PS8, Line 1397: string stderr;
:
> Don't we have a leader deterministically chosen already by calling StartEle
I guess that is just the convention tests follow in this suite. I have removed
this intermediate step of electing a leader and resumed next step by simply
assigning new_leader_uuid the uuid of leader i.e. leader->uuid(). That also
works fine.
http://gerrit.cloudera.org:8080/#/c/19024/8/src/kudu/tools/kudu-admin-test.cc@1449
PS8, Line 1449: FLAGS_num_replicas =
> I'd think we want to check for some specific error status here, right? May
Done
http://gerrit.cloudera.org:8080/#/c/19024/8/src/kudu/tools/kudu-admin-test.cc@1451
PS8, Line 1451:
: // Wait for the tablet to be running.
: vector tservers;
: AppendValuesFromMap(tablet_servers_, &tservers);
: ASS
> Why do we need this ASSERT_EVENTUALLY scope? I'd think that no change in t
This was added just to ensure that leader remains the same. I can drop this as
we are already checking for error above at line 1449.
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/tool_action_tablet.cc
File src/kudu/tools/tool_action_tablet.cc:
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/tool_action_tablet.cc@174
PS7, Line 174:*curr
> Why to differentiate between whether new_leader_uuid is specified or not if
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/tool_action_tablet.cc@176
PS7, Line 176: {
> style nit: there should be 4 spaces here, not 6; see https://google.github.
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/tool_replica_util.h
File src/kudu/tools/tool_replica_util.h:
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/tool_replica_util.h@95
PS7, Line 95: Get host/port information of the tablet server with the specified
> How about:
Done
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/tool_replica_util.cc
File src/kudu/tools/tool_replica_util.cc:
http://gerrit.cloudera.org:8080/#/c/19024/7/src/kudu/tools/tool_replica_util.cc@209
PS7, Line 209: Substitute("no replica of tablet $0 is hosted by "
: "tablet se
> How about:
Done
--
To view, visit http://gerrit.cloudera.org:8080/19024
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I40569faa40a8173c51504c7567aa84a6ae1fb64a
Gerrit-Change-Number: 19024
Gerrit-PatchSet: 9
Gerrit-Owner: Ashwani Raina
Gerrit-Reviewer: Abhishek Chennaka
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Ashwani Raina
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yuqi Du
Gerrit-Comment-Date: Thu, 08 Dec 2022 09:49:47 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3403 Enable kudu CLI to accept specific leader to step down
Hello Alexey Serbin, Yuqi Du, Kudu Jenkins, Abhishek Chennaka, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/19024 to look at the new patch set (#9). Change subject: KUDU-3403 Enable kudu CLI to accept specific leader to step down .. KUDU-3403 Enable kudu CLI to accept specific leader to step down Accept UUID as an argument to identify the leader and deduce other information like host, port, etc. This can be useful in scenario when master is out of sync and user wants to choose a specific leader to step down instead of relying on information about leader in the tablet configuration known to master. Change-Id: I40569faa40a8173c51504c7567aa84a6ae1fb64a --- M src/kudu/tools/kudu-admin-test.cc M src/kudu/tools/tool_action_common.cc M src/kudu/tools/tool_action_common.h M src/kudu/tools/tool_action_tablet.cc M src/kudu/tools/tool_replica_util.cc M src/kudu/tools/tool_replica_util.h 6 files changed, 144 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/24/19024/9 -- To view, visit http://gerrit.cloudera.org:8080/19024 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I40569faa40a8173c51504c7567aa84a6ae1fb64a Gerrit-Change-Number: 19024 Gerrit-PatchSet: 9 Gerrit-Owner: Ashwani Raina Gerrit-Reviewer: Abhishek Chennaka Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Ashwani Raina Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yuqi Du
