jojochuang commented on code in PR #11118:
URL: https://github.com/apache/ozone/pull/11118#discussion_r4051488016


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java:
##########
@@ -90,41 +93,38 @@ public OMClientResponse validateAndUpdateCache(OzoneManager 
ozoneManager, Execut
 
     List<OzoneManagerProtocolProtos.PurgePathRequest> purgeRequests =
         purgeDirsRequest.getDeletedPathList();
-    Map<Pair<String, String>, OmBucketInfo> volBucketInfoMap = new HashMap<>();
     OmMetadataManagerImpl omMetadataManager = (OmMetadataManagerImpl) 
ozoneManager.getMetadataManager();
-    Map<String, OmKeyInfo> openKeyInfoMap = new HashMap<>();
     OMMetrics omMetrics = ozoneManager.getMetrics();
     DeletingServiceMetrics deletingServiceMetrics = 
ozoneManager.getDeletionMetrics();
     OMResponse.Builder omResponse = OmResponseUtil.getOMResponseBuilder(
         getOmRequest());
     final SnapshotInfo fromSnapshotInfo;
 
-    Set<String> subDirNames = new HashSet<>();
-    Set<String> subFileNames = new HashSet<>();
-    Set<String> deletedDirNames = new HashSet<>();
-
     try {
-      fromSnapshotInfo = fromSnapshot != null ? 
SnapshotUtils.getSnapshotInfo(ozoneManager,
-          fromSnapshot) : null;
-      // Checking if this request is an old request or new one.
-      if (purgeDirsRequest.hasExpectedPreviousSnapshotID()) {
-        // Validating previous snapshot since while purging deletes, a 
snapshot create request could make this purge
-        // directory request invalid on AOS since the deletedDirectory would 
be in the newly created snapshot. Adding
-        // subdirectories could lead to not being able to reclaim sub-files 
and subdirectories since the
-        // file/directory would be present in the newly created snapshot.
-        // Validating previous snapshot can ensure the chain hasn't changed.
-        UUID expectedPreviousSnapshotId = 
purgeDirsRequest.getExpectedPreviousSnapshotID().hasUuid()
-            ? 
fromProtobuf(purgeDirsRequest.getExpectedPreviousSnapshotID().getUuid()) : null;
-        validatePreviousSnapshotId(fromSnapshotInfo, 
omMetadataManager.getSnapshotChainManager(),
-            expectedPreviousSnapshotId);
-      }
+      fromSnapshotInfo = resolveFromSnapshotInfo(ozoneManager, 
omMetadataManager, purgeDirsRequest, fromSnapshot);
     } catch (IOException e) {
-      LOG.error("Error occurred while performing OMDirectoriesPurge. ", e);
-      if (LOG.isDebugEnabled()) {
-        
AUDIT.logWriteFailure(ozoneManager.buildAuditMessageForFailure(OMSystemAction.DIRECTORY_DELETION,
 null, e));
+      return directoryPurgeFailure(ozoneManager, omResponse, e);
+    }
+    // Phase 1 (no lock): parse every purge entry and precompute its delete 
key, path key, replicated size and any
+    // hsync open-key name. None of this depends on the bucket write lock, so 
doing it up front keeps the string and
+    // protobuf work out of the critical section and shortens the write-lock 
hold.
+    Map<VolumeBucketId, BucketNameInfo> volumeBucketIdMap = 
purgeDirsRequest.getBucketNameInfosList().stream()
+        .collect(Collectors.toMap(bucketNameInfo ->

Review Comment:
   the VolumeBucketId should be the same for all items. The only exception is 
during upgrade where PurgeRequest prior to the upgrade may still contain items 
from different VolumeBucketId.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to