-
Notifications
You must be signed in to change notification settings - Fork 588
HDDS-3727. Volume space: check quotaUsageInBytes when write key. #1434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
hi @ChenSammi @xiaoyuyao, this PR add judge whether we can be write if space quota enable. Can you help to review this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
QUOTA_CHECK_ERROR -> QUOTA_EXCEEDED
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add used bytes check in each test case?
ea519b5 to
8d94b31
Compare
| long preAllocatedSpace = newLocationList.size() | ||
| * ozoneManager.getScmBlockSize() | ||
| * omKeyInfo.getFactor().getNumber(); | ||
| if (omVolumeArgs.getQuotaInBytes() > OzoneConsts.QUOTA_RESET) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a isQuotaInBytesSet check in OmVolumeArgs and use this function instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to put the if in checkVolumeQuotaInBytes, which might seem a little more compact.
| // volume Args will be persisted to the DB. | ||
| // TODO: There is a delay in updating DB in this way, and if necessary | ||
| // we can modify the ErrorOMResponse to avoid it. | ||
| omVolumeArgs.getUsedBytes().add(-keyAllocatedSpace); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If A object has multiple blocks, and quota_exceeded exception is thrown out at the last block, shall we only deduct the block space size at this step?
| // AllocateBlock failed, volume usedBytes should be 0 | ||
| Assert.assertEquals(0L, store.getVolume(volumeName).getUsedBytes()); | ||
|
|
||
| Assert.assertEquals(3, countException); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a multiple block write case here,so the first few block will succeed, while the later block will fail due to quota exceed?
| + allocateSize) + " Bytes.", | ||
| OMException.ResultCodes.QUOTA_EXCEEDED); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnessary empty line.
|
|
||
| return omClientResponse; | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnessary blank line.
|
Thanks @ChenSammi for the review. Review issues has been fixed. |
|
LGTM +1. |
What changes were proposed in this pull request?
In addition, the current Quota setting does not take effect. HDDS-541 gives all the work needed to perfect Quota.
This PR is a subtask of HDDS-541.
Volume has implemented increase usedBytes when write, and this PR is based on #1296.
In this PR we judge whether the Volume can be written when we write the key if the volume space quota enable.
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-3727
How was this patch tested?
UT added.