Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19622 )
Change subject: [multi-tenancy] KUDU-3413 update server key for multi-tenancy ...................................................................... Patch Set 5: (8 comments) http://gerrit.cloudera.org:8080/#/c/19622/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19622/5//COMMIT_MSG@7 PS5, Line 7: [multi-tenancy] KUDU-3413 nit: KUDU-3413 [multi-tenancy] http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs.proto File src/kudu/fs/fs.proto: http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs.proto@51 PS5, Line 51: clear the server_key related configuration items So it's not possible to downgrade Kudu servers once the fs layout has been loaded by a higher version of Kudu binary? Because the server key has been migrated to the default tenant key, act as a kind of "lost" for older version of Kudu. Instead, how about keep the server key fields as before, just add non-default tenant keys to the new added field 'tenants'? http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs_manager.h File src/kudu/fs/fs_manager.h: http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs_manager.h@439 PS5, Line 439: sever nit: server http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs_manager.h@544 PS5, Line 544: Belongs to the default tenant After the repeated field 'tenants' has multiple elements, 'metadata_' doesn't belong to the default tenant any more, right? http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs_manager.cc File src/kudu/fs/fs_manager.cc: http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs_manager.cc@629 PS5, Line 629: for server key upgrade to default tenant The log will print the function name, so it's not needed to add the details manually. http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs_manager.cc@632 PS5, Line 632: RETURN_NOT_OK_PREPEND If the server key removed failed, the tserver could not restart normally, it will crash in L511? http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs_manager.cc@633 PS5, Line 633: for server key upgrade to default tenant Same. http://gerrit.cloudera.org:8080/#/c/19622/5/src/kudu/fs/fs_manager.cc@697 PS5, Line 697: unique_ptr<InstanceMetadataPB> metadata(new InstanceMetadataPB); nit: use std::make_unique instead. See https://isocpp.org/blog/2019/06/quick-q-differences-between-stdmake-unique-and-stdunique-ptr-with-new -- To view, visit http://gerrit.cloudera.org:8080/19622 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e450d73940eb1dbaac6f905a46d6ccd084f15cf Gerrit-Change-Number: 19622 Gerrit-PatchSet: 5 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wang Xixu <[email protected]> Gerrit-Reviewer: Yifan Zhang <[email protected]> Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Reviewer: Yuqi Du <[email protected]> Gerrit-Comment-Date: Mon, 24 Apr 2023 09:38:00 +0000 Gerrit-HasComments: Yes
