You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sumitagrawl (via GitHub)" <gi...@apache.org> on 2023/05/05 06:32:38 UTC

[GitHub] [ozone] sumitagrawl opened a new pull request, #4660: HDDS-8463. S3 key uniqueness in deletedTable

sumitagrawl opened a new pull request, #4660:
URL: https://github.com/apache/ozone/pull/4660

   ## What changes were proposed in this pull request?
   
   for multipart, delete key is multi-part + objectId, where multi-part format is /volumeName/bucketName/keyname/uploadId
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-8463
   
   ## How was this patch tested?
   
   Tested with UT and integration cased present
   


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


[GitHub] [ozone] sumitagrawl commented on a diff in pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4660:
URL: https://github.com/apache/ozone/pull/4660#discussion_r1189843446


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);
+      // multi-part key format is volumeName/bucketName/keyName/uploadId
+      String deleteKey = omMetadataManager.getOzoneDeletePathKey(
+          currentKeyPartInfo.getObjectID(), multipartKey);

Review Comment:
   @szetszwo Each part is created with new object ID as independent upload, and combined to multipart table in repeatedomkeyinfo. ObjectId is not replaced in keyInfo for the part and remains unique.
   
   but part number / name is not unique as user can do re-upload with same part number. If same part number, it will overwrite. And this can cause duplicate entry in deletedTable. So this is not unique.



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


[GitHub] [ozone] szetszwo commented on pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #4660:
URL: https://github.com/apache/ozone/pull/4660#issuecomment-1537197220

   @sumitagrawl , we should fix HDDS-8550 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


[GitHub] [ozone] szetszwo commented on a diff in pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4660:
URL: https://github.com/apache/ozone/pull/4660#discussion_r1188875233


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);

Review Comment:
   After this, the `repeatedOmKeyInfo` parameter of `prepareKeyForDelete(..)` is always null.  Let's remove it.
   ```java
   +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java
   @@ -446,8 +446,8 @@ public static File createOMDir(String dirPath) {
       * create a new instance to include this key, else we update the existing
       * repeatedOmKeyInfo instance.
       * 3. Set the updateID to the transactionLogIndex.
       * @param keyInfo args supplied by client
   -   * @param repeatedOmKeyInfo key details from deletedTable
       * @param trxnLogIndex For Multipart keys, this is the transactionLogIndex
       *                     of the MultipartUploadAbort request which needs to
       *                     be set as the updateID of the partKeyInfos.
   @@ -456,8 +456,7 @@ public static File createOMDir(String dirPath) {
       * @return {@link RepeatedOmKeyInfo}
       */
      public static RepeatedOmKeyInfo prepareKeyForDelete(OmKeyInfo keyInfo,
   -      RepeatedOmKeyInfo repeatedOmKeyInfo, long trxnLogIndex,
   -      boolean isRatisEnabled) {
   +      long trxnLogIndex, boolean isRatisEnabled) {
        // If this key is in a GDPR enforced bucket, then before moving
        // KeyInfo to deletedTable, remove the GDPR related metadata and
        // FileEncryptionInfo from KeyInfo.
   @@ -473,15 +472,7 @@ public static RepeatedOmKeyInfo prepareKeyForDelete(OmKeyInfo keyInfo,
        // Set the updateID
        keyInfo.setUpdateID(trxnLogIndex, isRatisEnabled);
    
   -    if (repeatedOmKeyInfo == null) {
   -      //The key doesn't exist in deletedTable, so create a new instance.
   -      repeatedOmKeyInfo = new RepeatedOmKeyInfo(keyInfo);
   -    } else {
   -      //The key exists in deletedTable, so update existing instance.
   -      repeatedOmKeyInfo.addOmKeyInfo(keyInfo);
   -    }
   -
   -    return repeatedOmKeyInfo;
   +    return new RepeatedOmKeyInfo(keyInfo);
      }
   ```



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);
+      // multi-part key format is volumeName/bucketName/keyName/uploadId
+      String deleteKey = omMetadataManager.getOzoneDeletePathKey(
+          currentKeyPartInfo.getObjectID(), multipartKey);

Review Comment:
   It should have `partNumber`/`partName`.  Object id is the same for all parts.
   
   BTW, let's add a utility method to create multipart delete key.



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


[GitHub] [ozone] sumitagrawl commented on pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #4660:
URL: https://github.com/apache/ozone/pull/4660#issuecomment-1536535652

   @szetszwo plz 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


[GitHub] [ozone] szetszwo commented on a diff in pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4660:
URL: https://github.com/apache/ozone/pull/4660#discussion_r1192423188


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);
+      // multi-part key format is volumeName/bucketName/keyName/uploadId
+      String deleteKey = omMetadataManager.getOzoneDeletePathKey(
+          currentKeyPartInfo.getObjectID(), multipartKey);

Review Comment:
   > ... in createPartKeyInfo, it creates parts random an object id but not the id generated by OM.
   
   Indeed, the random ids could be the same since randomness does not prevent collision.



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


[GitHub] [ozone] szetszwo commented on a diff in pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4660:
URL: https://github.com/apache/ozone/pull/4660#discussion_r1192417254


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);
+      // multi-part key format is volumeName/bucketName/keyName/uploadId
+      String deleteKey = omMetadataManager.getOzoneDeletePathKey(
+          currentKeyPartInfo.getObjectID(), multipartKey);

Review Comment:
   @sumitagrawl , tried the `testAddDBToBatchWithParts` but it is not a real test -- in `createPartKeyInfo`, it creates parts random an object id but not the id generated by OM.
   
   We should have a test that a real client uploads multipart to OM and then the parts get deleted.



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


[GitHub] [ozone] szetszwo commented on a diff in pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4660:
URL: https://github.com/apache/ozone/pull/4660#discussion_r1191741917


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);
+      // multi-part key format is volumeName/bucketName/keyName/uploadId
+      String deleteKey = omMetadataManager.getOzoneDeletePathKey(
+          currentKeyPartInfo.getObjectID(), multipartKey);

Review Comment:
   @sumitagrawl , I am asking a unit test.  If there is one, which one?  I want to run it locally.  Thanks.



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


[GitHub] [ozone] sumitagrawl commented on a diff in pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4660:
URL: https://github.com/apache/ozone/pull/4660#discussion_r1191440176


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);
+      // multi-part key format is volumeName/bucketName/keyName/uploadId
+      String deleteKey = omMetadataManager.getOzoneDeletePathKey(
+          currentKeyPartInfo.getObjectID(), multipartKey);

Review Comment:
   @szetszwo This is already covered by existing testcase and same is updated with objectId as correction as part of this PR,
   https://github.com/apache/ozone/pull/4660/files#diff-c1e307469ed5f710651ae2ec86baed4d9dce85d742c1c282cb1e7d28a776b503
   
   As part of this, 2 parts are deleted with unique key: part1DeletedKeyName and part2DeletedKeyName



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


[GitHub] [ozone] szetszwo merged pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo merged PR #4660:
URL: https://github.com/apache/ozone/pull/4660


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


[GitHub] [ozone] szetszwo commented on a diff in pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #4660:
URL: https://github.com/apache/ozone/pull/4660#discussion_r1190658095


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);
+      // multi-part key format is volumeName/bucketName/keyName/uploadId
+      String deleteKey = omMetadataManager.getOzoneDeletePathKey(
+          currentKeyPartInfo.getObjectID(), multipartKey);

Review Comment:
   @sumitagrawl , thanks for the info.  Do we have a unit test showing it?  If not, could you add one?



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


[GitHub] [ozone] sumitagrawl commented on a diff in pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4660:
URL: https://github.com/apache/ozone/pull/4660#discussion_r1191440176


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);
+      // multi-part key format is volumeName/bucketName/keyName/uploadId
+      String deleteKey = omMetadataManager.getOzoneDeletePathKey(
+          currentKeyPartInfo.getObjectID(), multipartKey);

Review Comment:
   @szetszwo This is already covered by existing testcase and same is updated with objectId as correction as part of this PR,
   https://github.com/apache/ozone/pull/4660/files#diff-c1e307469ed5f710651ae2ec86baed4d9dce85d742c1c282cb1e7d28a776b503



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


[GitHub] [ozone] sumitagrawl commented on pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on PR #4660:
URL: https://github.com/apache/ozone/pull/4660#issuecomment-1540184830

   > @sumitagrawl , we should fix [HDDS-8550](https://issues.apache.org/jira/browse/HDDS-8550) first.
   
   It’s fixed and PR raised 


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


[GitHub] [ozone] sumitagrawl commented on a diff in pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4660:
URL: https://github.com/apache/ozone/pull/4660#discussion_r1191926617


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/s3/multipart/S3MultipartUploadAbortResponse.java:
##########
@@ -97,19 +97,20 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       OmKeyInfo currentKeyPartInfo =
           OmKeyInfo.getFromProtobuf(partKeyInfo.getPartKeyInfo());
 
-      RepeatedOmKeyInfo repeatedOmKeyInfo =
-          omMetadataManager.getDeletedTable().get(partKeyInfo.getPartName());
-
-      repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(currentKeyPartInfo,
-          repeatedOmKeyInfo, omMultipartKeyInfo.getUpdateID(), isRatisEnabled);
+      RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
+          currentKeyPartInfo, null, omMultipartKeyInfo.getUpdateID(),
+          isRatisEnabled);
+      // multi-part key format is volumeName/bucketName/keyName/uploadId
+      String deleteKey = omMetadataManager.getOzoneDeletePathKey(
+          currentKeyPartInfo.getObjectID(), multipartKey);

Review Comment:
   @szetszwo TestS3MultipartUploadAbortResponse:testAddDBToBatchWithParts cover the scenario as discussed.



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


[GitHub] [ozone] szetszwo commented on pull request #4660: HDDS-8463. S3 key uniqueness in deletedTable

Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on PR #4660:
URL: https://github.com/apache/ozone/pull/4660#issuecomment-1546814692

   I actually have a Multipart Upload test and I am able to verify that the object ids for each part are different. 
   {code}
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=1, batchSize=640 B, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854771968
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=2, batchSize=1.25 KB, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854771712
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=3, batchSize=1.88 KB, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854770688
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=4, batchSize=2.50 KB, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854770432
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=5, batchSize=3.13 KB, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854771456
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=6, batchSize=3.75 KB, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854771200
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=7, batchSize=4.38 KB, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854772992
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=8, batchSize=5 KB, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854772224
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=9, batchSize=5.63 KB, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854770176
   Batch-61: deletedTable put(key: 98 B, value: 542 B), #put=10, batchSize=6.25 KB, discarded: 0 (0 B); key=/s3v/s3bucket/testKey/76d7cb94-3369-470c-a83f-28448135f131-110365366127820801/-9223372036854772736
     Batch-61: deletedTable batchSize=6.25 KB, discarded: 0 (0 B), #put=10, #del=0
   {code}
   


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