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/07 14:37:24 UTC

[GitHub] [ozone] rakeshadr opened a new pull request #1668: HDDS-4321. Revisit quota implementation based on HDDS-4308 task

rakeshadr opened a new pull request #1668:
URL: https://github.com/apache/ozone/pull/1668


   ## What changes were proposed in this pull request?
   
   This is tracking Jira to revisit the quota implementation once HDDS-4308  task is committed.
   
   Need to revisit all the newly created "*RequestV1.java" and "*ResponseV1.java" classes and incorporate HDDS-4308 changes into it.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-4321
   
   ## How was this patch tested?
   
   Existing V1 UT cases.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

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] rakeshadr commented on pull request #1668: Fix compilation errors in HDDS-2939 branch: merge HDDS-4308 and HDDS-4473 changes

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


   Thanks @mukul1987 , @linyiqun , @bharatviswa504 for the reviews. I will merge 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] bharatviswa504 commented on a change in pull request #1668: HDDS-4321. Revisit quota implementation based on HDDS-4308 task

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java
##########
@@ -149,9 +149,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
       omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);
 
       long quotaReleased = sumBlockLengths(omKeyInfo);
-      // update usedBytes atomically.
-      omVolumeArgs.getUsedBytes().add(-quotaReleased);
-      omBucketInfo.getUsedBytes().add(-quotaReleased);
+      omBucketInfo.incrUsedBytes(-quotaReleased);

Review comment:
       SumBlockLengths for OmKeyInfo here when it is directory it will be zero.




----------------------------------------------------------------
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] rakeshadr commented on a change in pull request #1668: HDDS-4321. Revisit quota implementation based on HDDS-4308 task

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java
##########
@@ -149,9 +149,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
       omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);
 
       long quotaReleased = sumBlockLengths(omKeyInfo);
-      // update usedBytes atomically.
-      omVolumeArgs.getUsedBytes().add(-quotaReleased);
-      omBucketInfo.getUsedBytes().add(-quotaReleased);
+      omBucketInfo.incrUsedBytes(-quotaReleased);

Review comment:
       Thanks a lot @bharatviswa504 for reminding this point again. Since this requires more deeper discussion I've created HDDS-4565 jira.  
   
   Like we discussed in offline chat, it helps to unblock other PRs by fixing compilation errors in the branch(it occurred due to the recent rebase). I am changing the jira description as well reflecting the compilation error 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: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] rakeshadr commented on a change in pull request #1668: HDDS-4321. Revisit quota implementation based on HDDS-4308 task

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -2430,20 +2430,19 @@ private void listStatusFindKeyInTableCache(
       metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
               bucketName);
     }
+    List<OmKeyInfo> keyInfoList = new ArrayList<>(fileStatusList.size());
     for (OzoneFileStatus fileStatus : fileStatusList) {
       if (fileStatus.isFile()) {
-        // refreshPipeline flag check has been removed as part of
-        // https://issues.apache.org/jira/browse/HDDS-3658.
-        // Please refer this jira for more details.
-        refreshPipeline(fileStatus.getKeyInfo());
-
-        // No need to check if a key is deleted or not here, this is handled
-        // when adding entries to cacheKeyMap from DB.
-        if (args.getSortDatanodes()) {
-          sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress);
-        }
+        keyInfoList.add(fileStatus.getKeyInfo());
       }
     }
+    // refreshPipeline flag check has been removed as part of
+    // https://issues.apache.org/jira/browse/HDDS-3658.
+    // Please refer this jira for more details.
+    refreshPipeline(keyInfoList);

Review comment:
       Thanks @mukul1987  for the reviews. Yes, you are right. Actually after rebasing the feature branch, there are compilation errors and this is due to HDDS-4473 changes.




----------------------------------------------------------------
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 #1668: Fix compilation errors in HDDS-2939 branch: merge HDDS-4308 and HDDS-4473 changes

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


   As this is blocking further PR's and it is only a compilation issue fix, and anyway, quota will be revisited, I am fine with this PR going in.


----------------------------------------------------------------
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 #1668: Fix compilation errors in HDDS-2939 branch: merge HDDS-4308 and HDDS-4473 changes

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


   As this is blocking further PR's and it is only a compilation issue fix, and anyway, quota will be revisited, I am fine with 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] rakeshadr merged pull request #1668: Fix compilation errors : merge HDDS-4308 and HDDS-4473 changes into the branch

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


   


----------------------------------------------------------------
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 #1668: HDDS-4321. Revisit quota implementation based on HDDS-4308 task

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyDeleteRequestV1.java
##########
@@ -149,9 +149,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
       omBucketInfo = getBucketInfo(omMetadataManager, volumeName, bucketName);
 
       long quotaReleased = sumBlockLengths(omKeyInfo);
-      // update usedBytes atomically.
-      omVolumeArgs.getUsedBytes().add(-quotaReleased);
-      omBucketInfo.getUsedBytes().add(-quotaReleased);
+      omBucketInfo.incrUsedBytes(-quotaReleased);

Review comment:
       https://github.com/apache/ozone/pull/1607#discussion_r529974503
   Pls refer this comment




----------------------------------------------------------------
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] rakeshadr commented on pull request #1668: HDDS-4321. Revisit quota implementation based on HDDS-4308 task

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


   > Besides @mukul1987 's comment, volume quota change in OMFileCreateResponseV1 also needed to update,[OMFileCreateResponseV1.java#L82](https://github.com/apache/ozone/blob/HDDS-2939/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/file/OMFileCreateResponseV1.java#L82)
   
   Thanks @linyiqun for the comment. Will update 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] mukul1987 commented on a change in pull request #1668: HDDS-4321. Revisit quota implementation based on HDDS-4308 task

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



##########
File path: hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
##########
@@ -2430,20 +2430,19 @@ private void listStatusFindKeyInTableCache(
       metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
               bucketName);
     }
+    List<OmKeyInfo> keyInfoList = new ArrayList<>(fileStatusList.size());
     for (OzoneFileStatus fileStatus : fileStatusList) {
       if (fileStatus.isFile()) {
-        // refreshPipeline flag check has been removed as part of
-        // https://issues.apache.org/jira/browse/HDDS-3658.
-        // Please refer this jira for more details.
-        refreshPipeline(fileStatus.getKeyInfo());
-
-        // No need to check if a key is deleted or not here, this is handled
-        // when adding entries to cacheKeyMap from DB.
-        if (args.getSortDatanodes()) {
-          sortDatanodeInPipeline(fileStatus.getKeyInfo(), clientAddress);
-        }
+        keyInfoList.add(fileStatus.getKeyInfo());
       }
     }
+    // refreshPipeline flag check has been removed as part of
+    // https://issues.apache.org/jira/browse/HDDS-3658.
+    // Please refer this jira for more details.
+    refreshPipeline(keyInfoList);

Review comment:
       This change seems unrelated to quota changes. 




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