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


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/service/SnapshotDeletingService.java:
##########
@@ -136,316 +157,90 @@ 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 = chainManager.iterator(true);
+        List<String> 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, 
chainManager, iterator.next());
           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;
+          LOG.info("Started Snapshot Deletion Processing for snapshot : {}", 
snapInfo.getTableKey());
+          SnapshotInfo nextSnapshot = 
SnapshotUtils.getNextSnapshot(ozoneManager, chainManager, 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();
+          // 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) {
+            waitForKeyDeletingService();

Review Comment:
   done



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