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/04/04 08:36:22 UTC
[GitHub] [ozone] sumitagrawl opened a new pull request, #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
sumitagrawl opened a new pull request, #4529:
URL: https://github.com/apache/ozone/pull/4529
## What changes were proposed in this pull request?
- validation for namespace quota for bucket count when setQuota for volume for namespace quota
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8367
Please replace this section with the link to the Apache JIRA)
## How was this patch tested?
Unit test cases
--
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 #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4529:
URL: https://github.com/apache/ozone/pull/4529#discussion_r1159208193
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -94,9 +94,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
getOmRequest());
- // In production this will never happen, this request will be called only
- // when we have quota in bytes is set in setVolumePropertyRequest.
- if (!setVolumePropertyRequest.hasQuotaInBytes()) {
+ if (!setVolumePropertyRequest.hasQuotaInBytes()
+ && !setVolumePropertyRequest.hasQuotaInNamespace()) {
Review Comment:
@sadanand48 This check if both space quota and namespace quota is not present, then return failure. if Either of them is present, it will progress
--
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] sadanand48 commented on a diff in pull request #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #4529:
URL: https://github.com/apache/ozone/pull/4529#discussion_r1158249472
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -228,11 +227,24 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
return true;
}
- public boolean checkQuotaNamespaceValid(long quotaInNamespace) {
-
+ public boolean checkQuotaNamespaceValid(OMMetadataManager metadataManager,
+ long quotaInNamespace, String volumeName) throws IOException {
if (quotaInNamespace < OzoneConsts.QUOTA_RESET || quotaInNamespace == 0) {
return false;
}
+
+ // if volume quota is for reset, no need further check
+ if (quotaInNamespace == OzoneConsts.QUOTA_RESET) {
+ return true;
+ }
+
+ List<OmBucketInfo> bucketList = metadataManager.listBuckets(
+ volumeName, null, null, Integer.MAX_VALUE);
+ if (bucketList.size() > quotaInNamespace) {
Review Comment:
Doesn't volume namespace quota mean the Sum(all bucket namespace quotas) ?
According this code here volume namespace quota = number of buckets in volume as we are checking `bucketList.size()`
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -94,9 +94,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
getOmRequest());
- // In production this will never happen, this request will be called only
- // when we have quota in bytes is set in setVolumePropertyRequest.
- if (!setVolumePropertyRequest.hasQuotaInBytes()) {
+ if (!setVolumePropertyRequest.hasQuotaInBytes()
+ && !setVolumePropertyRequest.hasQuotaInNamespace()) {
Review Comment:
This means every set volume request needs to include a namespace quota param in its request, If I just want to change space quota does the request populate namespace quota by default?
--
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] sadanand48 commented on pull request #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on PR #4529:
URL: https://github.com/apache/ozone/pull/4529#issuecomment-1503624712
Thanks @sumitagrawl for the patch and @krishnaasawa1 for the 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] sumitagrawl commented on a diff in pull request #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4529:
URL: https://github.com/apache/ozone/pull/4529#discussion_r1162514546
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -228,11 +227,24 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
return true;
}
- public boolean checkQuotaNamespaceValid(long quotaInNamespace) {
-
+ public boolean checkQuotaNamespaceValid(OMMetadataManager metadataManager,
Review Comment:
Yes, it’sproper
--
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] sadanand48 merged pull request #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 merged PR #4529:
URL: https://github.com/apache/ozone/pull/4529
--
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] sadanand48 commented on a diff in pull request #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "sadanand48 (via GitHub)" <gi...@apache.org>.
sadanand48 commented on code in PR #4529:
URL: https://github.com/apache/ozone/pull/4529#discussion_r1159303216
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -228,11 +227,24 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
return true;
}
- public boolean checkQuotaNamespaceValid(long quotaInNamespace) {
-
+ public boolean checkQuotaNamespaceValid(OMMetadataManager metadataManager,
+ long quotaInNamespace, String volumeName) throws IOException {
if (quotaInNamespace < OzoneConsts.QUOTA_RESET || quotaInNamespace == 0) {
return false;
}
+
+ // if volume quota is for reset, no need further check
+ if (quotaInNamespace == OzoneConsts.QUOTA_RESET) {
+ return true;
+ }
+
+ List<OmBucketInfo> bucketList = metadataManager.listBuckets(
+ volumeName, null, null, Integer.MAX_VALUE);
+ if (bucketList.size() > quotaInNamespace) {
Review Comment:
Thanks @sumitagrawl , we should document this somewhere , I couldn’t find this info anywhere in code or existing docs, It can get confusing at times as namespace is usually a collection of all objects inside an entity.
--
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] krishnaasawa1 commented on a diff in pull request #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "krishnaasawa1 (via GitHub)" <gi...@apache.org>.
krishnaasawa1 commented on code in PR #4529:
URL: https://github.com/apache/ozone/pull/4529#discussion_r1162611113
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -228,11 +227,24 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
return true;
}
- public boolean checkQuotaNamespaceValid(long quotaInNamespace) {
-
+ public boolean checkQuotaNamespaceValid(OMMetadataManager metadataManager,
+ long quotaInNamespace, String volumeName) throws IOException {
if (quotaInNamespace < OzoneConsts.QUOTA_RESET || quotaInNamespace == 0) {
return false;
}
+
+ // if volume quota is for reset, no need further check
+ if (quotaInNamespace == OzoneConsts.QUOTA_RESET) {
+ return true;
+ }
+
+ List<OmBucketInfo> bucketList = metadataManager.listBuckets(
+ volumeName, null, null, Integer.MAX_VALUE);
+ if (bucketList.size() > quotaInNamespace) {
Review Comment:
okay saw in latest documentation "Linked bucket will consume namespace quota of volume"
--
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 #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4529:
URL: https://github.com/apache/ozone/pull/4529#discussion_r1159206337
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -228,11 +227,24 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
return true;
}
- public boolean checkQuotaNamespaceValid(long quotaInNamespace) {
-
+ public boolean checkQuotaNamespaceValid(OMMetadataManager metadataManager,
+ long quotaInNamespace, String volumeName) throws IOException {
if (quotaInNamespace < OzoneConsts.QUOTA_RESET || quotaInNamespace == 0) {
return false;
}
+
+ // if volume quota is for reset, no need further check
+ if (quotaInNamespace == OzoneConsts.QUOTA_RESET) {
+ return true;
+ }
+
+ List<OmBucketInfo> bucketList = metadataManager.listBuckets(
+ volumeName, null, null, Integer.MAX_VALUE);
+ if (bucketList.size() > quotaInNamespace) {
Review Comment:
@sadanand48 Volume quota is number of buckets it can hold, its not sum of namespace quota for all buckets.
--
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 #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4529:
URL: https://github.com/apache/ozone/pull/4529#discussion_r1162545796
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -228,11 +227,24 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
return true;
}
- public boolean checkQuotaNamespaceValid(long quotaInNamespace) {
-
+ public boolean checkQuotaNamespaceValid(OMMetadataManager metadataManager,
+ long quotaInNamespace, String volumeName) throws IOException {
if (quotaInNamespace < OzoneConsts.QUOTA_RESET || quotaInNamespace == 0) {
return false;
}
+
+ // if volume quota is for reset, no need further check
+ if (quotaInNamespace == OzoneConsts.QUOTA_RESET) {
+ return true;
+ }
+
+ List<OmBucketInfo> bucketList = metadataManager.listBuckets(
+ volumeName, null, null, Integer.MAX_VALUE);
+ if (bucketList.size() > quotaInNamespace) {
Review Comment:
There was a lack of clarity for linked bucket which is also updated and already merged as part of HDDS-8016,
linked bucket is included in namespace quota count of volume containing it.
--
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 #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #4529:
URL: https://github.com/apache/ozone/pull/4529#discussion_r1162514546
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -228,11 +227,24 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
return true;
}
- public boolean checkQuotaNamespaceValid(long quotaInNamespace) {
-
+ public boolean checkQuotaNamespaceValid(OMMetadataManager metadataManager,
Review Comment:
Yes, it handled earlier with HDDS-7481.
--
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] krishnaasawa1 commented on a diff in pull request #4529: HDDS-8367. setquota should have a check on usedNamespace [NameSpacequota]
Posted by "krishnaasawa1 (via GitHub)" <gi...@apache.org>.
krishnaasawa1 commented on code in PR #4529:
URL: https://github.com/apache/ozone/pull/4529#discussion_r1161599036
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -228,11 +227,24 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
return true;
}
- public boolean checkQuotaNamespaceValid(long quotaInNamespace) {
-
+ public boolean checkQuotaNamespaceValid(OMMetadataManager metadataManager,
Review Comment:
Similar to this, I assume we already have proper checks for bucket namespace quota is enabled, the total number of keys under the bucket can not be updated in case bucket already has more number of keys.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/volume/OMVolumeSetQuotaRequest.java:
##########
@@ -228,11 +227,24 @@ public boolean checkQuotaBytesValid(OMMetadataManager metadataManager,
return true;
}
- public boolean checkQuotaNamespaceValid(long quotaInNamespace) {
-
+ public boolean checkQuotaNamespaceValid(OMMetadataManager metadataManager,
+ long quotaInNamespace, String volumeName) throws IOException {
if (quotaInNamespace < OzoneConsts.QUOTA_RESET || quotaInNamespace == 0) {
return false;
}
+
+ // if volume quota is for reset, no need further check
+ if (quotaInNamespace == OzoneConsts.QUOTA_RESET) {
+ return true;
+ }
+
+ List<OmBucketInfo> bucketList = metadataManager.listBuckets(
+ volumeName, null, null, Integer.MAX_VALUE);
+ if (bucketList.size() > quotaInNamespace) {
Review Comment:
Just clarification -> This size will not include "Linked buckets" .correct?. Ozone quota doc talks about linked bucket should not be included in namespacequota
--
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