You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "devabhishekpal (via GitHub)" <gi...@apache.org> on 2026/03/05 05:05:48 UTC
[PR] HDDS-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
devabhishekpal opened a new pull request, #9867:
URL: https://github.com/apache/ozone/pull/9867
## What changes were proposed in this pull request?
HDDS-14659. Add new proto message for lightweight MutlipartPartInfo
Please describe your PR in detail:
* Currently the PartKeyInfo message is a huge object including several repeated fields from KeyInfo. This is not required as the parent MultipartKeyInfo message already includes some common fields
* As part of this patch we are introducing a new proto message `MultipartPartInfo`. This contains the absolute essential fields required for any given part and drops any extra fields.
* We also update `MultipartKeyInfo` message to pull in common metadata fields like `volumeName`, `bucketName`, `keyName`.
* We also add a field `schemaVersion` which will help us with deciding whether to use a new flow for writing MPU parts, or use the currently approach of storing parts inline.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-14659
## How was this patch tested?
Patch was tested via unit tests, but currently this change will not affect any behaviour.
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2889047195
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ required string partName = 1;
+ required uint32 partNumber = 2;
+ repeated KeyLocationList keyLocationList = 6;
+ required uint64 dataSize = 7;
+ required uint64 modificationTime = 8;
+ required uint64 objectID = 9;
+ required uint64 updateID = 10;
Review Comment:
Thanks @adoroszlai for the detailed explanation offline. I have set all fields to optional. We can enforce the required fields in the codecs.
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2894702690
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
Review Comment:
Addressed in the latest commit
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2888433786
##########
hadoop-ozone/interface-storage/src/main/proto/OmStorageProtocol.proto:
##########
@@ -30,6 +30,7 @@ option java_generate_equals_and_hash = true;
package hadoop.ozone;
import "hdds.proto";
+import "OmClientProtocol.proto";
Review Comment:
> Are you okay with keeping multipartinfo in OmClient then?
Yes... Sorry for the back and forth.
> We can remove the OzoneAclInfo from storage protos
No, that would defeat the purpose of `interface-storage`.
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "rakeshadr (via GitHub)" <gi...@apache.org>.
rakeshadr commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2895272007
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
Review Comment:
Agreed! Thanks for the useful links.
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2895199396
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
Review Comment:
So this was pointed out by Attila, one problem with keeping as `required` fields is:
- This makes it harder to migrate or change fields later down the line, like if say the field type needs to be changed, or partName needs to be split or something like that - older clients will always expect part name which will break that.
- proto3 no longer supports `required` fields and it is deprecated.
Instead the required fields will be enforced at the CODEC / application level and throw error in case they are empty
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on PR #9867:
URL: https://github.com/apache/ozone/pull/9867#issuecomment-4016707862
Thanks for the reviews @adoroszlai @ivandika3 @ChenSammi @rakeshadr.
Merging this change.
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal merged PR #9867:
URL: https://github.com/apache/ozone/pull/9867
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2888172410
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ required string partName = 1;
+ required uint32 partNumber = 2;
+ repeated KeyLocationList keyLocationList = 6;
+ required uint64 dataSize = 7;
+ required uint64 modificationTime = 8;
+ required uint64 objectID = 9;
+ required uint64 updateID = 10;
+ repeated hadoop.hdds.KeyValue metadata = 11;
Review Comment:
Added explicit eTag field
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2888421996
##########
hadoop-ozone/interface-storage/src/main/proto/OmStorageProtocol.proto:
##########
@@ -30,6 +30,7 @@ option java_generate_equals_and_hash = true;
package hadoop.ozone;
import "hdds.proto";
+import "OmClientProtocol.proto";
Review Comment:
Yes, here is the gist I kind of figured out https://github.com/apache/ozone/pull/9867#discussion_r2888386433
Thanks for the context. Are you okay with keeping multipartinfo in OmClient 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: 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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "rakeshadr (via GitHub)" <gi...@apache.org>.
rakeshadr commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2899003539
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
+ optional uint32 partNumber = 2;
+ optional string eTag = 3;
+ optional KeyLocationList keyLocationList = 4;
+ optional uint64 dataSize = 5;
+ optional uint64 modificationTime = 6;
+ optional uint64 objectID = 7;
+ optional uint64 updateID = 8;
+ optional FileEncryptionInfoProto fileEncryptionInfo = 9;
+ optional FileChecksumProto fileChecksum = 10;
Review Comment:
ok, adding owner to the MultipartInfo (parent table) is fine with me. Please make sure that only need to store the identity of the user who called CreateMultipartUpload API and don't overwrite with individual part uploads.
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on PR #9867:
URL: https://github.com/apache/ozone/pull/9867#issuecomment-4009887246
@ChenSammi @ivandika3 @errose28 could you take a look at this 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
Re: [PR] HDDS-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2894601489
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
Review Comment:
Shall we mark the partKeyInfoList deprecated?
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2895669746
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
+ optional uint32 partNumber = 2;
+ optional string eTag = 3;
+ optional KeyLocationList keyLocationList = 4;
+ optional uint64 dataSize = 5;
+ optional uint64 modificationTime = 6;
+ optional uint64 objectID = 7;
+ optional uint64 updateID = 8;
+ optional FileEncryptionInfoProto fileEncryptionInfo = 9;
+ optional FileChecksumProto fileChecksum = 10;
Review Comment:
I think per part owner metadata is not required.
This was mostly incidental metadata as enforcement of ACL did not rely on per-part owner. Also I think checks on native authorizer is done on bucket level for listing operations. But maybe key write does open key ACL check for that write.
I think owner metadata and acls can be pulled up to the MultipartInfo message as that is the equivalent of a key. Let me do that.
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2888410183
##########
hadoop-ozone/interface-storage/src/main/proto/OmStorageProtocol.proto:
##########
@@ -30,6 +30,7 @@ option java_generate_equals_and_hash = true;
package hadoop.ozone;
import "hdds.proto";
+import "OmClientProtocol.proto";
Review Comment:
I don't think we can import `OmClientProtocol` as is [due to](https://github.com/apache/ozone/actions/runs/22707097294/job/65836276038?pr=9867#step:13:3616):
```
OmStorageProtocol.proto:49:25: "hadoop.ozone.OzoneAclInfo.type" is already defined in file "OmClientProtocol.proto"
```
Project for separation of storage and client protocol was started few years ago, but not completed. We need to copy any message definition being used (and improve them in the process if possible).
Since we are adding new type for OM DB, ideally it should go in storage definition. However, this may turn out to be a big change, and I don't mean to delay the MPU improvement for this.
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2887835006
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
Review Comment:
If this only affects OM-side storage, it should be added in `interface-storage` instead of `interface-client`.
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ required string partName = 1;
+ required uint32 partNumber = 2;
+ repeated KeyLocationList keyLocationList = 6;
+ required uint64 dataSize = 7;
+ required uint64 modificationTime = 8;
+ required uint64 objectID = 9;
+ required uint64 updateID = 10;
+ repeated hadoop.hdds.KeyValue metadata = 11;
Review Comment:
See https://github.com/apache/ozone/pull/9793#pullrequestreview-3868200967
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ required string partName = 1;
+ required uint32 partNumber = 2;
+ repeated KeyLocationList keyLocationList = 6;
+ required uint64 dataSize = 7;
+ required uint64 modificationTime = 8;
+ required uint64 objectID = 9;
+ required uint64 updateID = 10;
Review Comment:
Shouldn't we add these as `optional`?
https://protobuf.dev/programming-guides/style/#required
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2895218993
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
Review Comment:
@rakeshadr please see
https://protobuf.dev/programming-guides/proto2/#required-deprecated
https://protobuf.dev/programming-guides/style/#required
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2888438726
##########
hadoop-ozone/interface-storage/src/main/proto/OmStorageProtocol.proto:
##########
@@ -30,6 +30,7 @@ option java_generate_equals_and_hash = true;
package hadoop.ozone;
import "hdds.proto";
+import "OmClientProtocol.proto";
Review Comment:
No problems. Thanks for the insights.
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2888171421
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
Review Comment:
Refactored this in the latest commit. Thanks for pointing this out
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2887919499
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ required string partName = 1;
+ required uint32 partNumber = 2;
+ repeated KeyLocationList keyLocationList = 6;
+ required uint64 dataSize = 7;
+ required uint64 modificationTime = 8;
+ required uint64 objectID = 9;
+ required uint64 updateID = 10;
Review Comment:
We can declare objectID, updateID as optional.
In the existing code flow these are not being consumed as such but they are always populated when creating OmMultipartPartInfo from OmKeyInfo.
I think keeping them as required is better because:
- prevents silent default 0 values if omitted,
- keeps the version semantics
- and also it stays in line with how currently these fields are being updated.
@adoroszlai if you think it might be better to keep these as optional as we do not explicitly consume this in complete/abort I am okay with doing that as well.
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on PR #9867:
URL: https://github.com/apache/ozone/pull/9867#issuecomment-4006842286
@ivandika3 @errose28 could you take another look at the current design doc?
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "rakeshadr (via GitHub)" <gi...@apache.org>.
rakeshadr commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2895188935
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
Review Comment:
Instead of keeping all optional, can you identify required fields and use required?
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "rakeshadr (via GitHub)" <gi...@apache.org>.
rakeshadr commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2895531942
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
+ optional uint32 partNumber = 2;
+ optional string eTag = 3;
+ optional KeyLocationList keyLocationList = 4;
+ optional uint64 dataSize = 5;
+ optional uint64 modificationTime = 6;
+ optional uint64 objectID = 7;
+ optional uint64 updateID = 8;
+ optional FileEncryptionInfoProto fileEncryptionInfo = 9;
+ optional FileChecksumProto fileChecksum = 10;
Review Comment:
Can you check whether "optional string ownerName" is required at part level. For example, any auditing purpose. Earlier, this was stored indirectly through KeyInfo.
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2895669746
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
+ optional uint32 partNumber = 2;
+ optional string eTag = 3;
+ optional KeyLocationList keyLocationList = 4;
+ optional uint64 dataSize = 5;
+ optional uint64 modificationTime = 6;
+ optional uint64 objectID = 7;
+ optional uint64 updateID = 8;
+ optional FileEncryptionInfoProto fileEncryptionInfo = 9;
+ optional FileChecksumProto fileChecksum = 10;
Review Comment:
I think per part owner metadata is not required.
This was mostly incidental metadata enforcement of ACL did not rely on per-part owner. Also I think checks on native authorizer is done on bucket level for listing operations. But maybe key write does open key ACL check for that write.
I think owner metadata and acls can be pulled up to the MultipartInfo message. Let me do that
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2899210423
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
+ optional uint32 partNumber = 2;
+ optional string eTag = 3;
+ optional KeyLocationList keyLocationList = 4;
+ optional uint64 dataSize = 5;
+ optional uint64 modificationTime = 6;
+ optional uint64 objectID = 7;
+ optional uint64 updateID = 8;
+ optional FileEncryptionInfoProto fileEncryptionInfo = 9;
+ optional FileChecksumProto fileChecksum = 10;
Review Comment:
Sure Rakesh, I'll handle that at the request implementation
--
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-14659. Add new proto message for lightweight MultipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2895669746
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
+ optional string partName = 1;
+ optional uint32 partNumber = 2;
+ optional string eTag = 3;
+ optional KeyLocationList keyLocationList = 4;
+ optional uint64 dataSize = 5;
+ optional uint64 modificationTime = 6;
+ optional uint64 objectID = 7;
+ optional uint64 updateID = 8;
+ optional FileEncryptionInfoProto fileEncryptionInfo = 9;
+ optional FileChecksumProto fileChecksum = 10;
Review Comment:
I think per part owner metadata is not required.
This was mostly incidental metadata enforcement of ACL did not rely on per-part owner. Also I think checks on native authorizer is done on bucket level.
I think owner metadata and acls can be pulled up to the MultipartInfo message. Let me do that
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "devabhishekpal (via GitHub)" <gi...@apache.org>.
devabhishekpal commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2888386433
##########
hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto:
##########
@@ -1726,6 +1730,19 @@ message PartKeyInfo {
required KeyInfo partKeyInfo = 3;
}
+message MultipartPartInfo {
Review Comment:
Hmm, moving this to storage protos seems to touch many files.
`OzoneAclInfo` in the `OmStorageProtocol` is the copy of the same message in `OmClientProtocol`
So currently build failures are caused by this duplicate conflict.
We can remove the OzoneAclInfo from storage protos and inherit from the client protos - this would need changes in the imports for classes which refer to the OzoneAclInfo from storage protos.
Alternatively we need to copy over `KeyLocationList`, `FileEncryptionInfoProto` and `FileChecksumProto` but that seems like duplicate changes.
--
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-14659. Add new proto message for lightweight MutlipartPartInfo [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #9867:
URL: https://github.com/apache/ozone/pull/9867#discussion_r2888433786
##########
hadoop-ozone/interface-storage/src/main/proto/OmStorageProtocol.proto:
##########
@@ -30,6 +30,7 @@ option java_generate_equals_and_hash = true;
package hadoop.ozone;
import "hdds.proto";
+import "OmClientProtocol.proto";
Review Comment:
> Are you okay with keeping multipartinfo in OmClient then?
Yes...
> We can remove the OzoneAclInfo from storage protos
No, that would defeat the purpose of `interface-storage`.
--
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