HDDS-7649. S3 multipart upload EC release space quota wrong for old version #4095
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
adoroszlai
reviewed
Dec 15, 2022
Contributor
adoroszlai
left a comment
adoroszlai
left a comment
There was a problem hiding this comment.
Thanks @sumitagrawl for the fix, LGTM. The tests could use some refactoring though.
sodonnel
reviewed
Dec 15, 2022
| partsMap.put(partNumber, partName); | ||
| bucket.completeMultipartUpload(keyName, uploadID, partsMap); | ||
|
|
||
| Assert.assertEquals(volume.getBucket(bucketName).getUsedBytes(), 137228); |
Contributor
There was a problem hiding this comment.
Where does "137228" come from? How it is derived?
Contributor
There was a problem hiding this 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.
sodonnel
reviewed
Dec 16, 2022
| @Test | ||
| public void testUploadTwiceWithEC() throws IOException { | ||
|
|
||
| String volumeName = UUID.randomUUID().toString(); |
Contributor
There was a problem hiding this 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?
sodonnel
reviewed
Dec 16, 2022
| String volumeName = UUID.randomUUID().toString(); | ||
| String bucketName = UUID.randomUUID().toString(); | ||
| String keyName = UUID.randomUUID().toString(); | ||
| StringBuilder sb = new StringBuilder("sample Value"); |
Contributor
There was a problem hiding this 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?
sumitagrawl
requested review from
sodonnel
and removed request for
siddhantsangwan
errose28
added a commit
to errose28/ozone
that referenced
this pull request
Dec 21, 2022
* master: (88 commits) HDDS-7463. SCM Pipeline scrubber never able to cleanup allocated pipeline. (apache#4093) HDDS-7683. EC: ReplicationManager - UnderRep maintenance handler should not request nodes if none needed (apache#4109) HDDS-7635. Update failure metrics when allocate block fails in preExecute. (apache#4086) HDDS-7565. FSO purge directory for old bucket can update quota for new bucket (apache#4021) HDDS-7654. EC: ReplicationManager - merge mis-rep queue into under replicated queue (apache#4099) HDDS-7621. Update SCM term in datanode from heartbeat without any commands (apache#4101) HDDS-7649. S3 multipart upload EC release space quota wrong for old version (apache#4095) HDDS-7399. Enable specifying external root ca (apache#4053) HDDS-7398. Tool to remove old certs from the scm db (apache#3972) HDDS-6650. S3MultipartUpload support update bucket usedNamespace. (apache#4081) HDDS-7605. Improve logging in Container Balancer (apache#4067) HDDS-7616. EC: Refactor Unhealthy Replicated Processor (apache#4063) HDDS-7426. Add a new acceptance test for Streaming Pipeline. (apache#4019) HDDS-7478. [Ozone-Streaming] NPE in when creating a file with o3fs. (apache#3949) HDDS-7425. Add documentation for the new Streaming Pipeline feature. (apache#3913) HDDS-7438. [Ozone-Streaming] Add a createStreamKey method to OzoneBucket. (apache#3914) HDDS-7431. [Ozone-Streaming] Disable data steam by default. (apache#3900) HDDS-6955. [Ozone-streaming] Add explicit stream flag in ozone shell (apache#3559) HDDS-6867. [Ozone-Streaming] PutKeyHandler should not use streaming to put EC key. (apache#3516) HDDS-6842. [Ozone-Streaming] Reduce the number of watch requests in StreamCommitWatcher. (apache#3492) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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