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 2022/12/15 15:45:03 UTC
[GitHub] [ozone] sumitagrawl opened a new pull request, #4095: HDDS-7649. S3 multipart upload EC release space quota wrong for old version
sumitagrawl opened a new pull request, #4095:
URL: https://github.com/apache/ozone/pull/4095
## What changes were proposed in this pull request?
Fix for size calculation for EC case in complete flow when old version file is for removal
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7649
## How was this patch tested?
Tested using UT cases and added for EC
--
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 #4095: HDDS-7649. S3 multipart upload EC release space quota wrong for old version
Posted by GitBox <gi...@apache.org>.
sumitagrawl commented on code in PR #4095:
URL: https://github.com/apache/ozone/pull/4095#discussion_r1050358200
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientMultipartUploadWithFSO.java:
##########
@@ -311,6 +315,118 @@ public void testUploadPartOverrideWithRatis() throws IOException {
commitUploadPartInfo.getPartName());
}
+ @Test
+ public void testUploadTwiceWithEC() throws IOException {
+
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName = UUID.randomUUID().toString();
+ StringBuilder sb = new StringBuilder("sample Value");
+ for (int i = 0; i < 4096; ++i) {
+ sb.append("01234567890123456789");
+ }
+ String sampleData = sb.toString();
+
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getOzoneECBucket(volumeName, bucketName);
+
+ // perform upload and complete
+ OmMultipartInfo multipartInfo = bucket.initiateMultipartUpload(keyName,
+ bucket.getReplicationConfig());
+
+ String uploadID = multipartInfo.getUploadID();
+ int partNumber = 1;
+
+ OzoneOutputStream ozoneOutputStream = bucket.createMultipartKey(keyName,
+ sampleData.length(), partNumber, uploadID);
+ ozoneOutputStream.write(string2Bytes(sampleData), 0, sampleData.length());
+ ozoneOutputStream.close();
+
+ OmMultipartCommitUploadPartInfo commitUploadPartInfo = ozoneOutputStream
+ .getCommitUploadPartInfo();
+
+ String partName = commitUploadPartInfo.getPartName();
+ Map<Integer, String> partsMap = new HashMap<>();
+ partsMap.put(partNumber, partName);
+ bucket.completeMultipartUpload(keyName, uploadID, partsMap);
+
+ Assert.assertEquals(volume.getBucket(bucketName).getUsedBytes(), 137228);
Review Comment:
@sodonnel 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
[GitHub] [ozone] sodonnel commented on a diff in pull request #4095: HDDS-7649. S3 multipart upload EC release space quota wrong for old version
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #4095:
URL: https://github.com/apache/ozone/pull/4095#discussion_r1050167672
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientMultipartUploadWithFSO.java:
##########
@@ -311,6 +315,118 @@ public void testUploadPartOverrideWithRatis() throws IOException {
commitUploadPartInfo.getPartName());
}
+ @Test
+ public void testUploadTwiceWithEC() throws IOException {
+
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName = UUID.randomUUID().toString();
+ StringBuilder sb = new StringBuilder("sample Value");
+ for (int i = 0; i < 4096; ++i) {
+ sb.append("01234567890123456789");
+ }
+ String sampleData = sb.toString();
+
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getOzoneECBucket(volumeName, bucketName);
+
+ // perform upload and complete
+ OmMultipartInfo multipartInfo = bucket.initiateMultipartUpload(keyName,
+ bucket.getReplicationConfig());
+
+ String uploadID = multipartInfo.getUploadID();
+ int partNumber = 1;
+
+ OzoneOutputStream ozoneOutputStream = bucket.createMultipartKey(keyName,
+ sampleData.length(), partNumber, uploadID);
+ ozoneOutputStream.write(string2Bytes(sampleData), 0, sampleData.length());
+ ozoneOutputStream.close();
+
+ OmMultipartCommitUploadPartInfo commitUploadPartInfo = ozoneOutputStream
+ .getCommitUploadPartInfo();
+
+ String partName = commitUploadPartInfo.getPartName();
+ Map<Integer, String> partsMap = new HashMap<>();
+ partsMap.put(partNumber, partName);
+ bucket.completeMultipartUpload(keyName, uploadID, partsMap);
+
+ Assert.assertEquals(volume.getBucket(bucketName).getUsedBytes(), 137228);
Review Comment:
Where does "137228" come from? How it is derived?
--
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] sodonnel merged pull request #4095: HDDS-7649. S3 multipart upload EC release space quota wrong for old version
Posted by GitBox <gi...@apache.org>.
sodonnel merged PR #4095:
URL: https://github.com/apache/ozone/pull/4095
--
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 #4095: HDDS-7649. S3 multipart upload EC release space quota wrong for old version
Posted by GitBox <gi...@apache.org>.
sumitagrawl commented on code in PR #4095:
URL: https://github.com/apache/ozone/pull/4095#discussion_r1051776885
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientMultipartUploadWithFSO.java:
##########
@@ -311,6 +316,124 @@ public void testUploadPartOverrideWithRatis() throws IOException {
commitUploadPartInfo.getPartName());
}
+ @Test
+ public void testUploadTwiceWithEC() throws IOException {
+
+ String volumeName = UUID.randomUUID().toString();
Review Comment:
Updated...
@sodonnel
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientMultipartUploadWithFSO.java:
##########
@@ -311,6 +316,124 @@ public void testUploadPartOverrideWithRatis() throws IOException {
commitUploadPartInfo.getPartName());
}
+ @Test
+ public void testUploadTwiceWithEC() throws IOException {
+
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName = UUID.randomUUID().toString();
+ StringBuilder sb = new StringBuilder("sample Value");
Review Comment:
updated @sodonnel
--
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] sodonnel commented on a diff in pull request #4095: HDDS-7649. S3 multipart upload EC release space quota wrong for old version
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #4095:
URL: https://github.com/apache/ozone/pull/4095#discussion_r1050568359
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientMultipartUploadWithFSO.java:
##########
@@ -311,6 +316,124 @@ public void testUploadPartOverrideWithRatis() throws IOException {
commitUploadPartInfo.getPartName());
}
+ @Test
+ public void testUploadTwiceWithEC() throws IOException {
+
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName = UUID.randomUUID().toString();
+ StringBuilder sb = new StringBuilder("sample Value");
Review Comment:
Could you create a small private method to create the same data, rather than duplicating the logic twice in this test and the other now 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] sodonnel commented on a diff in pull request #4095: HDDS-7649. S3 multipart upload EC release space quota wrong for old version
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #4095:
URL: https://github.com/apache/ozone/pull/4095#discussion_r1050567412
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientMultipartUploadWithFSO.java:
##########
@@ -311,6 +316,124 @@ public void testUploadPartOverrideWithRatis() throws IOException {
commitUploadPartInfo.getPartName());
}
+ @Test
+ public void testUploadTwiceWithEC() throws IOException {
+
+ String volumeName = UUID.randomUUID().toString();
Review Comment:
Nearly every test in that class starts with the same repeated code to create a volName, create the volume, create the bucketname, bucket etc. Could you refactor it into the setup method for all tests to reduce the repetition?
--
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] sodonnel commented on a diff in pull request #4095: HDDS-7649. S3 multipart upload EC release space quota wrong for old version
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #4095:
URL: https://github.com/apache/ozone/pull/4095#discussion_r1050172749
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientMultipartUploadWithFSO.java:
##########
@@ -311,6 +315,118 @@ public void testUploadPartOverrideWithRatis() throws IOException {
commitUploadPartInfo.getPartName());
}
+ @Test
+ public void testUploadTwiceWithEC() throws IOException {
+
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName = UUID.randomUUID().toString();
+ StringBuilder sb = new StringBuilder("sample Value");
+ for (int i = 0; i < 4096; ++i) {
+ sb.append("01234567890123456789");
+ }
+ String sampleData = sb.toString();
+
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getOzoneECBucket(volumeName, bucketName);
+
+ // perform upload and complete
+ OmMultipartInfo multipartInfo = bucket.initiateMultipartUpload(keyName,
+ bucket.getReplicationConfig());
+
+ String uploadID = multipartInfo.getUploadID();
+ int partNumber = 1;
+
+ OzoneOutputStream ozoneOutputStream = bucket.createMultipartKey(keyName,
+ sampleData.length(), partNumber, uploadID);
+ ozoneOutputStream.write(string2Bytes(sampleData), 0, sampleData.length());
+ ozoneOutputStream.close();
+
+ OmMultipartCommitUploadPartInfo commitUploadPartInfo = ozoneOutputStream
+ .getCommitUploadPartInfo();
+
+ String partName = commitUploadPartInfo.getPartName();
+ Map<Integer, String> partsMap = new HashMap<>();
+ partsMap.put(partNumber, partName);
+ bucket.completeMultipartUpload(keyName, uploadID, partsMap);
+
+ Assert.assertEquals(volume.getBucket(bucketName).getUsedBytes(), 137228);
Review Comment:
I guess its the EC data at 1024 chunk size stripped across the 3 nodes and 2 parity. But I think we need a comment or a calculation here to show were it comes from. Otherwise its just a magic number that doesn't make sense to anyone 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
[GitHub] [ozone] sodonnel commented on a diff in pull request #4095: HDDS-7649. S3 multipart upload EC release space quota wrong for old version
Posted by GitBox <gi...@apache.org>.
sodonnel commented on code in PR #4095:
URL: https://github.com/apache/ozone/pull/4095#discussion_r1050172749
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneClientMultipartUploadWithFSO.java:
##########
@@ -311,6 +315,118 @@ public void testUploadPartOverrideWithRatis() throws IOException {
commitUploadPartInfo.getPartName());
}
+ @Test
+ public void testUploadTwiceWithEC() throws IOException {
+
+ String volumeName = UUID.randomUUID().toString();
+ String bucketName = UUID.randomUUID().toString();
+ String keyName = UUID.randomUUID().toString();
+ StringBuilder sb = new StringBuilder("sample Value");
+ for (int i = 0; i < 4096; ++i) {
+ sb.append("01234567890123456789");
+ }
+ String sampleData = sb.toString();
+
+ store.createVolume(volumeName);
+ OzoneVolume volume = store.getVolume(volumeName);
+ OzoneBucket bucket = getOzoneECBucket(volumeName, bucketName);
+
+ // perform upload and complete
+ OmMultipartInfo multipartInfo = bucket.initiateMultipartUpload(keyName,
+ bucket.getReplicationConfig());
+
+ String uploadID = multipartInfo.getUploadID();
+ int partNumber = 1;
+
+ OzoneOutputStream ozoneOutputStream = bucket.createMultipartKey(keyName,
+ sampleData.length(), partNumber, uploadID);
+ ozoneOutputStream.write(string2Bytes(sampleData), 0, sampleData.length());
+ ozoneOutputStream.close();
+
+ OmMultipartCommitUploadPartInfo commitUploadPartInfo = ozoneOutputStream
+ .getCommitUploadPartInfo();
+
+ String partName = commitUploadPartInfo.getPartName();
+ Map<Integer, String> partsMap = new HashMap<>();
+ partsMap.put(partNumber, partName);
+ bucket.completeMultipartUpload(keyName, uploadID, partsMap);
+
+ Assert.assertEquals(volume.getBucket(bucketName).getUsedBytes(), 137228);
Review Comment:
I guess its the EC data at 1024 chunk size stripped across the 3 nodes and 2 parity. But I think we need a comment or a calculation here to show were it comes from. Otherwise its just a magic number that does make sense to anyone 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