You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "tanvipenumudy (via GitHub)" <gi...@apache.org> on 2025/07/15 07:52:55 UTC
[PR] HDDS-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
tanvipenumudy opened a new pull request, #8809:
URL: https://github.com/apache/ozone/pull/8809
## What changes were proposed in this pull request?
Recon OmTableHandlers currently rely on the full KeyInfo proto for key metadata. However, most of the fields in OMKeyInfo, such as KeyLocationList, FileEncryptionInfoProto, OzoneAclInfo, and FileChecksumProto, are not required and contribute to unnecessary serialization and deserialization overhead.
This inefficiency becomes significant for larger keys or multipart upload (MPU) keys, where the KeyLocationList can be very large. As a result, this can cause performance degradation for clients querying this API.
To address this, OMKeyInfo objects should be replaced with a lighter weight object - ReconBasicOmKeyInfo (based on the KeyInfoProtoLight proto) introduced in [PR #8699](https://github.com/apache/ozone/pull/8699). This lightweight version includes only the necessary fields required for event handling.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13442
## How was this patch tested?
- Existing tests under TestOmTableInsightTask should pass.
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #8809:
URL: https://github.com/apache/ozone/pull/8809#discussion_r2209791775
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ReconBasicOmKeyInfo.java:
##########
@@ -247,6 +247,30 @@ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.Key
return builder.build();
}
+ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.KeyInfo keyInfoProto) {
Review Comment:
Any reason this method is duplicated ?
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy merged PR #8809:
URL: https://github.com/apache/ozone/pull/8809
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #8809:
URL: https://github.com/apache/ozone/pull/8809#discussion_r2209949642
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ReconBasicOmKeyInfo.java:
##########
@@ -247,6 +247,30 @@ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.Key
return builder.build();
}
+ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.KeyInfo keyInfoProto) {
Review Comment:
I see that since PartKeyInfo is using KeyInfo proto and even though you are not using keylocations and other fields, these fields will still be decoded from byte buffer to proto fields. Is this Ok ?
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #8809:
URL: https://github.com/apache/ozone/pull/8809#discussion_r2209791775
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ReconBasicOmKeyInfo.java:
##########
@@ -247,6 +247,30 @@ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.Key
return builder.build();
}
+ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.KeyInfo keyInfoProto) {
Review Comment:
I see that since PartKeyInfo is using KeyInfo proto and even though you are not using keylocations and other fields, these fields will still be decoded from byte buffer to proto fields. Is this Ok ?
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #8809:
URL: https://github.com/apache/ozone/pull/8809#discussion_r2209791775
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ReconBasicOmKeyInfo.java:
##########
@@ -247,6 +247,30 @@ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.Key
return builder.build();
}
+ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.KeyInfo keyInfoProto) {
Review Comment:
Any reason you duplicated this method ?
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on PR #8809:
URL: https://github.com/apache/ozone/pull/8809#issuecomment-3077852122
> Please add class-level JavaDoc to clarify this.
Thank you @ArafatKhan2198 for the review! I believe this is already addressed in PR: https://github.com/apache/ozone/pull/8699, that introduces the `ReconBasicOmKeyInfo` class. I have added a JavaDoc to the newly introduced method with the relevant details.
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #8809:
URL: https://github.com/apache/ozone/pull/8809#discussion_r2222189129
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ReconBasicOmKeyInfo.java:
##########
@@ -247,6 +247,30 @@ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.Key
return builder.build();
}
+ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.KeyInfo keyInfoProto) {
Review Comment:
As discussed offline, created the ticket: [HDDS-13490](https://issues.apache.org/jira/browse/HDDS-13490) for addressing the remaining portion of the optimization!
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #8809:
URL: https://github.com/apache/ozone/pull/8809#discussion_r2210180803
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ReconBasicOmKeyInfo.java:
##########
@@ -247,6 +247,30 @@ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.Key
return builder.build();
}
+ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.KeyInfo keyInfoProto) {
Review Comment:
That's correct, the PR only addresses one half of the issue with performance.
> PartKeyInfo is using KeyInfo proto and even though you are not using keylocations and other fields, these fields will still be decoded from byte buffer to proto fields
To optimize this further, we may need a new implementation of PartKeyInfoMap that uses an updated version of Iterable<PartKeyInfo> based on the KeyInfoProtoLight proto object instead of KeyInfo. However, this could require significant changes, not sure if there's a good way to handle this!
cc: @sumitagrawl
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #8809:
URL: https://github.com/apache/ozone/pull/8809#discussion_r2209819267
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/types/ReconBasicOmKeyInfo.java:
##########
@@ -247,6 +247,30 @@ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.Key
return builder.build();
}
+ public static ReconBasicOmKeyInfo getFromProtobuf(OzoneManagerProtocolProtos.KeyInfo keyInfoProto) {
Review Comment:
Yes, the existing method takes `OzoneManagerProtocolProtos.KeyInfoProtoLight` as an argument while the new overloaded method takes `OzoneManagerProtocolProtos.KeyInfo` as an argument!
--
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-13442. Replace OMKeyInfo with light-weight ReconBasicOmKeyInfo under OmTableHandlers [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on PR #8809:
URL: https://github.com/apache/ozone/pull/8809#issuecomment-3102263320
Thank you @devabhishekpal, @devmadhuu, @priyeshkaratha and @ArafatKhan2198 for reviewing the patch!
--
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