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

Change subject: KUDU-3413 [multi-tenancy] 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]
Done


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
In the upgrade process, the server key is cleared because the relevant content 
has been written to the default tenant key, which does not mean that the 
information is lost. If the tenant information is saved in two separate places, 
two different processes will need to be processed when using the tenant 
information in the future, which will be very cumbersome.

As for the version downgrade you mentioned, I am not sure if it is a new 
feature development that needs support. If so, please let me know and I will 
think of other ways to handle it.


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
Done


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
There were two considerations before, one being that each tenant has their own 
fs_manager, in this case, the metadata will belong to the corresponding tenant; 
another scenario is that each tenant has their own env, which means that all  
tenants will share fs_manager, considering universality, I have adopted the 
second solution, which means that this metadata needs to exist in a place that 
ordinary tenants cannot access.

For now, I am considering adding restrictions on the acquisition of tenant 
information in the default tenant's metadata to ensure data security.


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
Done


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, i
Yes, this situation requires manual editing of the metadata file.

Do you mean to remove the detection of line 511 so that it can be restarted to 
try again?


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.
Done


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/quic
Done



--
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: Tue, 25 Apr 2023 09:47:30 +0000
Gerrit-HasComments: Yes

Reply via email to