[GitHub] [hadoop] jojochuang commented on a change in pull request #3885: HDFS-16214. Lock optimization for deleteing, no locks on the collection block

[GitBox <gi...@apache.org>]

jojochuang commented on a change in pull request #3885:
URL: https://github.com/apache/hadoop/pull/3885#discussion_r801406046



##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java
##########
@@ -88,19 +88,17 @@ static long delete(FSDirectory fsd, INodesInPath iip,
    * @param recursive boolean true to apply to all sub-directories recursively
    * @param logRetryCache whether to record RPC ids in editlog for retry cache
    *          rebuilding
-   * @return blocks collected from the deleted path
    * @throws IOException
    */
-  static BlocksMapUpdateInfo delete(
+  static boolean delete(
       FSNamesystem fsn, FSPermissionChecker pc, String src, boolean recursive,
-      boolean logRetryCache) throws IOException {
+      boolean logRetryCache, INodesInPath iip) throws IOException {

Review comment:
       please update the javadoc for this new parameter

##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/server/namenode/FSDirDeleteOp.java
##########
@@ -112,10 +110,76 @@ static BlocksMapUpdateInfo delete(
       }
       DFSUtil.checkProtectedDescendants(fsd, iip);
     }
-
     return deleteInternal(fsn, iip, logRetryCache);
   }
 
+  /**
+   * Clean files and update Quota after collecting blcoks.
+   * <p>
+   * When it takes a long time to collect blocks when encountering
+   * a large directory, the collection block here adopts no lock。
+   * <p>
+   *
+   * @param fsn namespace
+   * @param iip inodes of a path to be deleted
+   * @return blocks collected from the deleted path
+   * @throws IOException
+   */
+  static BlocksMapUpdateInfo clearFileAndUpdateQuota(FSNamesystem fsn, INodesInPath iip)
+      throws QuotaExceededException {
+    long filesRemoved = -1;
+    if (iip == null) {
+      return null;
+    }
+    // collect blocks no lock
+    INode.ReclaimContext context = FSDirDeleteOp.collectBlocks(fsn, iip);
+    filesRemoved = context.quotaDelta().getNsDelta();
+    if (filesRemoved < 0) {
+      return context.collectedBlocks;
+    }
+    // update Quota and clear UCFiles lease
+    fsn.writeLock();
+    try {
+      FSDirectory fsd = fsn.getFSDirectory();
+      fsd.removeInodeFromDeletingInodes(iip.getLastINode());
+      fsn.removeLeasesAndINodes(context.removedUCFiles,
+          context.removedINodes, true);
+      fsd.updateCount(iip, context.quotaDelta(), false);
+      incrDeletedFileCount(filesRemoved);
+      fsd.updateReplicationFactor(context.collectedBlocks().toUpdateReplicationInfo());
+    } finally {
+      fsn.writeUnlock();
+    }
+    return context.collectedBlocks;
+  }
+
+  /**
+   * Collect blocks from the deleted path.
+   *
+   * @param fsn namespace
+   * @param iip inodes of a path to be deleted
+   * @return blocks collected from the deleted path
+   */
+  static ReclaimContext collectBlocks(FSNamesystem fsn, INodesInPath iip) {

Review comment:
       I doubt this'll work without lock.




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

To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org