Yingchun Lai 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) 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? 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: - upgrade encryption key on servers which didn't enable rest encryption. - upgrade encryption key on servers which are already in tenant key version. 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 ? 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 would improve the readability if assert failed. 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 costly, set 'skip_block_manager' to true. http://gerrit.cloudera.org:8080/#/c/20103/2/src/kudu/tools/tool_action_fs.cc@1053 PS2, Line 1053: nit: 4 spaces indent. 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? -- 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 <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Sun, 25 Jun 2023 04:01:54 +0000 Gerrit-HasComments: Yes
