Skip to content

HDDS-14659. Add new proto message for lightweight MultipartPartInfo#9867

Open
devabhishekpal wants to merge 8 commits intoapache:masterfrom
devabhishekpal:HDDS-14659
Open

HDDS-14659. Add new proto message for lightweight MultipartPartInfo#9867
devabhishekpal wants to merge 8 commits intoapache:masterfrom
devabhishekpal:HDDS-14659

Conversation

@devabhishekpal
Copy link
Contributor

@devabhishekpal devabhishekpal commented Mar 5, 2026

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 absolutely 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.

Reference design doc: #9793

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.

@devabhishekpal devabhishekpal self-assigned this Mar 5, 2026
@devabhishekpal devabhishekpal added the s3 S3 Gateway label Mar 5, 2026
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @devabhishekpal for the patch.

@adoroszlai adoroszlai changed the title HDDS-14659. Add new proto message for lightweight MutlipartPartInfo HDDS-14659. Add new proto message for lightweight MultipartPartInfo Mar 5, 2026
Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @devabhishekpal for updating the patch, LGTM. Let's wait for others to take a look.

@devabhishekpal
Copy link
Contributor Author

@ChenSammi @ivandika3 @errose28 could you take a look at this patch?

@devabhishekpal devabhishekpal requested a review from rakeshadr March 6, 2026 08:30
Copy link
Contributor

@ChenSammi ChenSammi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM +1.

optional uint64 objectID = 7;
optional uint64 updateID = 8;
optional FileEncryptionInfoProto fileEncryptionInfo = 9;
optional FileChecksumProto fileChecksum = 10;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check whether "optional string ownerName" is required at part level. For example, any auditing purpose. Earlier, this was stored indirectly through KeyInfo.

Copy link
Contributor Author

@devabhishekpal devabhishekpal Mar 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

s3 S3 Gateway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants