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/08/07 02:14:58 UTC

[GitHub] [hadoop-ozone] captainzmc opened a new pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   ## What changes were proposed in this pull request?
   
   In addition, the current Quota setting does not take effect. HDDS-541 gives all the work needed to perfect Quota.
   This PR is a subtask of HDDS-541. 
   First, we increase quotaUsageInBytes of volume. Later, we will judge whether the volume can be written based on this when we write the key.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-4053
   
   ## How was this patch tested?
   
   UT is 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc closed pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
captainzmc closed pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296


   


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   @ChenSammi CC


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       I think that's good advice, and that's a problem that also exists in OzoneBucket. We can optimize these problems separately, I opened a JIRA HDDS-4223 to follow and optimize this part.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296#discussion_r485051300



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       NIT: Can we add a builder to avoid managing constructors with different parameters?




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   Had updated PR to fix review issues and resolve conflict. @ChenSammi Could you help take another look?


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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



##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java
##########
@@ -46,6 +47,7 @@
   private long quotaInBytes;
   private long quotaInCounts;
   private final OmOzoneAclMap aclMap;
+  private LongAdder quotaUsageInBytes = new LongAdder();

Review comment:
       QuotaUsageInBytes is a property of Volume that needs to be updated each time when CreateKey, AllocateBlock, CommitKey, DeleteKey, and the lock of Volume is usually used. 
   Previously, only Bucket locks were used for these operation, and Volume locks can greatly affect the concurrency performance of different buckets. 
   So to avoid Volume locking for performance, LongAdder is used here to complete the atomic update of quotaUsageInBytes.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   hi @bharatviswa504 @arp7 Could you help review the PR about 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   hi @bharatviswa504 @xiaoyuyao Could you help review the PR about 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   Thanks for @cxorm @xiaoyuyao to the review.  The new commit has been updated. Could you help take another look?


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] cxorm commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
cxorm commented on pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296#issuecomment-687539338


   Hi @captainzmc , 
   Could you be so kind to rebase this PR ?
   
   I would take a look on 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] ChenSammi commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   Thanks @captainzmc  for the contribution and @cxorm @xiaoyuyao for review the patch. 


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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



##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java
##########
@@ -46,6 +47,7 @@
   private long quotaInBytes;
   private long quotaInCounts;
   private final OmOzoneAclMap aclMap;
+  private LongAdder quotaUsageInBytes = new LongAdder();

Review comment:
       QuotaUsageInBytes is a property of Volume that needs to be updated each time when CreateKey, AllocateBlock, CommitKey, DeleteKey. and at the beginning, we used the volume lock. 
   But, Previously, only Bucket locks were used for these operation, and use volume lock greatly affect the concurrency performance of different buckets under same volume. 
   So to avoid Volume locking for poor performance, LongAdder is used here to complete the atomic update of quotaUsageInBytes.
   I did a performance test with freon. Multi-threading wrote different buckets under the same volume, and using LongAdder had little impact on the original performance.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296#discussion_r486775263



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc edited a comment on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   > Thanks @captainzmc for working on this. The patch LGTM, +1. Only concern I have is the cost of volume usage update per key write/delete. If you can post the perf diff with/wo update using freon, that will be great.
   
   Thanks for @xiaoyuyao 's review. Here are the freon test results:
   Using 10-100 threads to write data to different buckets under the same volume, adding quota Usage has little impact on performance. (LongAdder disperses the concurrency of a single cell to each cell, improving concurrency efficiency compared to atomicLong).  In my test, I used three virtual machines, each with a key size of 10K.
   ![image](https://user-images.githubusercontent.com/13825159/93158333-7c14ea80-f73e-11ea-86a0-8db45f5fc3e0.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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   hi @bharatviswa504 @arp7 @xiaoyuyao Could you help review the PR about 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] cxorm commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
cxorm commented on a change in pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296#discussion_r484607389



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
##########
@@ -269,6 +269,7 @@ private OzoneConsts() {
   public static final String KEY = "key";
   public static final String SRC_KEY = "srcKey";
   public static final String DST_KEY = "dstKey";
+  public static final String QUOTA_USAGE_IN_BYTES = "quotaUsageInBytes";

Review comment:
       How do we use `QUOTA_USAGE = "quotaUsage"` here ?
   
   Could you please update the rest part if you agree the idea ?

##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java
##########
@@ -46,6 +47,7 @@
   private long quotaInBytes;
   private long quotaInCounts;
   private final OmOzoneAclMap aclMap;
+  private LongAdder quotaUsageInBytes = new LongAdder();

Review comment:
       I'm fine with the `LongAdder` here.
   
   Out of curiosity,
   Could you be so kind to tell me why we use it here ? 
   (Or please describe the bad pattern if we use just `long` or `Long`) 

##########
File path: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
##########
@@ -360,6 +360,8 @@ message VolumeInfo {
     optional uint64 updateID = 9;
     optional uint64 modificationTime = 10;
     optional uint64 quotaInCounts = 11;
+    optional uint64 quotaUsageInBytes = 12;
+

Review comment:
       ```suggestion
       optional uint64 quotaUsage = 12;
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##########
@@ -222,6 +218,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
 
       acquireLock = omMetadataManager.getLock().acquireWriteLock(BUCKET_LOCK,
           volumeName, bucketName);
+

Review comment:
       ```suggestion
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -143,14 +143,25 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
               keyName)),
           new CacheValue<>(Optional.absent(), trxnLogIndex));
 
+      long totalKeySize = 0;

Review comment:
       ```suggestion
         long quotaReleased = 0;
   ```
   
   We could name the variable in a proper meaning IMHO,
   feel free to rename it if you have an idea.

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
##########
@@ -151,21 +154,32 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
         }
       }
 
+      long totalKeySize = 0;

Review comment:
       ```suggestion
         long quotaReleased = 0;
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMFileCreateRequest.java
##########
@@ -292,14 +289,21 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           bucketName, Optional.absent(), Optional.of(missingParentInfos),
           trxnLogIndex);
 
+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      long updateNum = newLocationList.size() * scmBlockSize

Review comment:
       ```suggestion
         long preAllocatedSpace = newLocationList.size() * scmBlockSize
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequest.java
##########
@@ -143,14 +143,25 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
               keyName)),
           new CacheValue<>(Optional.absent(), trxnLogIndex));
 
+      long totalKeySize = 0;
+      int keyFactor = omKeyInfo.getFactor().getNumber();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      OmKeyLocationInfoGroup keyLocationGroup =
+          omKeyInfo.getLatestVersionLocations();
+      for(OmKeyLocationInfo locationInfo: keyLocationGroup.getLocationList()){
+        totalKeySize += locationInfo.getLength() * keyFactor;
+      }
+      // atom update quotaUsage.

Review comment:
       ```suggestion
         // update quotaUsage atomically.
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeysDeleteRequest.java
##########
@@ -151,21 +154,32 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
         }
       }
 
+      long totalKeySize = 0;
+      OmVolumeArgs omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+
       // Mark all keys which can be deleted, in cache as deleted.
       for (OmKeyInfo omKeyInfo : omKeyInfoList) {
         omMetadataManager.getKeyTable().addCacheEntry(
             new CacheKey<>(omMetadataManager.getOzoneKey(volumeName, bucketName,
                 omKeyInfo.getKeyName())),
             new CacheValue<>(Optional.absent(), trxnLogIndex));
+
+        int keyFactor = omKeyInfo.getFactor().getNumber();
+        OmKeyLocationInfoGroup keyLocationGroup =
+            omKeyInfo.getLatestVersionLocations();
+        for(OmKeyLocationInfo locationInfo: keyLocationGroup.getLocationList()){
+          totalKeySize += locationInfo.getLength() * keyFactor;
+        }
       }
+      // atom update quotaUsage.

Review comment:
       ```suggestion
         // update quotaUsage atomically.
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##########
@@ -295,14 +293,28 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           new CacheKey<>(dbOpenKeyName),
           new CacheValue<>(Optional.of(omKeyInfo), trxnLogIndex));
 
+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+
+      // Here we refer to the implementation of HDFS:
+      // If the key size is 600MB, when createKey, keyLocationList is 3, and
+      // the every pre-allocated block length is 256MB. If the number of factor
+      // is 3, the total pre-allocated block size is 256MB*3*3.
+      // We will allocate more 256MB*3* 3-600mb *3 = 504MB in advance, and we
+      // will subtract this part when we finally commitKey.

Review comment:
       ```suggestion
         // Here we refer to the implementation of HDFS:
         // If the key size is 600MB, when createKey, keyLocationInfo in keyLocationList is 3, and
         // the every pre-allocated block length is 256MB. If the number of factor
         // is 3, the total pre-allocated block size is 256MB * 3 * 3.
         // We will allocate more 256MB * 3 * 3 - 600MB * 3 = 504MB in advance, and we
         // will subtract this part when we finally commitKey.
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java
##########
@@ -295,14 +293,28 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           new CacheKey<>(dbOpenKeyName),
           new CacheValue<>(Optional.of(omKeyInfo), trxnLogIndex));
 
+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+
+      // Here we refer to the implementation of HDFS:
+      // If the key size is 600MB, when createKey, keyLocationList is 3, and
+      // the every pre-allocated block length is 256MB. If the number of factor
+      // is 3, the total pre-allocated block size is 256MB*3*3.
+      // We will allocate more 256MB*3* 3-600mb *3 = 504MB in advance, and we
+      // will subtract this part when we finally commitKey.
+      long updateNum = newLocationList.size() * scmBlockSize

Review comment:
       ```suggestion
         long preAllocatedSpace = newLocationList.size() * scmBlockSize
   ```

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")
+  public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
+      String name, String admin, String owner, long quotaInBytes,
+      long quotaInCounts, long creationTime, long modificationTime,
+      List<OzoneAcl> acls, Map<String, String> metadata,
+      long quotaUsageInBytes) {
+    this(conf, proxy, name, admin, owner, quotaInBytes, quotaInCounts,
+        creationTime, acls, metadata);
+    this.modificationTime = Instant.ofEpochMilli(modificationTime);
+    this.quotaUsageInBytes = quotaUsageInBytes;

Review comment:
       ```suggestion
       this(conf, proxy, name, admin, owner, quotaInBytes, quotaInCounts,
           creationTime, modificationTime, acls, metadata);
       this.quotaUsageInBytes = quotaUsageInBytes;
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -182,8 +179,16 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           new CacheKey<>(dbOzoneKey),
           new CacheValue<>(Optional.of(omKeyInfo), trxnLogIndex));
 
+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      int factor = omKeyInfo.getFactor().getNumber();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      long updateNum = omKeyInfo.getDataSize() * factor -
+          locationInfoList.size() * scmBlockSize * factor;

Review comment:
       I'm confused about the meaning of the variable. 
   (Please let me know the meaning of the variable if I miss something.)
   
   Could you please add some comments about it ?

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##########
@@ -210,10 +206,17 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           new CacheKey<>(openKeyName),
           new CacheValue<>(Optional.of(openKeyInfo), trxnLogIndex));
 
+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      long updateNum = newLocationList.size() * scmBlockSize

Review comment:
       ```suggestion
         long preAllocatedSpace = newLocationList.size() * scmBlockSize
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java
##########
@@ -207,13 +209,21 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           new CacheKey<>(openKey),
           new CacheValue<>(Optional.absent(), trxnLogIndex));
 
+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      int factor = omKeyInfo.getFactor().getNumber();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      long updateNum = omKeyInfo.getDataSize() * factor -
+          keyArgs.getKeyLocationsList().size() * scmBlockSize * factor;

Review comment:
       The same as `OMKeyCommitRequest`

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMAllocateBlockRequest.java
##########
@@ -210,10 +206,17 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           new CacheKey<>(openKeyName),
           new CacheValue<>(Optional.of(openKeyInfo), trxnLogIndex));
 
+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      long updateNum = newLocationList.size() * scmBlockSize
+          * openKeyInfo.getFactor().getNumber();
+      // atom update quotaUsage.

Review comment:
       ```suggestion
         // update quotaUsage atomically.
   ```

##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeysDeleteResponse.java
##########
@@ -105,5 +108,10 @@ public void addToDBBatch(OMMetadataManager omMetadataManager,
       omMetadataManager.getDeletedTable().putWithBatch(batchOperation,
           deleteKey, repeatedOmKeyInfo);
     }
+
+    // update volume quotaUsageInBytes.
+    omMetadataManager.getVolumeTable().putWithBatch(batchOperation,
+        omMetadataManager.getVolumeKey(omVolumeArgs.getVolume()),
+        omVolumeArgs);
   }
 }

Review comment:
       ```suggestion
   }
   ```




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   > Thanks @captainzmc for working on this. The patch LGTM, +1. Only concern I have is the cost of volume usage update per key write/delete. If you can post the perf diff with/wo update using freon, that will be great.
   
   Thanks for @xiaoyuyao 's +1. Here are the freon test results:
   Using 10-100 threads to write data to different buckets under the same volume, adding quota Usage has little impact on performance. (LongAdder disperses the concurrency of a single cell to each cell, improving concurrency efficiency compared to atomicLong).  In my test, I used three virtual machines, each with a key size of 10K.
   ![image](https://user-images.githubusercontent.com/13825159/93158333-7c14ea80-f73e-11ea-86a0-8db45f5fc3e0.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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   hi @bharatviswa504 @arp7 Could you help review the PR about 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296#discussion_r486775263



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] ChenSammi commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
ChenSammi commented on a change in pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296#discussion_r477015608



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -79,6 +79,8 @@
 
   private int listCacheSize;
 
+  private long quotaUsageInBytes;

Review comment:
       quotaUsageInBytes -> usedBytes 

##########
File path: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
##########
@@ -353,6 +353,7 @@ message VolumeInfo {
     optional uint64 objectID = 8;
     optional uint64 updateID = 9;
     optional uint64 modificationTime = 10;
+    optional uint64 quotaUsageInBytes = 11;

Review comment:
       quotaUsageInBytes -> usedBytes

##########
File path: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/client/rpc/TestOzoneRpcClientAbstract.java
##########
@@ -703,6 +707,250 @@ public void testPutKey() throws IOException {
     }
   }
 
+  @Test
+  public void testVolumeQuotaUsageInBytes() throws IOException {
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+    OzoneVolume volume = null;
+
+    int blockSize = (int) ozoneManager.getConfiguration().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);
+
+    // Write data larger than one block size.
+    String value = generateData(blockSize + 100,
+        (byte) RandomUtils.nextLong()).toString();
+
+    int valueLength = value.getBytes().length;
+    long currentQuotaUsage = 0L;
+    store.createVolume(volumeName);
+    volume = store.getVolume(volumeName);
+    // The initial value should be 0
+    Assert.assertEquals(0L, volume.getQuotaUsageInBytes());
+    volume.createBucket(bucketName);
+    OzoneBucket bucket = volume.getBucket(bucketName);
+
+    //Case1: Test the volumeQuotaUsageInBytes of ONE replications.
+    String keyName1 = UUID.randomUUID().toString();
+    writeKey(bucket, keyName1, ONE, value, valueLength);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(valueLength, volume.getQuotaUsageInBytes());
+    currentQuotaUsage += valueLength;
+
+    // Case2: Test overwrite the same KeyName under ONE Replicates, the
+    // keyLocationVersions of the Key is 2.
+    String keyName2 = UUID.randomUUID().toString();
+    writeKey(bucket, keyName2, ONE, value, valueLength);
+    // Overwrite the keyName2
+    writeKey(bucket, keyName2, ONE, value, valueLength);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(valueLength * 2 + currentQuotaUsage,
+        volume.getQuotaUsageInBytes());
+    currentQuotaUsage += valueLength * 2;
+
+    // Case3: Test the volumeQuotaUsageInBytes of THREE replications.
+    String keyName3 = UUID.randomUUID().toString();
+    writeKey(bucket, keyName3, THREE, value, valueLength);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(valueLength * 3 + currentQuotaUsage,
+        volume.getQuotaUsageInBytes());
+    currentQuotaUsage += valueLength * 3;
+
+    // Case4: Test overwrite the same KeyName under THREE Replicates, the
+    // keyLocationVersions of the Key is 2.
+    String keyName4 = UUID.randomUUID().toString();
+    writeKey(bucket, keyName4, THREE, value, valueLength);
+    // Overwrite the keyName4
+    writeKey(bucket, keyName4, THREE, value, valueLength);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(valueLength * 3 * 2 + currentQuotaUsage,
+        volume.getQuotaUsageInBytes());
+    currentQuotaUsage += valueLength * 3 * 2;
+
+    //Case5: Do not specify the value Length, simulate HDFS api writing.
+    // Test the volumeQuotaUsageInBytes of ONE replications.
+    String keyName5 = UUID.randomUUID().toString();
+    writeFile(bucket, keyName5, ONE, value, 0);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(valueLength + currentQuotaUsage,
+        volume.getQuotaUsageInBytes());
+    currentQuotaUsage += valueLength;
+
+    // Case6: Do not specify the value Length, simulate HDFS api writing.
+    // Test overwrite the same KeyName under ONE Replicates, the
+    // keyLocationVersions of the Key is 2.
+    String keyName6 = UUID.randomUUID().toString();
+    writeFile(bucket, keyName6, ONE, value, 0);
+    // Overwrite the keyName6
+    writeFile(bucket, keyName6, ONE, value, 0);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(valueLength * 2 + currentQuotaUsage,
+        volume.getQuotaUsageInBytes());
+    currentQuotaUsage += valueLength * 2;
+
+    // Case7: Do not specify the value Length, simulate HDFS api writing.
+    // Test the volumeQuotaUsageInBytes of THREE replications.
+    String keyName7 = UUID.randomUUID().toString();
+    writeFile(bucket, keyName7, THREE, value, 0);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(valueLength * 3 + currentQuotaUsage,
+        volume.getQuotaUsageInBytes());
+    currentQuotaUsage += valueLength * 3;
+
+    // Case8: Do not specify the value Length, simulate HDFS api writing.
+    // Test overwrite the same KeyName under THREE Replicates, the
+    // keyLocationVersions of the Key is 2.
+    String keyName8 = UUID.randomUUID().toString();
+    writeFile(bucket, keyName8, THREE, value, 0);
+    // Overwrite the keyName8
+    writeFile(bucket, keyName8, THREE, value, 0);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(valueLength * 3 * 2 + currentQuotaUsage,
+        volume.getQuotaUsageInBytes());
+    currentQuotaUsage += valueLength * 3 * 2;
+
+    // Case9: Test volumeQuotaUsageInBytes when delete key of ONE replications.
+    bucket.deleteKey(keyName1);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(currentQuotaUsage - valueLength,
+        volume.getQuotaUsageInBytes());
+    currentQuotaUsage -= valueLength;
+
+    // Case10: Test volumeQuotaUsageInBytes when delete key of THREE
+    // replications.
+    bucket.deleteKey(keyName3);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(currentQuotaUsage - valueLength * 3,
+        volume.getQuotaUsageInBytes());
+    currentQuotaUsage -= valueLength * 3;
+
+    // Case11: Test volumeQuotaUsageInBytes when Test Delete keys. At this
+    // point all keys are deleted, volumeQuotaUsageInBytes should be 0
+    List<String> keyList = new ArrayList<>();
+    keyList.add(keyName2);
+    keyList.add(keyName4);
+    keyList.add(keyName5);
+    keyList.add(keyName6);
+    keyList.add(keyName7);
+    keyList.add(keyName8);
+    bucket.deleteKeys(keyList);
+    volume = store.getVolume(volumeName);
+    Assert.assertEquals(0, volume.getQuotaUsageInBytes());
+  }
+
+  @Test
+  public void testVolumeQuotaWithMultiThread() throws IOException,
+      InterruptedException{
+    String volumeName = UUID.randomUUID().toString();
+
+    int blockSize = (int) ozoneManager.getConfiguration().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);
+    // Write data larger than one block size.
+    String value = generateData(blockSize + 100,
+        (byte) RandomUtils.nextLong()).toString();
+
+    int valueLength = value.getBytes().length;
+    long currentQuotaUsage = 0L;
+    store.createVolume(volumeName);
+    OzoneVolume volume = store.getVolume(volumeName);
+    // The initial value should be 0
+    Assert.assertEquals(0L, volume.getQuotaUsageInBytes());
+
+    CountDownLatch latch = new CountDownLatch(2);
+    AtomicInteger failCount = new AtomicInteger(0);
+
+    // Multiple threads write different buckets and ensure that the volume
+    // quota is correct.
+    Runnable r = () -> {
+      try {
+        for (int i = 0; i < 10; i++) {
+          String keyName = UUID.randomUUID().toString();
+          String bucketName = UUID.randomUUID().toString();
+          volume.createBucket(bucketName);
+          OzoneBucket bucket = volume.getBucket(bucketName);
+          OzoneOutputStream out = bucket.createKey(keyName, valueLength,
+              STAND_ALONE, ONE, new HashMap<>());
+          out.write(value.getBytes());
+          out.close();
+        }
+        latch.countDown();
+      } catch (IOException ex) {
+        latch.countDown();
+        failCount.incrementAndGet();
+      }
+    };
+
+    Thread thread1 = new Thread(r);
+    Thread thread2 = new Thread(r);
+
+    thread1.start();
+    thread2.start();
+
+    latch.await(6000, TimeUnit.SECONDS);
+
+    if (failCount.get() > 0) {
+      fail("testVolumeQuotaWithMultiThread failed");
+    }
+    currentQuotaUsage += valueLength * 10 * 2;
+    Assert.assertEquals(currentQuotaUsage,
+        store.getVolume(volumeName).getQuotaUsageInBytes());
+
+  }
+
+  private void writeKey(OzoneBucket bucket, String keyName,
+      ReplicationFactor replication, String value, int valueLength)
+      throws IOException{
+    OzoneOutputStream out = bucket.createKey(keyName, valueLength, STAND_ALONE,
+        replication, new HashMap<>());
+    out.write(value.getBytes());
+    out.close();
+  }
+
+  private void writeFile(OzoneBucket bucket, String keyName,
+      ReplicationFactor replication, String value, int valueLength)
+      throws IOException{
+    OzoneOutputStream out = bucket.createFile(keyName, valueLength, STAND_ALONE,
+        replication, true, true);
+    out.write(value.getBytes());
+    out.close();
+  }
+
+  @Test
+  public void testVolumeQuotaWithUploadPart() throws IOException {
+    String volumeName = UUID.randomUUID().toString();
+    String bucketName = UUID.randomUUID().toString();
+    String keyName = UUID.randomUUID().toString();
+    int blockSize = (int) ozoneManager.getConfiguration().getStorageSize(
+        OZONE_SCM_BLOCK_SIZE, OZONE_SCM_BLOCK_SIZE_DEFAULT, StorageUnit.BYTES);
+    String sampleData = generateData(blockSize + 100,
+        (byte) RandomUtils.nextLong()).toString();
+    int valueLength = sampleData.getBytes().length;
+
+    store.createVolume(volumeName);
+    OzoneVolume volume = store.getVolume(volumeName);
+    // The initial value should be 0
+    Assert.assertEquals(0L, volume.getQuotaUsageInBytes());
+    volume.createBucket(bucketName);
+    OzoneBucket bucket = volume.getBucket(bucketName);
+    OmMultipartInfo multipartInfo = bucket.initiateMultipartUpload(keyName,
+        STAND_ALONE, ONE);
+
+    assertNotNull(multipartInfo);
+    String uploadID = multipartInfo.getUploadID();
+    Assert.assertEquals(volumeName, multipartInfo.getVolumeName());
+    Assert.assertEquals(bucketName, multipartInfo.getBucketName());
+    Assert.assertEquals(keyName, multipartInfo.getKeyName());
+    assertNotNull(multipartInfo.getUploadID());
+
+    OzoneOutputStream ozoneOutputStream = bucket.createMultipartKey(keyName,
+        sampleData.length(), 1, uploadID);

Review comment:
       Can we upload more parts?

##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java
##########
@@ -386,7 +408,7 @@ public OmVolumeArgs copyObject() {
     OmOzoneAclMap cloneAclMap = aclMap.copyObject();
 
     return new OmVolumeArgs(adminName, ownerName, volume, quotaInBytes,
-        quotaInCounts, cloneMetadata, cloneAclMap, creationTime,
-        modificationTime, objectID, updateID);
+        quotaInCounts, cloneMetadata, quotaUsageInBytes.sum(), cloneAclMap,
+        creationTime, modificationTime, objectID, updateID);
   }
 }

Review comment:
       Should getObjectInfo also be changed?

##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java
##########
@@ -46,6 +47,7 @@
   private long quotaInBytes;
   private long quotaInCounts;
   private final OmOzoneAclMap aclMap;
+  private LongAdder quotaUsageInBytes = new LongAdder();

Review comment:
       add final 

##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java
##########
@@ -172,6 +176,14 @@ public long getQuotaInCounts() {
   public OmOzoneAclMap getAclMap() {
     return aclMap;
   }
+
+  public LongAdder getQuotaUsageInBytes() {
+    return quotaUsageInBytes;
+  }
+
+  public void setQuotaUsageInBytes(long quotaUsageInBytes) {

Review comment:
       setQuotaUsageInBytes -> increase***




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   hi @bharatviswa504 @xiaoyuyao Could you help review the PR about 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   > Hi @captainzmc ,
   > Could you be so kind to rebase this PR ?
   > 
   > I would take a look on this.
   
   Thanks for yisheng's attention, PR  has been updated. @cxorm Now this PR can be reviewed.  


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   Hi @elek @adoroszlai Could you help review the PR about 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296#discussion_r486775263



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good to me. 

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good to me. 

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good to me. 

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good to me. 

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good to me. 

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good to me. 

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good to me. 

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good to me. 

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneVolume.java
##########
@@ -131,6 +133,18 @@ public OzoneVolume(ConfigurationSource conf, ClientProtocol proxy,
     this.modificationTime = Instant.ofEpochMilli(modificationTime);
   }
 
+  @SuppressWarnings("parameternumber")

Review comment:
       sounds good 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc closed pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
captainzmc closed pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296


   


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   hi @bharatviswa504 @arp7 @xiaoyuyao Could you help review the PR about 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] ChenSammi merged pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

Posted by GitBox <gi...@apache.org>.
ChenSammi merged pull request #1296:
URL: https://github.com/apache/hadoop-ozone/pull/1296


   


----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java
##########
@@ -182,8 +179,16 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
           new CacheKey<>(dbOzoneKey),
           new CacheValue<>(Optional.of(omKeyInfo), trxnLogIndex));
 
+      long scmBlockSize = ozoneManager.getScmBlockSize();
+      int factor = omKeyInfo.getFactor().getNumber();
+      omVolumeArgs = getVolumeInfo(omMetadataManager, volumeName);
+      long updateNum = omKeyInfo.getDataSize() * factor -
+          locationInfoList.size() * scmBlockSize * factor;

Review comment:
       I will add comment to describe the calculation 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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



##########
File path: hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmVolumeArgs.java
##########
@@ -46,6 +47,7 @@
   private long quotaInBytes;
   private long quotaInCounts;
   private final OmOzoneAclMap aclMap;
+  private LongAdder quotaUsageInBytes = new LongAdder();

Review comment:
       QuotaUsageInBytes is a property of Volume that needs to be updated each time when CreateKey, AllocateBlock, CommitKey, DeleteKey, and the lock of Volume is usually used. 
   Previously, only Bucket locks were used for these operation, and Volume locks can greatly affect the concurrency performance of different buckets. 
   So to avoid Volume locking for poor performance, LongAdder is used here to complete the atomic update of quotaUsageInBytes.
   I did a performance test with freon. Multi-threading wrote different buckets under the same volume, and using LongAdder had little impact on the original performance.




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc removed a comment on pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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


   Hi @elek @adoroszlai Could you help review the PR about 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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org

[GitHub] [hadoop-ozone] captainzmc commented on a change in pull request #1296: HDDS-4053. Volume space: add quotaUsageInBytes and update it when write and delete key.

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



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
##########
@@ -269,6 +269,7 @@ private OzoneConsts() {
   public static final String KEY = "key";
   public static final String SRC_KEY = "srcKey";
   public static final String DST_KEY = "dstKey";
+  public static final String QUOTA_USAGE_IN_BYTES = "quotaUsageInBytes";

Review comment:
       Using quotaUsage here may not be appropriate because it makes it impossible to distinguish between QuotaUsageInBytes and later QuotaUsageInCount. 
   Here we can use the usage in ContainerInfo, [use usedBytes](https://github.com/apache/hadoop-ozone/blob/971a36eea16b38257892244d6de862b4f9461138/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerInfo.java#L55).
   




----------------------------------------------------------------
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: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org