KeDeng has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20103 )

Change subject: [multi-tenancy] add a server key upgrade tool
......................................................................


Patch Set 2:

(7 comments)

Thanks for your reviews.

http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/kudu-tool-test.cc@a1364
PS2, Line 1364:
> Why remove this?
The matching rules here may have some defects, and after adding new commands 
for testing, they may become mismatched. I will try to modify the matching 
logic later.


http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/kudu-tool-test.cc@8095
PS2, Line 8095: TEST_F(ToolTest, TestFsUpgradeEncryptionKey) {
> Add tests to check:
Done


http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/kudu-tool-test.cc@8134
PS2, Line 8134: tenant key
> Also to check tenants_size equals to 1 ?
Done


http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/kudu-tool-test.cc@8138
PS2, Line 8138:     ASSERT_EQ(fs_manager->tenant_id(fs::kDefaultTenantName), 
fs::kDefaultTenantID);
> nit: reorder the parameters like ASSERT_EQ(expect_value, actual_value), it
Done


http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/tool_action_fs.cc
File src/kudu/tools/tool_action_fs.cc:

http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/tool_action_fs.cc@179
PS2, Line 179: fs_opts
> It seems it's not necessary to open the whole containers which is very cost
Done


http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/tool_action_fs.cc@1053
PS2, Line 1053:
> nit: 4 spaces indent.
Done


http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/tool_action_fs.cc@1059
PS2, Line 1059: cluster
> nit: Use "tserver" would be more accurate?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2fe50fdcdf294b5955756dfcb92d3b627534bcc7
Gerrit-Change-Number: 20103
Gerrit-PatchSet: 2
Gerrit-Owner: KeDeng <kdeng...@gmail.com>
Gerrit-Reviewer: KeDeng <kdeng...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <1450306...@qq.com>
Gerrit-Reviewer: Yingchun Lai <laiyingc...@apache.org>
Gerrit-Reviewer: Yuqi Du <shenxingwuy...@gmail.com>
Gerrit-Comment-Date: Sun, 25 Jun 2023 09:09:22 +0000
Gerrit-HasComments: Yes

Reply via email to