You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "duongkame (via GitHub)" <gi...@apache.org> on 2023/10/13 17:17:31 UTC
[PR] HDDS-9447. Redundant ACL checks in getKeyInfo for S3 use case [ozone]
duongkame opened a new pull request, #5440:
URL: https://github.com/apache/ozone/pull/5440
## What changes were proposed in this pull request?
Redundant ACL checks in getKeyInfo for S3 use case. See details in the JIRA.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9447
## How was this patch tested?
Should be covered by the existing tests in 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
Re: [PR] HDDS-9447. Redundant ACL checks in getKeyInfo for S3 use case [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #5440:
URL: https://github.com/apache/ozone/pull/5440#discussion_r1362018476
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3515,9 +3519,18 @@ public S3VolumeContext getS3VolumeContext() throws IOException {
}
}
- // getVolumeInfo() performs acl checks and checks volume existence.
+
+ final OmVolumeArgs volumeInfo;
+ if (internal) {
Review Comment:
A minor nit: can we consider renaming `internal` to something more descriptive, like `skipChecks`?
--
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
Re: [PR] HDDS-9447. Redundant ACL checks in getKeyInfo for S3 use case [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #5440:
URL: https://github.com/apache/ozone/pull/5440
--
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
Re: [PR] HDDS-9447. Redundant ACL checks in getKeyInfo for S3 use case [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5440:
URL: https://github.com/apache/ozone/pull/5440#issuecomment-1771242100
Thanks @duongkame for the patch, @smengcl, @tanvipenumudy, @xBis7 for the review.
--
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
Re: [PR] HDDS-9447. Redundant ACL checks in getKeyInfo for S3 use case [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on code in PR #5440:
URL: https://github.com/apache/ozone/pull/5440#discussion_r1364360846
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3515,9 +3519,18 @@ public S3VolumeContext getS3VolumeContext() throws IOException {
}
}
- // getVolumeInfo() performs acl checks and checks volume existence.
+
+ final OmVolumeArgs volumeInfo;
+ if (internal) {
Review Comment:
Thanks for the recommendation. Updated.
--
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
Re: [PR] HDDS-9447. Redundant ACL checks in getKeyInfo for S3 use case [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #5440:
URL: https://github.com/apache/ozone/pull/5440#issuecomment-1764859092
cc @SaketaChalamchala @tanvipenumudy @swamirishi @xBis7 can you please take a look.
--
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
Re: [PR] HDDS-9447. Redundant ACL checks in getKeyInfo for S3 use case [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #5440:
URL: https://github.com/apache/ozone/pull/5440#discussion_r1364900963
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -3515,9 +3519,19 @@ public S3VolumeContext getS3VolumeContext() throws IOException {
}
}
- // getVolumeInfo() performs acl checks and checks volume existence.
+
+ final OmVolumeArgs volumeInfo;
+ if (skipChecks) {
+ // for internal usages, skip acl checks and metrics.
+ volumeInfo = volumeManager.getVolumeInfo(s3Volume);
+ } else {
+ // if external usages, getVolumeInfo() performs acl checks
+ // and update metrics.
Review Comment:
```suggestion
// for external usages, getVolumeInfo() performs acl checks
// and metric updates.
```
--
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