You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/10/28 18:40:26 UTC
[GitHub] [ozone] duongkame opened a new pull request, #3911: HDDS-7324. S3G: Avoid calling getS3VolumeContext by using GetGeyInfo …
duongkame opened a new pull request, #3911:
URL: https://github.com/apache/ozone/pull/3911
## What changes were proposed in this pull request?
HDDS-7230 implements the GetKeyInfo with the ability to assume S3 volume context by OM. That releases S3G from having to call `getS3VolumeContext` prior to calling `lookupKey`, as it does today.
Changes include:
- S3G Get Key API to call GetKeyInfo via RpcClient with `assumeS3Context=true`, and without a proper `volumeName`. S3G doesn't need to call GetS3VolumeContext prior to GetKeyInfo.
- RpcClient once it sees an S3 context in the GetKeyInfo response, takes the user principal, and sets it to the [S3LocalAuthContext](https://github.com/apache/ozone/blob/master/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/protocol/ClientProtocol.java#L923-L923).
That code change reduces once roundtrip between S3G and OM for every S3 Object Get/Head request.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7324
## How was this patch tested?
Standard CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] duongkame commented on a diff in pull request #3911: HDDS-7324. S3G: Avoid calling getS3VolumeContext by using GetKeyInfo
Posted by GitBox <gi...@apache.org>.
duongkame commented on code in PR #3911:
URL: https://github.com/apache/ozone/pull/3911#discussion_r1010814047
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -510,7 +511,23 @@ public OzoneVolume getVolumeDetails(String volumeName)
@Override
public S3VolumeContext getS3VolumeContext() throws IOException {
- return ozoneManagerClient.getS3VolumeContext();
+ S3VolumeContext resp = ozoneManagerClient.getS3VolumeContext();
+ String userPrincipal = resp.getUserPrincipal();
+ updateS3Principal(userPrincipal);
+ return resp;
+ }
+
+ private void updateS3Principal(String userPrincipal) {
+ S3Auth s3Auth = this.getThreadLocalS3Auth();
+ // Update user principal if needed to be used for KMS client
+ if (s3Auth != null) {
+ // Update userPrincipal field with the value returned from OM. So that
+ // in multi-tenancy, KMS client can use the correct identity
+ // (instead of using accessId) to communicate with KMS.
+ LOG.debug("Updating S3Auth.userPrincipal to {}", userPrincipal);
+ s3Auth.setUserPrincipal(userPrincipal);
+ this.setThreadLocalS3Auth(s3Auth);
Review Comment:
Basically, when a client calls S3G get api, the client's AwsAccessId is stored as the current UserPrincipal in threadlocal. Then S3G calls OM, through RpcClient, to grab the key information and OM internally resolves the user information to the right Kerberos UserPrincipal and returns to S3G. RpcClient updates this actual Kerberos UserPrincipal in its thread context and uses it to talk to datanodes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime merged pull request #3911: HDDS-7324. S3G: Avoid calling getS3VolumeContext by using GetKeyInfo
Posted by GitBox <gi...@apache.org>.
kerneltime merged PR #3911:
URL: https://github.com/apache/ozone/pull/3911
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on a diff in pull request #3911: HDDS-7324. S3G: Avoid calling getS3VolumeContext by using GetGeyInfo
Posted by GitBox <gi...@apache.org>.
kerneltime commented on code in PR #3911:
URL: https://github.com/apache/ozone/pull/3911#discussion_r1009049571
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -510,7 +511,23 @@ public OzoneVolume getVolumeDetails(String volumeName)
@Override
public S3VolumeContext getS3VolumeContext() throws IOException {
- return ozoneManagerClient.getS3VolumeContext();
+ S3VolumeContext resp = ozoneManagerClient.getS3VolumeContext();
+ String userPrincipal = resp.getUserPrincipal();
+ updateS3Principal(userPrincipal);
+ return resp;
+ }
+
+ private void updateS3Principal(String userPrincipal) {
+ S3Auth s3Auth = this.getThreadLocalS3Auth();
+ // Update user principal if needed to be used for KMS client
+ if (s3Auth != null) {
+ // Update userPrincipal field with the value returned from OM. So that
+ // in multi-tenancy, KMS client can use the correct identity
+ // (instead of using accessId) to communicate with KMS.
+ LOG.debug("Updating S3Auth.userPrincipal to {}", userPrincipal);
Review Comment:
I am wondering if we should audit this? Maybe we can file a Jira for the future to add metrics for each user principal-based counts? cc @smengcl
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] DaveTeng0 commented on a diff in pull request #3911: HDDS-7324. S3G: Avoid calling getS3VolumeContext by using GetGeyInfo
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3911:
URL: https://github.com/apache/ozone/pull/3911#discussion_r1009967160
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -510,7 +511,23 @@ public OzoneVolume getVolumeDetails(String volumeName)
@Override
public S3VolumeContext getS3VolumeContext() throws IOException {
- return ozoneManagerClient.getS3VolumeContext();
+ S3VolumeContext resp = ozoneManagerClient.getS3VolumeContext();
+ String userPrincipal = resp.getUserPrincipal();
+ updateS3Principal(userPrincipal);
+ return resp;
+ }
+
+ private void updateS3Principal(String userPrincipal) {
+ S3Auth s3Auth = this.getThreadLocalS3Auth();
+ // Update user principal if needed to be used for KMS client
+ if (s3Auth != null) {
+ // Update userPrincipal field with the value returned from OM. So that
+ // in multi-tenancy, KMS client can use the correct identity
+ // (instead of using accessId) to communicate with KMS.
+ LOG.debug("Updating S3Auth.userPrincipal to {}", userPrincipal);
+ s3Auth.setUserPrincipal(userPrincipal);
+ this.setThreadLocalS3Auth(s3Auth);
Review Comment:
hmm.. just for myself learning.. so does that mean a same user always access the same rpcClient instance so we use threadLocal variable?
and is it possible a rpcClient instance used by different users?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] DaveTeng0 commented on a diff in pull request #3911: HDDS-7324. S3G: Avoid calling getS3VolumeContext by using GetKeyInfo
Posted by GitBox <gi...@apache.org>.
DaveTeng0 commented on code in PR #3911:
URL: https://github.com/apache/ozone/pull/3911#discussion_r1011097456
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -510,7 +511,23 @@ public OzoneVolume getVolumeDetails(String volumeName)
@Override
public S3VolumeContext getS3VolumeContext() throws IOException {
- return ozoneManagerClient.getS3VolumeContext();
+ S3VolumeContext resp = ozoneManagerClient.getS3VolumeContext();
+ String userPrincipal = resp.getUserPrincipal();
+ updateS3Principal(userPrincipal);
+ return resp;
+ }
+
+ private void updateS3Principal(String userPrincipal) {
+ S3Auth s3Auth = this.getThreadLocalS3Auth();
+ // Update user principal if needed to be used for KMS client
+ if (s3Auth != null) {
+ // Update userPrincipal field with the value returned from OM. So that
+ // in multi-tenancy, KMS client can use the correct identity
+ // (instead of using accessId) to communicate with KMS.
+ LOG.debug("Updating S3Auth.userPrincipal to {}", userPrincipal);
+ s3Auth.setUserPrincipal(userPrincipal);
+ this.setThreadLocalS3Auth(s3Auth);
Review Comment:
got it! thanks Duong for the explanation!!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org