peterxcli commented on code in PR #11707:
URL: https://github.com/apache/ozone/pull/11707#discussion_r4368084303


##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -3060,7 +3057,6 @@ public OzoneKey headS3Object(String bucketName, String 
keyName)
   public void setThreadLocalS3Auth(
       S3Auth ozoneSharedSecretAuth) {
     ozoneManagerClient.setThreadLocalS3Auth(ozoneSharedSecretAuth);
-    this.s3gUgi = 
UserGroupInformation.createRemoteUser(getThreadLocalS3Auth().getUserPrincipal());

Review Comment:
   this removal actually help to reduce ugi creation, as `GET`, `HEAD`, `list` 
and `delete` request dont call `getRealUserInfo()`



##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -754,8 +750,9 @@ private UserGroupInformation getRealUserInfo() {
     // After HDDS-5881 the user will not be different,
     // as S3G uses single RpcClient. So we should be checking thread-local
     // S3Auth and use it during proxy.
-    if (ozoneManagerClient.getThreadLocalS3Auth() != null) {
-      return s3gUgi;
+    S3Auth s3Auth = ozoneManagerClient.getThreadLocalS3Auth();
+    if (s3Auth != null) {
+      return UserGroupInformation.createRemoteUser(s3Auth.getUserPrincipal());

Review Comment:
   > @peterxcli Thanks for the fix. Is it possible to make s3gUgi to be 
ThreadLocal instead? The argument of the review 
https://github.com/apache/ozone/pull/4188#discussion_r1550036354 is that UGI 
creation is wasteful.
   
   @ivandika3 I think the change that addressed [apache/ozone#4188 
(comment)](https://github.com/apache/ozone/pull/4188#discussion_r1550036354) is 
actually the cause of this bug. It uses a single s3gUgi field for all threads 
that use the RpcClient, so one user’s request can overwrite the owner of 
another user’s key.
   
   Also, after this patch a request uses the UGI at most once, so caching it in 
a ThreadLocal won’t help. If we need it more than once in the future, we can 
add the ThreadLocal then.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to