You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "ivandika3 (via GitHub)" <gi...@apache.org> on 2024/04/06 14:50:02 UTC
[PR] HDDS-10653. Support custom metadata for MPU key [ozone]
ivandika3 opened a new pull request, #6489:
URL: https://github.com/apache/ozone/pull/6489
## What changes were proposed in this pull request?
[HDDS-6440](https://issues.apache.org/jira/browse/HDDS-6440) added support for custom metadata for normal object creation.
We should also support custom metadata creation for multipart upload key.
The metadata should be set during the multipart upload initiation request, which will be committed during the multipart upload complete request.
See: https://docs.aws.amazon.com/cli/latest/reference/s3api/create-multipart-upload.html
Should be tested using unit tests and acceptance tests.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10653
## How was this patch tested?
Unit, integration, and acceptance tests.
Clean CI: https://github.com/ivandika3/ozone/actions/runs/8579748398 (Before the TestReconTasks revert)
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on PR #6489:
URL: https://github.com/apache/ozone/pull/6489#issuecomment-2051545530
@ivandika3 thanks for the changes. LGTM at first glance, going to continue the review a little bit later
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #6489:
URL: https://github.com/apache/ozone/pull/6489#discussion_r1563631722
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java:
##########
@@ -42,6 +42,7 @@
import java.util.Map;
import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
+import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.anyLong;
Review Comment:
nit: import from `Mockito`.
```suggestion
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.anyLong;
import static org.mockito.Mockito.anyMap;
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -3661,12 +3688,26 @@ private void doMultipartUpload(OzoneBucket bucket, String keyName, byte val,
latestVersionLocations.getBlocksLatestVersionOnly()
.forEach(omKeyLocationInfo ->
assertNotEquals(-1, omKeyLocationInfo.getPartNumber()));
+
+ Map<String, String> keyMetadata = omKeyInfo.getMetadata();
+ assertNotNull(keyMetadata.get(ETAG));
+ if (customMetadata != null) {
+ for (Map.Entry<String, String> customEntry : customMetadata.entrySet()) {
+ assertTrue(keyMetadata.containsKey(customEntry.getKey()));
+ assertEquals(customEntry.getValue(), keyMetadata.get(customEntry.getKey()));
+ }
Review Comment:
Use `assertThat` instead of `assertTrue` for better message in case of failure. It also provides assertions for maps.
```suggestion
assertThat(keyMetadata).containsAllEntriesOf(customMetadata);
```
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #6489:
URL: https://github.com/apache/ozone/pull/6489#issuecomment-2041111088
@kerneltime @DaveTeng0 @SaketaChalamchala Could you help take a look when you have time?
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6489:
URL: https://github.com/apache/ozone/pull/6489#discussion_r1563853150
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -3661,12 +3688,26 @@ private void doMultipartUpload(OzoneBucket bucket, String keyName, byte val,
latestVersionLocations.getBlocksLatestVersionOnly()
.forEach(omKeyLocationInfo ->
assertNotEquals(-1, omKeyLocationInfo.getPartNumber()));
+
+ Map<String, String> keyMetadata = omKeyInfo.getMetadata();
+ assertNotNull(keyMetadata.get(ETAG));
+ if (customMetadata != null) {
+ for (Map.Entry<String, String> customEntry : customMetadata.entrySet()) {
+ assertTrue(keyMetadata.containsKey(customEntry.getKey()));
+ assertEquals(customEntry.getValue(), keyMetadata.get(customEntry.getKey()));
+ }
Review Comment:
Thank you for the suggestion. Updated with `assertThat` (also updated for other similar tests).
I'll try to use more `assertThat` APIs in the future.
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6489:
URL: https://github.com/apache/ozone/pull/6489#discussion_r1574018325
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java:
##########
@@ -210,7 +211,7 @@ public OzoneDataStreamOutput createMultipartStreamKey(String key,
int partNumber,
String uploadID)
throws IOException {
- String multipartUploadID = multipartUploadIdMap.get(key);
+ String multipartUploadID = keyToMultipartUpload.get(key).getUploadId();
if (multipartUploadID == null || !multipartUploadID.equals(uploadID)) {
Review Comment:
Thank you for catching it. I have updated it as the same logic as `createMultipartKey`.
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #6489:
URL: https://github.com/apache/ozone/pull/6489#issuecomment-2068510015
Thanks @ivandika3 for the patch, @vtutrinov 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-10653. Support custom metadata for MPU key [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6489:
URL: https://github.com/apache/ozone/pull/6489#discussion_r1563853150
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -3661,12 +3688,26 @@ private void doMultipartUpload(OzoneBucket bucket, String keyName, byte val,
latestVersionLocations.getBlocksLatestVersionOnly()
.forEach(omKeyLocationInfo ->
assertNotEquals(-1, omKeyLocationInfo.getPartNumber()));
+
+ Map<String, String> keyMetadata = omKeyInfo.getMetadata();
+ assertNotNull(keyMetadata.get(ETAG));
+ if (customMetadata != null) {
+ for (Map.Entry<String, String> customEntry : customMetadata.entrySet()) {
+ assertTrue(keyMetadata.containsKey(customEntry.getKey()));
+ assertEquals(customEntry.getValue(), keyMetadata.get(customEntry.getKey()));
+ }
Review Comment:
Thank you for the suggestion. Updated with `assertThat` (also updated for other similar tests).
I'll try to use `assertThat` API in the future.
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6489:
URL: https://github.com/apache/ozone/pull/6489#discussion_r1563899072
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -3661,12 +3688,26 @@ private void doMultipartUpload(OzoneBucket bucket, String keyName, byte val,
latestVersionLocations.getBlocksLatestVersionOnly()
.forEach(omKeyLocationInfo ->
assertNotEquals(-1, omKeyLocationInfo.getPartNumber()));
+
+ Map<String, String> keyMetadata = omKeyInfo.getMetadata();
+ assertNotNull(keyMetadata.get(ETAG));
+ if (customMetadata != null) {
+ for (Map.Entry<String, String> customEntry : customMetadata.entrySet()) {
+ assertTrue(keyMetadata.containsKey(customEntry.getKey()));
+ assertEquals(customEntry.getValue(), keyMetadata.get(customEntry.getKey()));
+ }
Review Comment:
Some test failure due to the last commit, let me fix it first.
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #6489:
URL: https://github.com/apache/ozone/pull/6489
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "vtutrinov (via GitHub)" <gi...@apache.org>.
vtutrinov commented on PR #6489:
URL: https://github.com/apache/ozone/pull/6489#issuecomment-2058601261
@ivandika3 thanks for the patch, LGTM, +1
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on PR #6489:
URL: https://github.com/apache/ozone/pull/6489#issuecomment-2043165902
cc @SaketaChalamchala @tanvipenumudy @vtutrinov can you'll 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-10653. Support custom metadata for MPU key [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #6489:
URL: https://github.com/apache/ozone/pull/6489#discussion_r1573827052
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/client/OzoneBucketStub.java:
##########
@@ -210,7 +211,7 @@ public OzoneDataStreamOutput createMultipartStreamKey(String key,
int partNumber,
String uploadID)
throws IOException {
- String multipartUploadID = multipartUploadIdMap.get(key);
+ String multipartUploadID = keyToMultipartUpload.get(key).getUploadId();
if (multipartUploadID == null || !multipartUploadID.equals(uploadID)) {
Review Comment:
Wouldn't `keyToMultipartUpload.get(key) == null` indicate "no such upload? We'd hit NPE in that case. I guess `MultipartInfoStub#uploadID` would rarely be `null`.
(`createMultipartKey` seems to have the correct logic.)
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #6489:
URL: https://github.com/apache/ozone/pull/6489#issuecomment-2068510464
Thank you @vtutrinov and @adoroszlai for the reviews.
--
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-10653. Support custom metadata for MPU key [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6489:
URL: https://github.com/apache/ozone/pull/6489#discussion_r1563852635
##########
hadoop-ozone/s3gateway/src/test/java/org/apache/hadoop/ozone/s3/endpoint/TestPermissionCheck.java:
##########
@@ -42,6 +42,7 @@
import java.util.Map;
import static java.net.HttpURLConnection.HTTP_FORBIDDEN;
+import static org.mockito.ArgumentMatchers.anyMap;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyBoolean;
import static org.mockito.Mockito.anyLong;
Review Comment:
Thank you for the review. 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-10653. Support custom metadata for MPU key [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on code in PR #6489:
URL: https://github.com/apache/ozone/pull/6489#discussion_r1564036180
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java:
##########
@@ -3661,12 +3688,26 @@ private void doMultipartUpload(OzoneBucket bucket, String keyName, byte val,
latestVersionLocations.getBlocksLatestVersionOnly()
.forEach(omKeyLocationInfo ->
assertNotEquals(-1, omKeyLocationInfo.getPartNumber()));
+
+ Map<String, String> keyMetadata = omKeyInfo.getMetadata();
+ assertNotNull(keyMetadata.get(ETAG));
+ if (customMetadata != null) {
+ for (Map.Entry<String, String> customEntry : customMetadata.entrySet()) {
+ assertTrue(keyMetadata.containsKey(customEntry.getKey()));
+ assertEquals(customEntry.getValue(), keyMetadata.get(customEntry.getKey()));
+ }
Review Comment:
Fixed.
--
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