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 2022/10/25 10:05:56 UTC

[GitHub] [ozone] Xushaohong commented on a diff in pull request #3844: HDDS-7328. Improve Deletion of FSO Paths

Xushaohong commented on code in PR #3844:
URL: https://github.com/apache/ozone/pull/3844#discussion_r1004272779


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/DirectoryDeletingService.java:
##########
@@ -125,73 +134,95 @@ public int getPriority() {
     @Override
     public BackgroundTaskResult call() throws Exception {
       if (shouldRun()) {
+        if (LOG.isDebugEnabled()) {
+          LOG.debug("Running DirectoryDeletingService");
+        }
         runCount.incrementAndGet();
-        long count = pathLimitPerTask;
+        int dirNum = 0;
+        int subDirNum = 0;
+        int subFileNum = 0;
+        long remainNum = pathLimitPerTask;
+        List<PurgePathRequest> purgePathRequestList = new ArrayList<>();
+
+        Table.KeyValue<String, OmKeyInfo> pendingDeletedDirInfo;
         try {
+          TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
+              deleteTableIterator = ozoneManager.getMetadataManager().
+              getDeletedDirTable().iterator();
+
           long startTime = Time.monotonicNow();
-          // step-1) Get one pending deleted directory
-          Table.KeyValue<String, OmKeyInfo> pendingDeletedDirInfo =
-              ozoneManager.getKeyManager().getPendingDeletionDir();
-          if (pendingDeletedDirInfo != null) {
+          while (remainNum > 0 && deleteTableIterator.hasNext()) {
+            pendingDeletedDirInfo = deleteTableIterator.next();
+            // step-0: Get one pending deleted directory
             if (LOG.isDebugEnabled()) {
               LOG.debug("Pending deleted dir name: {}",
                   pendingDeletedDirInfo.getValue().getKeyName());
             }
             final String[] keys = pendingDeletedDirInfo.getKey()
-                    .split(OM_KEY_PREFIX);
+                .split(OM_KEY_PREFIX);
             final long volumeId = Long.parseLong(keys[1]);
             final long bucketId = Long.parseLong(keys[2]);
 
             // step-1: get all sub directories under the deletedDir
-            List<OmKeyInfo> dirs = ozoneManager.getKeyManager()
+            List<OmKeyInfo> subDirs = ozoneManager.getKeyManager()
                 .getPendingDeletionSubDirs(volumeId, bucketId,
-                        pendingDeletedDirInfo.getValue(), count);
-            count = count - dirs.size();
-            List<OmKeyInfo> deletedSubDirList = new ArrayList<>();
-            for (OmKeyInfo dirInfo : dirs) {
-              deletedSubDirList.add(dirInfo);
-              if (LOG.isDebugEnabled()) {
-                LOG.debug("deleted sub dir name: {}",
-                    dirInfo.getKeyName());
+                    pendingDeletedDirInfo.getValue(), remainNum);
+            remainNum = remainNum - subDirs.size();
+
+            if (LOG.isDebugEnabled()) {
+              for (OmKeyInfo dirInfo : subDirs) {
+                LOG.debug("Moved sub dir name: {}", dirInfo.getKeyName());
               }
             }
 
             // step-2: get all sub files under the deletedDir
-            List<OmKeyInfo> purgeDeletedFiles = ozoneManager.getKeyManager()
+            List<OmKeyInfo> subFiles = ozoneManager.getKeyManager()
                 .getPendingDeletionSubFiles(volumeId, bucketId,
-                        pendingDeletedDirInfo.getValue(), count);
-            count = count - purgeDeletedFiles.size();
+                    pendingDeletedDirInfo.getValue(), remainNum);
+            remainNum = remainNum - subFiles.size();
 
             if (LOG.isDebugEnabled()) {
-              for (OmKeyInfo fileInfo : purgeDeletedFiles) {
-                LOG.debug("deleted sub file name: {}", fileInfo.getKeyName());
+              for (OmKeyInfo fileInfo : subFiles) {
+                LOG.debug("Moved sub file name: {}", fileInfo.getKeyName());
               }
             }
 
-            // step-3: Since there is a boundary condition of 'numEntries' in
-            // each batch, check whether the sub paths count reached batch size
-            // limit. If count reached limit then there can be some more child
-            // paths to be visited and will keep the parent deleted directory
-            // for one more pass.
-            final Optional<String> purgeDeletedDir = count > 0 ?
-                    Optional.of(pendingDeletedDirInfo.getKey()) :
-                    Optional.empty();
-
-            if (isRatisEnabled()) {
-              submitPurgePaths(volumeId, bucketId, purgeDeletedDir,
-                      purgeDeletedFiles, deletedSubDirList);
+          // step-3: Since there is a boundary condition of 'numEntries' in
+          // each batch, check whether the sub paths count reached batch size
+          // limit. If count reached limit then there can be some more child
+          // paths to be visited and will keep the parent deleted directory
+          // for one more pass.
+            final Optional<String> purgeDeletedDir = remainNum > 0 ?
+                Optional.of(pendingDeletedDirInfo.getKey()) :
+                Optional.empty();
+
+            PurgePathRequest request = wrapPurgeRequest(volumeId, bucketId,
+                purgeDeletedDir, subFiles, subDirs);
+            purgePathRequestList.add(request);
+
+            // Count up the purgeDeletedDir, subDirs and subFiles
+            if (purgeDeletedDir.isPresent()) {
+              dirNum++;
             }
-            // TODO: need to handle delete with non-ratis
+            subDirNum += subDirs.size();
+            subFileNum += subFiles.size();
+          }
 
-            deletedDirsCount.incrementAndGet();
-            deletedFilesCount.addAndGet(purgeDeletedFiles.size());
-            if (LOG.isDebugEnabled()) {
-              LOG.debug("Number of dirs deleted: {}, Number of files moved:" +
-                      " {} to DeletedTable, elapsed time: {}ms",
-                  deletedDirsCount, deletedFilesCount,
-                  Time.monotonicNow() - startTime);
-            }
+          // TODO: need to handle delete with non-ratis
+          if (isRatisEnabled()) {
+            submitPurgePaths(purgePathRequestList);
           }
+
+          deletedDirsCount.addAndGet(dirNum);
+          movedDirsCount.addAndGet(subDirNum);
+          movedFilesCount.addAndGet(subFileNum);
+          LOG.info("Number of dirs deleted: {}, Number of sub-files moved:" +

Review Comment:
   Make sense! I have updated.



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

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

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


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