You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/10/25 00:23:05 UTC
[GitHub] [ozone] kuenishi opened a new pull request #2763: HDDS-5893. Potential data loss on multipart upload commit
kuenishi opened a new pull request #2763:
URL: https://github.com/apache/ozone/pull/2763
## What changes were proposed in this pull request?
This patch prevents moving live OmKeyInfo to delete table, which seems a bug.
Bug description follows: In case mutlipart upload completion with unused parts, and the key is first created (no previous version), the committed live version of OmKeyInfo is added to deletion table. The key will be live until the deletion service collects the blocks, but after blocks deleted, the key metadata will be visible from user API while the blocks are already deleted.
## What is the link to the Apache JIRA
HDDS-5893
## How was this patch tested?
Unit tests will follow
--
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] kuenishi commented on a change in pull request #2763: HDDS-5893. Potential data loss on multipart upload commit
Posted by GitBox <gi...@apache.org>.
kuenishi commented on a change in pull request #2763:
URL: https://github.com/apache/ozone/pull/2763#discussion_r735323777
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCompleteResponseWithFSO.java
##########
@@ -186,17 +235,14 @@ public void testAddDBToBatchWithParts() throws Exception {
Assert.assertNull(omMetadataManager.getOpenKeyTable(getBucketLayout())
.get(dbMultipartOpenKey));
- // As 1 unused parts exists, so 1 unused entry should be there in delete
- // table.
- deleteEntryCount++;
- Assert.assertEquals(deleteEntryCount, omMetadataManager.countRowsInTable(
- omMetadataManager.getDeletedTable()));
Review comment:
Weird thing here is even before this patch, this assertion contradicts with the above comment, which states `deleteEntryCount` is 2 here.
--
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] kuenishi commented on pull request #2763: HDDS-5893. Potential data loss on multipart upload commit
Posted by GitBox <gi...@apache.org>.
kuenishi commented on pull request #2763:
URL: https://github.com/apache/ozone/pull/2763#issuecomment-952506497
Thanks for the review, too.
--
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] kuenishi commented on a change in pull request #2763: HDDS-5893. Potential data loss on multipart upload commit
Posted by GitBox <gi...@apache.org>.
kuenishi commented on a change in pull request #2763:
URL: https://github.com/apache/ozone/pull/2763#discussion_r735324546
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCompleteResponseWithFSO.java
##########
@@ -133,10 +133,57 @@ public void testAddDBToBatchWithParts() throws Exception {
TestOMRequestUtils.addVolumeAndBucketToDB(volumeName, bucketName,
omMetadataManager);
createParentPath(volumeName, bucketName);
+ runAddDBToBatchWithParts(volumeName, bucketName, keyName, 0);
- String multipartUploadID = UUID.randomUUID().toString();
+ // As 1 unused parts exists, so 1 unused entry should be there in delete
+ // table.
+ Assert.assertEquals(2, omMetadataManager.countRowsInTable(
+ omMetadataManager.getDeletedTable()));
Review comment:
See my comment below in line 193 about contradiction between the comment and the assertion.
--
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] bharatviswa504 merged pull request #2763: HDDS-5893. Potential data loss on multipart upload commit
Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #2763:
URL: https://github.com/apache/ozone/pull/2763
--
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] kuenishi commented on pull request #2763: HDDS-5893. Potential data loss on multipart upload commit
Posted by GitBox <gi...@apache.org>.
kuenishi commented on pull request #2763:
URL: https://github.com/apache/ozone/pull/2763#issuecomment-950636507
@bharatviswa504 Thank you for taking review. I added a unit test to reproduce the scenario you mentioned, by copying existing unit test. Robot testing would also be nice, but that'd be too heavy for quick fix.
--
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] bharatviswa504 commented on a change in pull request #2763: HDDS-5893. Potential data loss on multipart upload commit
Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #2763:
URL: https://github.com/apache/ozone/pull/2763#discussion_r736870681
##########
File path: hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/response/s3/multipart/TestS3MultipartUploadCompleteResponseWithFSO.java
##########
@@ -225,7 +272,7 @@ private OmKeyInfo commitS3MultipartUpload(String volumeName,
omMetadataManager.getStore().commitBatchOperation(batchOperation);
// As 1 parts are created, so 1 entry should be there in delete table.
- Assert.assertEquals(1, omMetadataManager.countRowsInTable(
+ Assert.assertEquals(deleteEntryCount, omMetadataManager.countRowsInTable(
Review comment:
Looks like in test what part we trying to override, same is in the multipart key info.
Not related, just reading code observed it to understand why 2 entries are there in delete table.
--
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