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/11 22:04:12 UTC
[PR] HDDS-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
duongkame opened a new pull request, #5427:
URL: https://github.com/apache/ozone/pull/5427
## What changes were proposed in this pull request?
OM's getKeyInfo API internally does the following steps:
1. Resolve bucket link: confirms that bucket/volume exists, resolve the bucket's real name if the bucket param points to a soft link.
2. ACL check: check if the user has access to the requested resource.
3. Read key metadata from rocks db.
4. Get block container location cache/SCM.
5. Generate block token for read.
Although during (1), bucket/volume info is already retrieved from db/cache, step (2) and (3) still re-read the same information to extract different sub-attribute. e.g. (2) needs to read bucket/volume to get bucket/volume owner name, (3) needs to get bucket layout.
Those redundant reads unnecessarily add a significant cost to API, given that each read involves locking, encoding/decoding results...
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-9297
## How was this patch tested?
I did a quick load test in an internal cluster. This change helped the peak OPPS of OM pure reads (getKeyInfo) increase from **150K** to **190K**.
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5427:
URL: https://github.com/apache/ozone/pull/5427#issuecomment-1766886924
Unit and integration test failures seem to be related.
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on code in PR #5427:
URL: https://github.com/apache/ozone/pull/5427#discussion_r1384214892
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java:
##########
@@ -65,7 +65,8 @@ public interface KeyManager extends OzoneManagerFS, IOzoneAcl {
* @return a OmKeyInfo instance client uses to talk to container.
* @throws IOException
*/
- OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress) throws IOException;
+ OmKeyInfo lookupKey(OmKeyArgs args, BucketLayout bucketLayout,
Review Comment:
Definitely not intentional, I missed it out of the blue.
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #5427:
URL: https://github.com/apache/ozone/pull/5427#issuecomment-1763453398
Thanks @duongkame for working on this. Please check compile error.
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #5427:
URL: https://github.com/apache/ozone/pull/5427
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5427:
URL: https://github.com/apache/ozone/pull/5427#discussion_r1383940396
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java:
##########
@@ -65,7 +65,8 @@ public interface KeyManager extends OzoneManagerFS, IOzoneAcl {
* @return a OmKeyInfo instance client uses to talk to container.
* @throws IOException
*/
- OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress) throws IOException;
+ OmKeyInfo lookupKey(OmKeyArgs args, BucketLayout bucketLayout,
Review Comment:
Thanks @duongkame for updating `lookupKey` with this. Have you intentionally left `getKeyInfo` with the `BucketLayout` parameter?
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on PR #5427:
URL: https://github.com/apache/ozone/pull/5427#issuecomment-1779834287
> Unit and integration test failures seem to be related.
Thanks @adoroszlai . I just fixed all the failing tests.
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on code in PR #5427:
URL: https://github.com/apache/ozone/pull/5427#discussion_r1383710376
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java:
##########
@@ -65,7 +65,8 @@ public interface KeyManager extends OzoneManagerFS, IOzoneAcl {
* @return a OmKeyInfo instance client uses to talk to container.
* @throws IOException
*/
- OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress) throws IOException;
+ OmKeyInfo lookupKey(OmKeyArgs args, BucketLayout bucketLayout,
Review Comment:
That makes sense. Thanks. Changed to pass `ResolvedBucket`, `OmBucketInfo` is a cached thing so I'd avoid passing them around.
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #5427:
URL: https://github.com/apache/ozone/pull/5427#discussion_r1372234209
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/TestCleanupTableInfo.java:
##########
@@ -132,7 +133,9 @@ public void setupOzoneManagerMock()
invocationOnMock -> {
Pair<String, String> pair =
Pair.of(TEST_VOLUME_NAME, TEST_BUCKET_NAME);
Review Comment:
`pair` is no longer used. (And removing it will make `import ...Pair` unused, too.)
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManager.java:
##########
@@ -65,7 +65,8 @@ public interface KeyManager extends OzoneManagerFS, IOzoneAcl {
* @return a OmKeyInfo instance client uses to talk to container.
* @throws IOException
*/
- OmKeyInfo lookupKey(OmKeyArgs args, String clientAddress) throws IOException;
+ OmKeyInfo lookupKey(OmKeyArgs args, BucketLayout bucketLayout,
Review Comment:
Passing `BucketLayout` to `lookupKey`/`getKeyInfo` seems strange to me. I would prefer passing `OmBucketInfo` (or `ResolvedBucket` if more convenient) instead:
1. to hide the implementation detail that we need `bucketLayout` for some internal logic
2. make future changes easier, should we need another bucket property
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on PR #5427:
URL: https://github.com/apache/ozone/pull/5427#issuecomment-1795598209
> Thanks @duongkame for the patch.
Thanks for the detailed review @adoroszlai
--
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-9297. getKeyInfo re-read bucket/volume info multiple times [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #5427:
URL: https://github.com/apache/ozone/pull/5427#issuecomment-1758918936
cc @tanvipenumudy
--
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