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 2020/12/09 02:38:22 UTC

[GitHub] [ozone] captainzmc opened a new pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

captainzmc opened a new pull request #1677:
URL: https://github.com/apache/ozone/pull/1677


   ## What changes were proposed in this pull request?
   
   long quotaInBytes are new fields in the bucketArgs, this field will go to 0 by default in the old bucket when the old cluster is upgraded. At this point, the data writes are rejected.
   
   This is similar to creating a bucket without specifying quotaInBytes, where quotaInBytes is set to -1 by default via getQuotaValue. We can use 0 as a special term.
   
   ![image](https://user-images.githubusercontent.com/13825159/101566773-875c7700-3a0a-11eb-847e-f40d20a49cc4.png)
   
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-4562
   
   ## How was this patch tested?
   
   ut added
   


----------------------------------------------------------------
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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-743944374


   > Hi @captainzmc
   > Few questions:
   > 
   > 1. Can we make use of proto default value to -1 for quotaInBytes value in proto.
   > 2. So on old buckets, we cannot set quota, as we don't have any info of bytesUsed/namespace count, or if it can be set how this will be handled?
   > 3. And how upgrades are handled for quota feature overall, like for older volumes and buckets under it?
   
   Thanks for @bharatviswa504’s advices.
   1. For now, we use getQuotaValue(long quota) to handle the default case in [RpcClient.](https://github.com/apache/ozone/blob/master/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java#L463) And setting the default in proto is also a good way, I'll change that later.
   2. After adding usedByets, the original keys of the old bucket cannot be counted, so we can only count the newly written keys. Therefore, we temporarily did not recommend old buckets to enable Quota (because their usedByets were inaccurate).
   3. For old buckets we do not recommend that quota be enabled, but in order not to affect the write, I handle the default at [checkBucketQuotaInBytes](https://github.com/apache/ozone/pull/1677/files#diff-2841a017f8d700d802af1b1c49fea9ca281098728796384f5b2e79c23f09ab65R579) in this PR.


----------------------------------------------------------------
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.

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] linyiqun commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545549566



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Okay, go ahead for this.




----------------------------------------------------------------
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.

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 edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-746688407


   > > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > > 
   > > 
   > > If we want to disallow the quota support for older buckets, one idea to do at code level.
   > 
   > User using old Ozone version may still has the desire to use the new Quota feature. It's better not shut the door completely.
   > @adoroszlai , I'm not sure if it's feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user? So that user has the choice either tolerate the inaccurate bytesInUsed or a relative longer upgrade time to get accurate bytesInUsed of buckets.
   
   @ChenSammi Above idea is to detect older buckets/volumes in the Ozone cluster, from discussion as it is said not recommended, said we can fail the operation But, if we want to support, when they setQuota, we can know this is an old bucket, we can say a warning message to the user, older keys are not considered.
   
   I am fine with whatever way we go, if we clearly document the behavior it will not be confusing to end-users.


----------------------------------------------------------------
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.

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 pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745414550


   >The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   
   If we want to disallow the quota support for older buckets, one idea to do at code level.


----------------------------------------------------------------
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.

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 #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r543558554



##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota
 
 Administrators should be able to define how much storage space a Volume or Bucket can use. The following Settings for Storage space quota are currently supported:
+
 a. By default, the quota for volume and bucket is not enabled.
+
 b. When volume quota is enabled, the total size of bucket quota cannot exceed volume.
+
 c. Bucket quota can be set separately without enabling Volume quota. The size of bucket quota is unrestricted at this point.
+
 d. Volume quota is not currently supported separately, and volume quota takes effect only if bucket quota is set. Because ozone only check the usedBytes of the bucket when we write the key.
 
+e. If the cluster is upgraded from old version less than 1.1.0, use of quota on older volumes and buckets is not recommended. Since the old key is not counted to the bucket's usedBytes, the quota setting is inaccurate at this point.

Review comment:
       Can we also add documentation about the clear space quota and the behavior?

##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota
 
 Administrators should be able to define how much storage space a Volume or Bucket can use. The following Settings for Storage space quota are currently supported:
+
 a. By default, the quota for volume and bucket is not enabled.
+
 b. When volume quota is enabled, the total size of bucket quota cannot exceed volume.

Review comment:
       Minor:
   When volume quota is enabled, the total quota of the buckets, cannot exceed the volume quota.




----------------------------------------------------------------
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.

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 #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545610249



##########
File path: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
##########
@@ -503,8 +503,8 @@ message BucketInfo {
     optional string sourceVolume = 12;
     optional string sourceBucket = 13;
     optional uint64 usedBytes = 14;
-    optional uint64 quotaInBytes = 15;
-    optional uint64 quotaInNamespace = 16;
+    optional int64 quotaInBytes = 15 [default = -2];

Review comment:
       https://github.com/apache/ozone/pull/1677#issuecomment-745019611
   Any reason for moving back to int64?




----------------------------------------------------------------
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.

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 pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-743398722


   Not related to this PR, can you also provide info on how the clear space quota works and it's usage.
   
   Because I see when bucket clear space quota comes, we reset to -1, but what will happen to volume quota (Will it reclaim bucket quota? or any information on clear space quota usage and how it works also can you share some info.
   


----------------------------------------------------------------
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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745268294


   Updated PR and fixed review issues.


----------------------------------------------------------------
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.

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 #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r546333895



##########
File path: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
##########
@@ -503,8 +503,8 @@ message BucketInfo {
     optional string sourceVolume = 12;
     optional string sourceBucket = 13;
     optional uint64 usedBytes = 14;
-    optional uint64 quotaInBytes = 15;
-    optional uint64 quotaInNamespace = 16;
+    optional int64 quotaInBytes = 15 [default = -2];

Review comment:
       Interesting, thanks for info.




----------------------------------------------------------------
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.

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] amaliujia commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
amaliujia commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r543633161



##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota

Review comment:
       I can add namespace quota to this documentation: https://issues.apache.org/jira/browse/HDDS-4594. Before adding to documentation, I will first finish namespace support on bucket: https://issues.apache.org/jira/browse/HDDS-4277




----------------------------------------------------------------
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.

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] amaliujia commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745521187


   O, actually from the code seems that setting default value as -1 is solved in proto. If so please ignore my previous comment (which assumed that proto cannot set -1 as default value for quota)


----------------------------------------------------------------
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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745019611


   > Might be use int64, but protobuf document said it is inefficient for negative numbers, not sure it is good idea. Just thought of bringing this up here?
   
   I'll change the type to sint64, which is both negative by default and more efficient than int64.


----------------------------------------------------------------
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.

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 edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-746694383


   > > > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > > > 
   > > > 
   > > > If we want to disallow the quota support for older buckets, one idea to do at code level.
   > > 
   > > 
   > > User using old Ozone version may still has the desire to use the new Quota feature. It's better not shut the door completely.
   > > @adoroszlai , I'm not sure if it's feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user? So that user has the choice either tolerate the inaccurate bytesInUsed or a relative longer upgrade time to get accurate bytesInUsed of buckets.
   > 
   > Hi @ChenSammi, I think it's dangerous recalculating usedBytes in upgrade . If the amount of data is too large, it will greatly increase the time, and if the processing is not good, it will cause the upgrade to fail.
   > If we want use Quota in old buckets, we can add a command to recalculate bucket usedBytes to minimize the impact. User can decide for himself which buckets to recalculate.
   
   >Yes during upgrade might not be a feasible idea, it will take a long time for an upgrade if there are many buckets/keys in the cluster.
   
   
    If we want to use Quota in old buckets, we can add a command to recalculate bucket usedBytes to minimize the impact. User can decide for himself which buckets to recalculate.
   Even this will be a costly operation, during that operation we should acquire bucket read lock and do the calculation, so all writes will be stalled in the system. (As during this operation, we should not allow new writes to get accurate bytesused).
   
   
   


----------------------------------------------------------------
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.

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] linyiqun commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545183356



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Why we change above condition check? Above check just means that we enabled the quota, and current quota isn't equal to default value. This should be a more accurate check. 




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r544215535



##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota
 
 Administrators should be able to define how much storage space a Volume or Bucket can use. The following Settings for Storage space quota are currently supported:
+
 a. By default, the quota for volume and bucket is not enabled.
+
 b. When volume quota is enabled, the total size of bucket quota cannot exceed volume.
+
 c. Bucket quota can be set separately without enabling Volume quota. The size of bucket quota is unrestricted at this point.
+
 d. Volume quota is not currently supported separately, and volume quota takes effect only if bucket quota is set. Because ozone only check the usedBytes of the bucket when we write the key.
 
+e. If the cluster is upgraded from old version less than 1.1.0, use of quota on older volumes and buckets is not recommended. Since the old key is not counted to the bucket's usedBytes, the quota setting is inaccurate at this point.

Review comment:
       > Can we also add documentation about the clear space quota and the behavior?
   
   In HDDS-4588, I will modify the current clear logic, and also modify the document of clear space quota.




----------------------------------------------------------------
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.

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] ChenSammi edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
ChenSammi edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745806878


   > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > 
   > If we want to disallow the quota support for older buckets, one idea to do at code level.
   
   User using old Ozone version may still has the desire to use the new Quota feature.  It's better not shut the door completely. 
   @adoroszlai ,  I'm not sure if it's feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user?   So that user has the choice either tolerate the inaccurate bytesInUsed or a relative longer upgrade time to get accurate bytesInUsed of 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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545526391



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       @linyiqun Here with omVolumeArgs.getQuotaInNamespace () > 0 is enough? Because when it > 0 is certainly is not equal to -1.
   




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and Old Quoat, there is no negative case.




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and old quota, there is no negative case.
   
   If add omVolumeArgs. getQuotaInNamespace ()! = 2. The judgment condition will increase too much and the code will not be clear enough, we can use omVolumeArgs. getQuotaInNamespace () > 0, this will make the code much cleaner.




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r543018536



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -291,9 +291,10 @@ public boolean setVolumeOwner(String volumeName, String owner)
   public void setVolumeQuota(String volumeName, long quotaInCounts,
       long quotaInBytes) throws IOException {
     HddsClientUtils.verifyResourceName(volumeName);
-    verifyCountsQuota(quotaInCounts);
-    verifySpaceQuota(quotaInBytes);
-    ozoneManagerClient.setQuota(volumeName, quotaInCounts, quotaInBytes);
+    verifyCountsQuota(getQuotaValue(quotaInCounts));

Review comment:
       This check simply ensures that when we use RpcClient set Quota, the value of quota must be legal (not less than -1). But the getQuotaValue conversion step inside is redundant,  I‘ll  delete this conversion.




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Thanks for @linyiqun's feedback.
   Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and old quota, there is no negative case.
   
   In addition to using omVolumeArgs.getQuotaInNamespace ()! = OzoneConsts.QUOTA_RESET also need add omvolumeargs.getQuotainNamespace ()! = 2. The judgment condition will increase too much and the code will not be clear enough, we can use omVolumeArgs. getQuotaInNamespace () > 0, this will make the code much cleaner.




----------------------------------------------------------------
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.

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] captainzmc edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-746150718


   > > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > > 
   > > 
   > > If we want to disallow the quota support for older buckets, one idea to do at code level.
   > 
   > User using old Ozone version may still has the desire to use the new Quota feature. It's better not shut the door completely.
   > @adoroszlai , I'm not sure if it's feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user? So that user has the choice either tolerate the inaccurate bytesInUsed or a relative longer upgrade time to get accurate bytesInUsed of buckets.
   
   Hi @ChenSammi, I think it's dangerous recalculating usedBytes in upgrade . If the amount of data is too large, it will greatly increase the time, and if the processing is not good, it will cause the upgrade to fail.
   If we want use Quota in old buckets, we can add a command to recalculate bucket usedBytes to minimize the impact. User can decide for himself which buckets to recalculate.


----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545657153



##########
File path: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
##########
@@ -503,8 +503,8 @@ message BucketInfo {
     optional string sourceVolume = 12;
     optional string sourceBucket = 13;
     optional uint64 usedBytes = 14;
-    optional uint64 quotaInBytes = 15;
-    optional uint64 quotaInNamespace = 16;
+    optional int64 quotaInBytes = 15 [default = -2];

Review comment:
       Using sint64, I found that the default value changed to -9223372036854775808. This could be a compatibility issue. So I switched to int64.
   ![image](https://user-images.githubusercontent.com/13825159/102593001-4291b800-414f-11eb-84b8-3bd0b33290a6.png)
   




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r544233345



##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota
 
 Administrators should be able to define how much storage space a Volume or Bucket can use. The following Settings for Storage space quota are currently supported:
+
 a. By default, the quota for volume and bucket is not enabled.
+
 b. When volume quota is enabled, the total size of bucket quota cannot exceed volume.
+
 c. Bucket quota can be set separately without enabling Volume quota. The size of bucket quota is unrestricted at this point.
+
 d. Volume quota is not currently supported separately, and volume quota takes effect only if bucket quota is set. Because ozone only check the usedBytes of the bucket when we write the key.
 
+e. If the cluster is upgraded from old version less than 1.1.0, use of quota on older volumes and buckets is not recommended. Since the old key is not counted to the bucket's usedBytes, the quota setting is inaccurate at this point.

Review comment:
       Currently, we have not agreed on whether bucket can be enabled for the old quota. At present, we only put forward the suggestion in the document very briefly. If the requirement is clear, we can issue Jira and solve 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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-743945558


   > @captainzmc see #1691 for acceptance test with [two lines that are currently disabled](https://github.com/apache/ozone/pull/1691/files#diff-1a0d871a569dc91266016ce6723339dc51189088e204b3460b092d925e553c2eR68-R70) to verify this fix. If you could review that PR, then uncommenting them in this PR would improve test coverage.
   
   Thanks for @adoroszlai 's suggestion, I will deal with the acceptance test 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.

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] captainzmc edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745026394


   > Example:
   > V1 - 100 MB
   > V1/B1 - 50MB
   > V1/B2- 50MB
   > 
   > Now clear quota on V1/B2 we set quota of V1/B2 to -1, and now a new bucket can be still created in this volume?
   > 
   > New Bucket created with quota 50MB. (V1/B3)
   > 
   > So, in this volume there are 3 buckets, only 2 are considered in to count, as clearQuota is run(on V1/B2), and we are not considering this bucket in quota calculations, so this bucket will not be counted under quota, but for volume we have crossed the quota of that volume. (As user has run clear quota on V1/B2)
   > 
   > So, here clear quota means resetting the quota to -1? Or what is the real purpose of this clearQuota usage on clusters, in what scenarios this will be useful on the cluster?
   
   Thanks to @bharatviswa504‘s advice, you found a very important point.
   Before HDDS-4308, we had usedBytes in volume, so before writing the key, we checked both bucket quota and volume quota. Setting bucket quota to -1 alone at this point does not matter, since we are able to ensure that usedBytes do not exceed volume quota.
   But we can't do that now because volume doesn't have usedBytes. I created a new JIRA HDDS-4588 to fix this problem. If volume's quota is enabled then bucket's quota cannot be cleared. We need to prompt the user to clear volume quota first.
   
   > But I don't see any guards in the cluster to not allow quota operations for older buckets/volumes.
   > 
   > And also do you think, we need to document this in our docs?
   
   I’ll  refine the docs to make it clear that older volumes/buckets are not recommended to use quota.
   
   
   
   


----------------------------------------------------------------
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.

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] linyiqun commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545530749



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       @captainzmc , here my point is that we should firstly check if the quota was enabled by using one variable flag(here I understand is OzoneConsts.QUOTA_RESET). Then we check the quota value if it's valid. Is there a switch config for the quota now?  If the quota is not enabled, we don't need to do the quota check anymore.




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545657153



##########
File path: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
##########
@@ -503,8 +503,8 @@ message BucketInfo {
     optional string sourceVolume = 12;
     optional string sourceBucket = 13;
     optional uint64 usedBytes = 14;
-    optional uint64 quotaInBytes = 15;
-    optional uint64 quotaInNamespace = 16;
+    optional int64 quotaInBytes = 15 [default = -2];

Review comment:
       Using sint64, I found that the default value changed to -9223372036854775808 after switching to long. This could be a compatibility issue. So I switched to int64.
   ![image](https://user-images.githubusercontent.com/13825159/102593001-4291b800-414f-11eb-84b8-3bd0b33290a6.png)
   




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and Old Quoat, there is no negative case.
   
   If add omVolumeArgs. getQuotaInNamespace ()! = 2. The judgment condition will increase too much and the code will not be clear enough, we can use omVolumeArgs. getQuotaInNamespace () > 0.




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and Old Quoat, there is no negative case.
   
   If add omVolumeArgs. getQuotaInNamespace ()! = 2. The judgment condition will increase too much and the code will not be clear enough, we can use omVolumeArgs. getQuotaInNamespace () > 0, this will make the code much cleaner.




----------------------------------------------------------------
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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-743944634


   > Not related to this PR, can you also provide info on how the clear space quota works and it's usage.
   > 
   > Because I see when bucket clear space quota comes, we reset to -1, but what will happen to volume quota (Will it reclaim bucket quota? or any information on clear space quota usage and how it works also can you share some info.
   
   Yes, I have submitted the use of the latest quota in Doc. Can be seen here:
   https://ci-hadoop.apache.org/view/Hadoop%20Ozone/job/ozone-doc-master/lastSuccessfulBuild/artifact/hadoop-hdds/docs/public/feature/quota.html


----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Thanks for @linyiqun's feedback.
   Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/329bcc44968da2d16420d77a0b8eb55c94c6f4ee/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java#L453), Therefore, except for clear quota and old quota, there is no negative case.
   
   In addition to using omVolumeArgs.getQuotaInNamespace ()! = OzoneConsts.QUOTA_RESET also need add omvolumeargs.getQuotainNamespace ()! = 2. The judgment condition will increase too much and the code will not be clear enough, we can use omVolumeArgs. getQuotaInNamespace () > 0, this will make the code much cleaner.




----------------------------------------------------------------
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.

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 edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-744772894


   > https://ci-hadoop.apache.org/view/Hadoop%20Ozone/job/ozone-doc-master/lastSuccessfulBuild/artifact/hadoop->hdds/docs/public/feature/quota.html
   
   >1. When clear volume quota(Assuming volume quota was set before), it is just like setting the quota on the volume to -1, >when clear volume quota is called?
   >2. When volume quota is set, bucket clear quota, it is like just setting the quota for that bucket to -1, and it will allow new >buckets can be created if volume quota is still available(As previous bucket quota cleared quota)
   
   When volume quota is not set and bucket quota is set, it is just reset quota of the bucket to -1. when clear bucket quota operation is performed
   
   Is my understanding correct here? 
   
   Example:
   V1 - 100 MB
   V1/B1 - 50MB
   V1/B2- 50MB
   
   
   Now clear quota on V1/B2 we set quota of V1/B2 to -1, and now a new bucket can be still created in this volume? 
   
   New Bucket created with quota 50MB. (V1/B3)
   
   So, in this volume there are 3 buckets, only 2 are considered in to count, as clearQuota is run(on V1/B2), and we are not considering this bucket in quota calculations, so this bucket will not be counted under quota, but for volume we have crossed the quota of that volume. (As user has run clear quota on V1/B2)
   
   So, here clear quota means resetting the quota to -1? Or what is the real purpose of this clearQuota usage on clusters, in what scenarios this will be useful on the cluster?
   
   
   
   >For old buckets, we do not recommend that quota be enabled, but in order not to affect the write, I handle the default at >checkBucketQuotaInBytes in this PR.
   
   But I don't see any guards in the cluster to not allow this for older 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.

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 pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-746694383


   > > > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > > > 
   > > > 
   > > > If we want to disallow the quota support for older buckets, one idea to do at code level.
   > > 
   > > 
   > > User using old Ozone version may still has the desire to use the new Quota feature. It's better not shut the door completely.
   > > @adoroszlai , I'm not sure if it's feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user? So that user has the choice either tolerate the inaccurate bytesInUsed or a relative longer upgrade time to get accurate bytesInUsed of buckets.
   > 
   > Hi @ChenSammi, I think it's dangerous recalculating usedBytes in upgrade . If the amount of data is too large, it will greatly increase the time, and if the processing is not good, it will cause the upgrade to fail.
   > If we want use Quota in old buckets, we can add a command to recalculate bucket usedBytes to minimize the impact. User can decide for himself which buckets to recalculate.
   
   Yes during upgrade might not be a feasible idea, it will take a long time for an upgrade if there are many buckets/keys in the cluster.
   
   
   > If we want use Quota in old buckets, we can add a command to recalculate bucket usedBytes to minimize the impact. User can decide for himself which buckets to recalculate.
   Even this will be a costly operation, during that operation we should acquire bucket read lock and do the calculation, so all writes will be stalled in the system. (As during this operation, we should not allow new writes to get accurate bytesused).
   
   
   


----------------------------------------------------------------
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.

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 edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-744841055


   >Currently, Unsigned field can't have negative default value in proto. If we set [default = -1], an error will be compiled.
   >For now, we use getQuotaValue(long quota) to handle the default case in RpcClient..
   
   Might be use int64, but protobuf document said it is inefficient for negative numbers, not sure it is good idea. Just thought of bringing this up here?
   
   int64 | Uses variable-length encoding. Inefficient for encoding negative numbers – if your field is likely to have negative values, use sint64 instead. | int64 | long | int/long[3] | *int64
   -- | -- | -- | -- | -- | --
   
   I think it will also help if we plan to support quota on older buckets.
   
   The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets quota on older buckets in this way, we can figure it out, just an idea saying 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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r544215535



##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota
 
 Administrators should be able to define how much storage space a Volume or Bucket can use. The following Settings for Storage space quota are currently supported:
+
 a. By default, the quota for volume and bucket is not enabled.
+
 b. When volume quota is enabled, the total size of bucket quota cannot exceed volume.
+
 c. Bucket quota can be set separately without enabling Volume quota. The size of bucket quota is unrestricted at this point.
+
 d. Volume quota is not currently supported separately, and volume quota takes effect only if bucket quota is set. Because ozone only check the usedBytes of the bucket when we write the key.
 
+e. If the cluster is upgraded from old version less than 1.1.0, use of quota on older volumes and buckets is not recommended. Since the old key is not counted to the bucket's usedBytes, the quota setting is inaccurate at this point.

Review comment:
       > Can we also add documentation about the clear space quota and the behavior?
   
   In HDDS-4588, I will modify the current clear logic, and also modify the document of clear space quota.




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and Old Quoat, there is no negative case.
   Increase omVolumeArgs. getQuotaInNamespace ()! = 2. This condition is more, we can use omVolumeArgs. getQuotaInNamespace () > 0.




----------------------------------------------------------------
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.

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] amaliujia commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
amaliujia commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745519108


   @captainzmc 
   
   Can we use `0` to indicate that a quota is not set? Will this better handle the cluster upgrade issue that some buckets by default has 0 quota? 
   
   If a case is really set "0" quota to a bucket thus lock the bucket from creating, then users can choose to use a negative value.


----------------------------------------------------------------
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.

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] captainzmc edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-746119708


   > So, now if the volume quota is not set, the bucket Clearspace quota is just disabling quota on the buckets.
   > 
   > And also just setting quota at volume level will not be counted, until bucket level quotas are set.
   > 
   > For my understanding so volume quota is set, and bucket quota is set, clearing volume quota is just set to -1, so that now quota is tracked at bucket level. And now when bucket quota is cleared, when volume quota is set, we disallow the operation.
   > This will be the behavior of clear space quota, let me know if i am missing something.
   
   @bharatviswa504  Yes. And the problem of clear quota will be fixed in HDDS-4588.


----------------------------------------------------------------
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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-743944607


   > reclaim
   Yes, I have submitted the use of the latest quota in Doc. Can be seen here:
   https://ci-hadoop.apache.org/view/Hadoop%20Ozone/job/ozone-doc-master/lastSuccessfulBuild/artifact/hadoop-hdds/docs/public/feature/quota.html


----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545537817



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       > Current there are two case -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and Old Quoat, there is no negative case.
   > 
   >If add omVolumeArgs. getQuotaInNamespace ()! = 2. The judgment condition will increase too much and the code will not be clear enough, we can use omVolumeArgs. getQuotaInNamespace () > 0.
   
   




----------------------------------------------------------------
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.

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 edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-746694383


   > > > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > > > 
   > > > 
   > > > If we want to disallow the quota support for older buckets, one idea to do at code level.
   > > 
   > > 
   > > User using old Ozone version may still has the desire to use the new Quota feature. It's better not shut the door completely.
   > > @adoroszlai , I'm not sure if it's feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user? So that user has the choice either tolerate the inaccurate bytesInUsed or a relative longer upgrade time to get accurate bytesInUsed of buckets.
   > 
   > Hi @ChenSammi, I think it's dangerous recalculating usedBytes in upgrade . If the amount of data is too large, it will greatly increase the time, and if the processing is not good, it will cause the upgrade to fail.
   > If we want use Quota in old buckets, we can add a command to recalculate bucket usedBytes to minimize the impact. User can decide for himself which buckets to recalculate.
   
   >Yes during upgrade might not be a feasible idea, it will take a long time for an upgrade if there are many buckets/keys in the cluster.
   
   
   > If we want to use Quota in old buckets, we can add a command to recalculate bucket usedBytes to minimize the impact. User can decide for himself which buckets to recalculate.
   
   Even this will be a costly operation, during that operation we should acquire bucket read lock and do the calculation, so all writes will be stalled in the system. (As during this operation, we should not allow new writes to get accurate bytesused).
   
   
   


----------------------------------------------------------------
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.

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 pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-749744666


   If no more comments, I will commit this by tomorrow.


----------------------------------------------------------------
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.

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 #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 merged pull request #1677:
URL: https://github.com/apache/ozone/pull/1677


   


----------------------------------------------------------------
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.

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] linyiqun commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545206641



##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota
 
 Administrators should be able to define how much storage space a Volume or Bucket can use. The following Settings for Storage space quota are currently supported:
+
 a. By default, the quota for volume and bucket is not enabled.
+
 b. When volume quota is enabled, the total size of bucket quota cannot exceed volume.
+
 c. Bucket quota can be set separately without enabling Volume quota. The size of bucket quota is unrestricted at this point.
+
 d. Volume quota is not currently supported separately, and volume quota takes effect only if bucket quota is set. Because ozone only check the usedBytes of the bucket when we write the key.
 
+e. If the cluster is upgraded from old version less than 1.1.0, use of quota on older volumes and buckets is not recommended. Since the old key is not counted to the bucket's usedBytes, the quota setting is inaccurate at this point.

Review comment:
       Make sense to me.




----------------------------------------------------------------
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.

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 #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r542917563



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java
##########
@@ -291,9 +291,10 @@ public boolean setVolumeOwner(String volumeName, String owner)
   public void setVolumeQuota(String volumeName, long quotaInCounts,
       long quotaInBytes) throws IOException {
     HddsClientUtils.verifyResourceName(volumeName);
-    verifyCountsQuota(quotaInCounts);
-    verifySpaceQuota(quotaInBytes);
-    ozoneManagerClient.setQuota(volumeName, quotaInCounts, quotaInBytes);
+    verifyCountsQuota(getQuotaValue(quotaInCounts));

Review comment:
       Why do we need to do this during set Operation?




----------------------------------------------------------------
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.

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] linyiqun commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545530749



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       @captainzmc , here my point is that we should firstly check if the quota was enabled by using one variable flag(here I understand is OzoneConsts.QUOTA_RESET). Then we check the quota value if it's valid. Is there a switch config for the quota now?  If the quota is not enabled, we don't need to do the quota check anymore.
   
   For the current value OzoneConsts.QUOTA_RESET is -1, omVolumeArgs.getQuotaInNamespace () > 0 makes the sense. But once OzoneConsts.QUOTA_RESET  to a positive value, this check won't work. So omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET would still be a better check.




----------------------------------------------------------------
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.

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] captainzmc edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-743944374


   > Hi @captainzmc
   > Few questions:
   > 
   > 1. Can we make use of proto default value to -1 for quotaInBytes value in proto.
   > 2. So on old buckets, we cannot set quota, as we don't have any info of bytesUsed/namespace count, or if it can be set how this will be handled?
   > 3. And how upgrades are handled for quota feature overall, like for older volumes and buckets under it?
   
   Thanks for @bharatviswa504’s advices.
   1. Currently, Unsigned field can't have negative default value in proto. If we set [default = -1], an error will be compiled.
   For now, we use getQuotaValue(long quota) to handle the default case in [RpcClient.](https://github.com/apache/ozone/blob/master/hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java#L463). 
   2. After adding usedByets, the original keys of the old bucket cannot be counted, so we can only count the newly written keys. Therefore, we temporarily did not recommend old buckets to enable Quota (because their usedByets were inaccurate).
   3. For old buckets we do not recommend that quota be enabled, but in order not to affect the write, I handle the default at [checkBucketQuotaInBytes](https://github.com/apache/ozone/pull/1677/files#diff-2841a017f8d700d802af1b1c49fea9ca281098728796384f5b2e79c23f09ab65R579) in this PR.


----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Thanks for @linyiqun's feedback.
   Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and old quota, there is no negative case.
   
   In addition to using omVolumeArgs.getQuotaInNamespace ()! = ozoneconsts.quota_reset also need add omvolumeargs.getQuotainNamespace ()! = 2. The judgment condition will increase too much and the code will not be clear enough, we can use omVolumeArgs. getQuotaInNamespace () > 0, this will make the code much cleaner.




----------------------------------------------------------------
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.

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] ChenSammi commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745806878


   > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > 
   > If we want to disallow the quota support for older buckets, one idea to do at code level.
   
   Is it feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user?  @adoroszlai 


----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545592488



##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota
 
 Administrators should be able to define how much storage space a Volume or Bucket can use. The following Settings for Storage space quota are currently supported:
+
 a. By default, the quota for volume and bucket is not enabled.
+
 b. When volume quota is enabled, the total size of bucket quota cannot exceed volume.
+
 c. Bucket quota can be set separately without enabling Volume quota. The size of bucket quota is unrestricted at this point.
+
 d. Volume quota is not currently supported separately, and volume quota takes effect only if bucket quota is set. Because ozone only check the usedBytes of the bucket when we write the key.
 
+e. If the cluster is upgraded from old version less than 1.1.0, use of quota on older volumes and buckets is not recommended. Since the old key is not counted to the bucket's usedBytes, the quota setting is inaccurate at this point.

Review comment:
       Yes. To prevent conflicts, I have updated the usage and behavior of clear space quota in this PR.




----------------------------------------------------------------
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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-747201968


   Updated PR, set default to -2.  No matter whether or not we support old volume/bucket setting quota, we need to distinguish which volumes and buckets are old. So Here I take @bharatviswa504's advice.


----------------------------------------------------------------
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.

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 pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745413945


   > > Example:
   > > V1 - 100 MB
   > > V1/B1 - 50MB
   > > V1/B2- 50MB
   > > Now clear quota on V1/B2 we set quota of V1/B2 to -1, and now a new bucket can be still created in this volume?
   > > New Bucket created with quota 50MB. (V1/B3)
   > > So, in this volume there are 3 buckets, only 2 are considered in to count, as clearQuota is run(on V1/B2), and we are not considering this bucket in quota calculations, so this bucket will not be counted under quota, but for volume we have crossed the quota of that volume. (As user has run clear quota on V1/B2)
   > > So, here clear quota means resetting the quota to -1? Or what is the real purpose of this clearQuota usage on clusters, in what scenarios this will be useful on the cluster?
   > 
   > Thanks to @bharatviswa504‘s advice, you found a very important point.
   > Before [HDDS-4308](https://issues.apache.org/jira/browse/HDDS-4308), we had usedBytes in volume, so before writing the key, we checked both bucket quota and volume quota. Setting bucket quota to -1 alone at this point does not matter, since we are able to ensure that usedBytes do not exceed volume quota.
   > But we can't do that now because volume doesn't have usedBytes. I created a new JIRA [HDDS-4588](https://issues.apache.org/jira/browse/HDDS-4588) to fix this problem. If volume's quota is enabled then bucket's quota cannot be cleared. We need to prompt the user to clear volume quota first.
   > 
   > > But I don't see any guards in the cluster to not allow quota operations for older buckets/volumes.
   > > And also do you think, we need to document this in our docs?
   > 
   > I’ll refine the docs to make it clear that older volumes/buckets are not recommended to use quota.
   
   So, now if the volume quota is not set, the bucket Clearspace quota is just disabling quota on the buckets.
   
   And also just setting quota at volume level will not be counted, until bucket level quotas are set.
   
   For my understanding so volume quota is set, and bucket quota is set, clearing volume quota is just set to -1, so that now quota is tracked at bucket level. And now when bucket quota is cleared, when volume quota is set, we disallow the operation.
   This will be the behavior of clear space quota, let me know if i am missing something.
    
   > But we can't do that now because volume doesn't have usedBytes. I created a new JIRA [HDDS-4588 
   >(https://issues.apache.org/jira/browse/HDDS-4588) to fix this problem. If volume's quota is enabled then bucket's quota >cannot be cleared. We need to prompt the user to clear volume quota first.
   
   Now the example mentioned scenarios will not happen. Thanks for opening this Jira and tracking this issue.
   
   


----------------------------------------------------------------
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.

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] linyiqun commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r543394366



##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota

Review comment:
       The namespace quota introduction can also be added. We could file a new JIRA for tacking this.

##########
File path: hadoop-hdds/docs/content/feature/Quota.md
##########
@@ -32,11 +32,16 @@ So far, we know that Ozone allows users to create volumes, buckets, and keys. A
 1. Storage Space level quota
 
 Administrators should be able to define how much storage space a Volume or Bucket can use. The following Settings for Storage space quota are currently supported:
+
 a. By default, the quota for volume and bucket is not enabled.
+
 b. When volume quota is enabled, the total size of bucket quota cannot exceed volume.
+
 c. Bucket quota can be set separately without enabling Volume quota. The size of bucket quota is unrestricted at this point.
+
 d. Volume quota is not currently supported separately, and volume quota takes effect only if bucket quota is set. Because ozone only check the usedBytes of the bucket when we write the key.
 
+e. If the cluster is upgraded from old version less than 1.1.0, use of quota on older volumes and buckets is not recommended. Since the old key is not counted to the bucket's usedBytes, the quota setting is inaccurate at this point.

Review comment:
       We can make this check in OM side and throw exception once user try to set quota on older version volumes/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.

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] captainzmc removed a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc removed a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-743944607


   > reclaim
   Yes, I have submitted the use of the latest quota in Doc. Can be seen here:
   https://ci-hadoop.apache.org/view/Hadoop%20Ozone/job/ozone-doc-master/lastSuccessfulBuild/artifact/hadoop-hdds/docs/public/feature/quota.html


----------------------------------------------------------------
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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745026394


   > Example:
   > V1 - 100 MB
   > V1/B1 - 50MB
   > V1/B2- 50MB
   > 
   > Now clear quota on V1/B2 we set quota of V1/B2 to -1, and now a new bucket can be still created in this volume?
   > 
   > New Bucket created with quota 50MB. (V1/B3)
   > 
   > So, in this volume there are 3 buckets, only 2 are considered in to count, as clearQuota is run(on V1/B2), and we are not considering this bucket in quota calculations, so this bucket will not be counted under quota, but for volume we have crossed the quota of that volume. (As user has run clear quota on V1/B2)
   > 
   > So, here clear quota means resetting the quota to -1? Or what is the real purpose of this clearQuota usage on clusters, in what scenarios this will be useful on the cluster?
   
   Thanks to @bharatviswa504‘s advice, you found a very important point.
   Before HDDS-4308, we had usedBytes in volume, so before writing the key, we checked both bucket quota and volume quota. Setting bucket quota to -1 alone at this point does not matter, since we are able to ensure that usedBytes do not exceed volume quota.
   But we can't do that now because volume doesn't have usedBytes. I created a new JIRA HDDS-4588 to fix this problem. If volume's quota is enabled then bucket's quota cannot be cleared. We need to prompt the user to clear volume quota first.
   
   > But I don't see any guards in the cluster to not allow quota operations for older buckets/volumes.
   > 
   > And also do you think, we need to document this in our docs?
   I’ll  refine the docs to make it clear that older volumes/buckets are not recommended to use quota.
   
   
   
   


----------------------------------------------------------------
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.

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 pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-744772894


   > https://ci-hadoop.apache.org/view/Hadoop%20Ozone/job/ozone-doc-master/lastSuccessfulBuild/artifact/hadoop-hdds/docs/public/feature/quota.html
   
   1. When clear volume quota(Assuming volume quota was set before), it is just like setting the quota on the volume to -1, when clear volume quota is called?
   2. When volume quota is set, bucket clear quota, it is like just setting the quota for that bucket to -1, and it will allow new buckets can be created if volume quota is still available(As previous bucket quota cleared quota)
   
   When volume quota is not set and bucket quota is set, it is just reset quota of the bucket to -1. when clear bucket quota operation is performed
   
   Is my understanding correct here? 
   
   Example:
   V1 - 100 MB
   V1/B1 - 50MB
   V1/B2- 50MB
   
   
   Now clear quota on V1/B2 we set quota of V1/B2 to -1, and now a new bucket can be still created in this volume? 
   
   New Bucket created with quota 50MB. (V1/B3)
   
   So, in this volume there are 3 buckets, only 2 are considered in to count, as clearQuota is run(on V1/B2), and we are not considering this bucket in quota calculations, so this bucket will not be counted under quota, but for volume we have crossed the quota of that volume. (As user has run clear quota on V1/B2)
   
   So, here clear quota means resetting the quota to -1? Or what is the real purpose of this clearQuota usage on clusters, in what scenarios this will be useful on the cluster?
   
   
   
   >For old buckets, we do not recommend that quota be enabled, but in order not to affect the write, I handle the default at >checkBucketQuotaInBytes in this PR.
   
   But I don't see any guards in the cluster to not allow this for older 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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745267117


   When the acceptance test upgrade, namespace will also have the problem that old volumes cannot create buckets. This also fixed in this PR.  R: @amaliujia @linyiqun 
   ![image](https://user-images.githubusercontent.com/13825159/102216412-0de6ec00-3f16-11eb-8241-b3ec4b73172f.png)
   


----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and Old Quoat, there is no negative case.
   
   Increase omVolumeArgs. getQuotaInNamespace ()! = 2. This condition is more, we can use omVolumeArgs. getQuotaInNamespace () > 0.




----------------------------------------------------------------
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.

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 edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-744841055


   >Currently, Unsigned field can't have negative default value in proto. If we set [default = -1], an error will be compiled.
   >For now, we use getQuotaValue(long quota) to handle the default case in RpcClient..
   
   Might be use int64, but protobuf document said it is inefficient for negative numbers, not sure it is good idea. Just thought of bringing this up here?
   
   int64 | Uses variable-length encoding. Inefficient for encoding negative numbers – if your field is likely to have negative values, use sint64 instead. | int64 | long | int/long[3] | *int64
   -- | -- | -- | -- | -- | --
   
   I think it will also help if we plan to support quota on older buckets.
   
   The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if one some sets quota on older buckets in this way, we can figure it out.
   
   


----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545536353



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Thanks for @linyiqun's feedback.
   Current there are two case  -1 or -2 (old volume/bucket) not check quota, actually we have guarantee on the client, [user cann’t set negative quota](https://github.com/apache/ozone/blob/master/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/OzoneQuota.java#L194), Therefore, except for clear quota and old quota, there is no negative case.
   
   If add omVolumeArgs. getQuotaInNamespace ()! = 2. The judgment condition will increase too much and the code will not be clear enough, we can use omVolumeArgs. getQuotaInNamespace () > 0, this will make the code much cleaner.




----------------------------------------------------------------
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.

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] adoroszlai commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-743740143


   @captainzmc see #1691 for acceptance test with [two lines that are currently disabled](https://github.com/apache/ozone/pull/1691/files#diff-1a0d871a569dc91266016ce6723339dc51189088e204b3460b092d925e553c2eR68-R70) to verify this fix.  If you could review that PR, then uncommenting them in this PR would improve test coverage.


----------------------------------------------------------------
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.

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] linyiqun commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
linyiqun commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545183356



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java
##########
@@ -314,7 +314,9 @@ private BucketEncryptionInfoProto getBeinfo(
    */
   private void checkQuotaInNamespace(OmVolumeArgs omVolumeArgs,
       long allocatedNamespace) throws IOException {
-    if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET) {
+    // Client has ensured that no negative values other than -1 (clear quota)
+    // will occur when quota is set.
+    if (omVolumeArgs.getQuotaInNamespace() > OzoneConsts.QUOTA_RESET) {

Review comment:
       Why we change above condition check? Above check is to confirmed that the quota is enabled, not a validation check. If we want to make sure quota value is a positive value, I prefer to additionally add omVolumeArgs.getQuotaInNamespace() > 0 check.
   ```java
   if (omVolumeArgs.getQuotaInNamespace() != OzoneConsts.QUOTA_RESET && omVolumeArgs.getQuotaInNamespace() > 0) {
   }
   ```




----------------------------------------------------------------
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.

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] captainzmc commented on a change in pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#discussion_r545657153



##########
File path: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
##########
@@ -503,8 +503,8 @@ message BucketInfo {
     optional string sourceVolume = 12;
     optional string sourceBucket = 13;
     optional uint64 usedBytes = 14;
-    optional uint64 quotaInBytes = 15;
-    optional uint64 quotaInNamespace = 16;
+    optional int64 quotaInBytes = 15 [default = -2];

Review comment:
       Using sint64, I found that the default value changed to -9223372036854775808. This could be a compatibility issue. So I switched to int64, this is correct.
   ![image](https://user-images.githubusercontent.com/13825159/102593001-4291b800-414f-11eb-84b8-3bd0b33290a6.png)
   




----------------------------------------------------------------
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.

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 edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-744772894


   > https://ci-hadoop.apache.org/view/Hadoop%20Ozone/job/ozone-doc-master/lastSuccessfulBuild/artifact/hadoop->hdds/docs/public/feature/quota.html
   
   >1. When clear volume quota(Assuming volume quota was set before), it is just like setting the quota on the volume to -1, >when clear volume quota is called?
   >2. When volume quota is set, bucket clear quota, it is like just setting the quota for that bucket to -1, and it will allow new >buckets can be created if volume quota is still available(As previous bucket quota cleared quota)
   
   When volume quota is not set and bucket quota is set, it is just reset quota of the bucket to -1. when clear bucket quota operation is performed
   
   Is my understanding correct here? 
   
   Example:
   V1 - 100 MB
   V1/B1 - 50MB
   V1/B2- 50MB
   
   
   Now clear quota on V1/B2 we set quota of V1/B2 to -1, and now a new bucket can be still created in this volume? 
   
   New Bucket created with quota 50MB. (V1/B3)
   
   So, in this volume there are 3 buckets, only 2 are considered in to count, as clearQuota is run(on V1/B2), and we are not considering this bucket in quota calculations, so this bucket will not be counted under quota, but for volume we have crossed the quota of that volume. (As user has run clear quota on V1/B2)
   
   So, here clear quota means resetting the quota to -1? Or what is the real purpose of this clearQuota usage on clusters, in what scenarios this will be useful on the cluster?
   
   
   
   >For old buckets, we do not recommend that quota be enabled, but in order not to affect the write, I handle the default at >checkBucketQuotaInBytes in this PR.
   
   But I don't see any guards in the cluster to not allow quota operations for older buckets/volumes. 
   
   And also do you think, we need to document this in our docs?
   
   
   


----------------------------------------------------------------
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.

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 pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-746688407


   > > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > > 
   > > 
   > > If we want to disallow the quota support for older buckets, one idea to do at code level.
   > 
   > User using old Ozone version may still has the desire to use the new Quota feature. It's better not shut the door completely.
   > @adoroszlai , I'm not sure if it's feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user? So that user has the choice either tolerate the inaccurate bytesInUsed or a relative longer upgrade time to get accurate bytesInUsed of buckets.
   
   @ChenSammi Above idea is to detect older buckets/volumes in the Ozone cluster, from discussion as it is said not recommended, said we can fail the operation But, if we want to support, when they setQuota, we can know this is an old bucket, we can say a warning message to the user, older keys are not considered.


----------------------------------------------------------------
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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-746150718


   > > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > > 
   > > 
   > > If we want to disallow the quota support for older buckets, one idea to do at code level.
   > 
   > User using old Ozone version may still has the desire to use the new Quota feature. It's better not shut the door completely.
   > @adoroszlai , I'm not sure if it's feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user? So that user has the choice either tolerate the inaccurate bytesInUsed or a relative longer upgrade time to get accurate bytesInUsed of buckets.
   
   Hi @ChenSammi, I think it's dangerous recalculating usedBytes in upgrade . If the amount of data is too large, it will greatly increase the time, and if the processing is not good, it will cause the upgrade to fail.
   If we want use Quota in old buckets, we can add a command to recalculate bucket usedBytes to minimize the impact.


----------------------------------------------------------------
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.

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 pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-744841055


   Currently, Unsigned field can't have negative default value in proto. If we set [default = -1], an error will be compiled.
   For now, we use getQuotaValue(long quota) to handle the default case in RpcClient..
   
   Might be use int64, but protobuf document said it is inefficient for negative numbers, not sure it is good idea. Just thought of bringing this up here?
   
   int64 | Uses variable-length encoding. Inefficient for encoding negative numbers – if your field is likely to have negative values, use sint64 instead. | int64 | long | int/long[3] | *int64
   -- | -- | -- | -- | -- | --
   
   I think it will also help if we plan to support quota on older buckets.
   
   The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if one some sets quota on older buckets in this way, we can figure it out.
   
   


----------------------------------------------------------------
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.

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] captainzmc commented on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-746119708


   > So, now if the volume quota is not set, the bucket Clearspace quota is just disabling quota on the buckets.
   > 
   > And also just setting quota at volume level will not be counted, until bucket level quotas are set.
   > 
   > For my understanding so volume quota is set, and bucket quota is set, clearing volume quota is just set to -1, so that now quota is tracked at bucket level. And now when bucket quota is cleared, when volume quota is set, we disallow the operation.
   > This will be the behavior of clear space quota, let me know if i am missing something.
   
   Yes. 
   And, the problem of clear quota will be fixed in HDDS-4588.


----------------------------------------------------------------
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.

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] captainzmc edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-743944634


   > Not related to this PR, can you also provide info on how the clear space quota works and it's usage.
   > 
   > Because I see when bucket clear space quota comes, we reset to -1, but what will happen to volume quota (Will it reclaim bucket quota? or any information on clear space quota usage and how it works also can you share some info.
   
   Yes, I have submitted the use of the latest quota in Doc. Can be seen here:
   https://ci-hadoop.apache.org/view/Hadoop%20Ozone/job/ozone-doc-master/lastSuccessfulBuild/artifact/hadoop-hdds/docs/public/feature/quota.html
   Bucket quota can be set separately without enabling Volume quota. So if volume's quota is cleared, his bucket will not be affected. However, if the volume quota is set to another value, it cannot be less than the quota size 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.

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 edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-744772894


   > https://ci-hadoop.apache.org/view/Hadoop%20Ozone/job/ozone-doc-master/lastSuccessfulBuild/artifact/hadoop->hdds/docs/public/feature/quota.html
   
   >1. When clear volume quota(Assuming volume quota was set before), it is just like setting the quota on the volume to -1, >when clear volume quota is called?
   >2. When volume quota is set, bucket clear quota, it is like just setting the quota for that bucket to -1, and it will allow new >buckets can be created if volume quota is still available(As previous bucket quota cleared quota)
   
   When volume quota is not set and bucket quota is set, it is just reset quota of the bucket to -1. when clear bucket quota operation is performed
   
   Is my understanding correct here? 
   
   Example:
   V1 - 100 MB
   V1/B1 - 50MB
   V1/B2- 50MB
   
   
   Now clear quota on V1/B2 we set quota of V1/B2 to -1, and now a new bucket can be still created in this volume? 
   
   New Bucket created with quota 50MB. (V1/B3)
   
   So, in this volume there are 3 buckets, only 2 are considered in to count, as clearQuota is run(on V1/B2), and we are not considering this bucket in quota calculations, so this bucket will not be counted under quota, but for volume we have crossed the quota of that volume. (As user has run clear quota on V1/B2)
   
   So, here clear quota means resetting the quota to -1? Or what is the real purpose of this clearQuota usage on clusters, in what scenarios this will be useful on the cluster?
   
   
   
   >For old buckets, we do not recommend that quota be enabled, but in order not to affect the write, I handle the default at >checkBucketQuotaInBytes in this PR.
   
   But I don't see any guards in the cluster to not allow quota operations for older buckets/volumes.
   
   
   


----------------------------------------------------------------
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.

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 pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 commented on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-750353909


   Thank you @captainzmc for the contribution and everyone 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.

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 edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
bharatviswa504 edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-750353909


   Thank you @captainzmc for the contribution and everyone for the review.
   
   if any one has more comments, we can open a new Jira to fix them.


----------------------------------------------------------------
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.

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] ChenSammi edited a comment on pull request #1677: HDDS-4562. Old bucket needs to be accessible after the cluster was upgraded to the Quota version.

Posted by GitBox <gi...@apache.org>.
ChenSammi edited a comment on pull request #1677:
URL: https://github.com/apache/ozone/pull/1677#issuecomment-745806878


   > > The default will be -2, all older buckets will have -2, new buckets created after this feature will have -1. So, if some one sets >quota on older buckets in this way, we can figure it out, just an idea saying here.
   > 
   > If we want to disallow the quota support for older buckets, one idea to do at code level.
   
   User using old Ozone version may still has the desire to use the new Quota feature.  It's better not shut the door completely. 
   @adoroszlai ,  I'm not sure if it's feasible that we provide a option of calculating the bytesInUsed for each bucket during 1.0.0 -> 1.1. upgrade to user?   So that user has the choice either olerate the inaccurate bytesInUsed or a relative longer upgrade time to get accurate bytesInUsed of 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.

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