prashantpogde commented on code in PR #7200:
URL: https://github.com/apache/ozone/pull/7200#discussion_r1763624427


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java:
##########
@@ -136,316 +134,105 @@ public BackgroundTaskResult call() throws 
InterruptedException {
 
       getRunCount().incrementAndGet();
 
-      ReferenceCounted<OmSnapshot> rcOmSnapshot = null;
-      ReferenceCounted<OmSnapshot> rcOmPreviousSnapshot = null;
-
-      Table<String, SnapshotInfo> snapshotInfoTable =
-          ozoneManager.getMetadataManager().getSnapshotInfoTable();
-      List<String> purgeSnapshotKeys = new ArrayList<>();
-      try (TableIterator<String, ? extends Table.KeyValue
-          <String, SnapshotInfo>> iterator = snapshotInfoTable.iterator()) {
-
+      try {
+        int remaining = keyLimitPerTask;
+        Iterator<UUID> iterator = snapshotChainManager.iterator(true);
+        List<SnapshotInfo> snapshotsToBePurged = new ArrayList<>();
         long snapshotLimit = snapshotDeletionPerTask;
-
         while (iterator.hasNext() && snapshotLimit > 0) {
-          SnapshotInfo snapInfo = iterator.next().getValue();
-
-          // Only Iterate in deleted snapshot
+          SnapshotInfo snapInfo = SnapshotUtils.getSnapshotInfo(ozoneManager, 
snapshotChainManager, iterator.next());
+          // Only Iterate in deleted snapshot & only if all the changes have 
been flushed into disk.
           if (shouldIgnoreSnapshot(snapInfo)) {
             continue;
           }
 
-          // Note: Can refactor this to use try-with-resources.
-          // Handling RC decrements manually for now to minimize conflicts.
-          rcOmSnapshot = omSnapshotManager.getSnapshot(
-              snapInfo.getVolumeName(),
-              snapInfo.getBucketName(),
-              snapInfo.getName());
-          OmSnapshot omSnapshot = rcOmSnapshot.get();
-
-          Table<String, RepeatedOmKeyInfo> snapshotDeletedTable =
-              omSnapshot.getMetadataManager().getDeletedTable();
-          Table<String, OmKeyInfo> snapshotDeletedDirTable =
-              omSnapshot.getMetadataManager().getDeletedDirTable();
-
-          Table<String, String> renamedTable =
-              omSnapshot.getMetadataManager().getSnapshotRenamedTable();
-
-          long volumeId = ozoneManager.getMetadataManager()
-              .getVolumeId(snapInfo.getVolumeName());
-          // Get bucketInfo for the snapshot bucket to get bucket layout.
-          String dbBucketKey = ozoneManager.getMetadataManager().getBucketKey(
-              snapInfo.getVolumeName(), snapInfo.getBucketName());
-          OmBucketInfo bucketInfo = ozoneManager.getMetadataManager()
-              .getBucketTable().get(dbBucketKey);
-
-          if (bucketInfo == null) {
-            // Decrement ref count
-            rcOmSnapshot.close();
-            rcOmSnapshot = null;
-            throw new IllegalStateException("Bucket " + "/" +
-                snapInfo.getVolumeName() + "/" + snapInfo.getBucketName() +
-                " is not found. BucketInfo should not be null for snapshotted" 
+
-                " bucket. The OM is in unexpected state.");
-          }
-
-          String snapshotBucketKey = dbBucketKey + OzoneConsts.OM_KEY_PREFIX;
-          String dbBucketKeyForDir = ozoneManager.getMetadataManager()
-              .getBucketKey(Long.toString(volumeId),
-                  Long.toString(bucketInfo.getObjectID())) + OM_KEY_PREFIX;
-
-          if (isSnapshotReclaimable(snapshotDeletedTable,
-              snapshotDeletedDirTable, snapshotBucketKey, dbBucketKeyForDir)) {
-            purgeSnapshotKeys.add(snapInfo.getTableKey());
-            // Decrement ref count
-            rcOmSnapshot.close();
-            rcOmSnapshot = null;
+          SnapshotInfo nextSnapshot = 
SnapshotUtils.getNextSnapshot(ozoneManager, snapshotChainManager, snapInfo);
+          // Continue if the next snapshot is not active. This is to avoid 
unnecessary copies from one snapshot to
+          // another.
+          if (nextSnapshot != null &&
+              nextSnapshot.getSnapshotStatus() != 
SnapshotInfo.SnapshotStatus.SNAPSHOT_ACTIVE) {
             continue;
           }
 
-          //TODO: [SNAPSHOT] Add lock to deletedTable and Active DB.
-          SnapshotInfo previousSnapshot = getPreviousActiveSnapshot(snapInfo, 
chainManager);
-          Table<String, OmKeyInfo> previousKeyTable = null;
-          Table<String, OmDirectoryInfo> previousDirTable = null;
-          OmSnapshot omPreviousSnapshot = null;
-
-          // Split RepeatedOmKeyInfo and update current snapshot 
deletedKeyTable
-          // and next snapshot deletedKeyTable.
-          if (previousSnapshot != null) {
-            rcOmPreviousSnapshot = omSnapshotManager.getSnapshot(
-                previousSnapshot.getVolumeName(),
-                previousSnapshot.getBucketName(),
-                previousSnapshot.getName());
-            omPreviousSnapshot = rcOmPreviousSnapshot.get();
-
-            previousKeyTable = omPreviousSnapshot
-                
.getMetadataManager().getKeyTable(bucketInfo.getBucketLayout());
-            previousDirTable = omPreviousSnapshot
-                .getMetadataManager().getDirectoryTable();
-          }
-
-          // Move key to either next non deleted snapshot's deletedTable
-          // or keep it in current snapshot deleted table.
-          List<SnapshotMoveKeyInfos> toReclaimList = new ArrayList<>();
-          List<SnapshotMoveKeyInfos> toNextDBList = new ArrayList<>();
-          // A list of renamed keys/files/dirs
-          List<HddsProtos.KeyValue> renamedList = new ArrayList<>();
-          List<String> dirsToMove = new ArrayList<>();
-
-          long remainNum = handleDirectoryCleanUp(snapshotDeletedDirTable,
-              previousDirTable, renamedTable, dbBucketKeyForDir, snapInfo,
-              omSnapshot, dirsToMove, renamedList);
-          int deletionCount = 0;
-
-          try (TableIterator<String, ? extends Table.KeyValue<String,
-              RepeatedOmKeyInfo>> deletedIterator = snapshotDeletedTable
-              .iterator()) {
-
-            List<BlockGroup> keysToPurge = new ArrayList<>();
-            deletedIterator.seek(snapshotBucketKey);
-
-            while (deletedIterator.hasNext() &&
-                deletionCount < remainNum) {
-              Table.KeyValue<String, RepeatedOmKeyInfo>
-                  deletedKeyValue = deletedIterator.next();
-              String deletedKey = deletedKeyValue.getKey();
-
-              // Exit if it is out of the bucket scope.
-              if (!deletedKey.startsWith(snapshotBucketKey)) {
-                // If snapshot deletedKeyTable doesn't have any
-                // entry in the snapshot scope it can be reclaimed
-                break;
-              }
-
-              RepeatedOmKeyInfo repeatedOmKeyInfo = deletedKeyValue.getValue();
-
-              SnapshotMoveKeyInfos.Builder toReclaim = SnapshotMoveKeyInfos
-                  .newBuilder()
-                  .setKey(deletedKey);
-              SnapshotMoveKeyInfos.Builder toNextDb = SnapshotMoveKeyInfos
-                  .newBuilder()
-                  .setKey(deletedKey);
-              HddsProtos.KeyValue.Builder renamedKey = HddsProtos.KeyValue
-                  .newBuilder();
-
-              for (OmKeyInfo keyInfo : repeatedOmKeyInfo.getOmKeyInfoList()) {
-                splitRepeatedOmKeyInfo(toReclaim, toNextDb, renamedKey,
-                    keyInfo, previousKeyTable, renamedTable,
-                    bucketInfo, volumeId);
-              }
-
-              // If all the KeyInfos are reclaimable in RepeatedOmKeyInfo
-              // then no need to update current snapshot deletedKeyTable.
-              if (!(toReclaim.getKeyInfosCount() ==
-                  repeatedOmKeyInfo.getOmKeyInfoList().size())) {
-                toReclaimList.add(toReclaim.build());
-                toNextDBList.add(toNextDb.build());
-              } else {
-                // The key can be reclaimed here.
-                List<BlockGroup> blocksForKeyDelete = omSnapshot
-                    .getMetadataManager()
-                    .getBlocksForKeyDelete(deletedKey);
-                if (blocksForKeyDelete != null) {
-                  keysToPurge.addAll(blocksForKeyDelete);
-                }
-              }
+          // nextSnapshot = null means entries would be moved to AOS, hence 
ensure that KeyDeletingService &
+          // DirectoryDeletingService is not running while the entries are 
moving.
+          if (nextSnapshot == null) {
+            KeyDeletingService keyDeletingService = 
getOzoneManager().getKeyManager().getDeletingService();
+            while (keyDeletingService != null && 
keyDeletingService.isRunningOnAOS()) {
+              keyDeletingService.wait(serviceTimeout);
+            }
 
-              if (renamedKey.hasKey() && renamedKey.hasValue()) {
-                renamedList.add(renamedKey.build());
+            DirectoryDeletingService directoryDeletingService = 
getOzoneManager().getKeyManager()
+                .getDirDeletingService();
+            while (directoryDeletingService != null && 
directoryDeletingService.isRunningOnAOS()) {
+              directoryDeletingService.wait(serviceTimeout);

Review Comment:
   Same comment with serviceTimeout as above



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