You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "hemantk-12 (via GitHub)" <gi...@apache.org> on 2023/04/25 23:30:51 UTC

[GitHub] [ozone] hemantk-12 opened a new pull request, #4617: HDDS-8390. [Snapshot] Added active snapshots check before returning diff report to client and in diff report generation to fail fast.

hemantk-12 opened a new pull request, #4617:
URL: https://github.com/apache/ozone/pull/4617

   ## What changes were proposed in this pull request?
   This change is to add the from and to snapshots are active before returning diff report page to the client to achieve synchronization between snapshot diff response generation and snapshot deletion. Also added the same check in diff report generation to fail fast if form, to or both snapshot/s get/s deleted in report generation because it is long running task.
   
   ## What is the link to the Apache JIRA
   https://issues.apache.org/jira/browse/HDDS-8390
   
   ## How was this patch tested?
   Existing snapshot integration tests.
   


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


[GitHub] [ozone] hemantk-12 commented on pull request #4617: HDDS-8390. [Snapshot] Added active snapshots check before returning diff report to client and in diff report generation to fail fast.

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on PR #4617:
URL: https://github.com/apache/ozone/pull/4617#issuecomment-1528137880

   > There are a few conflicts in `SnapshotDiffManager`.
   
   Resolved the conflicts. 


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


[GitHub] [ozone] prashantpogde merged pull request #4617: HDDS-8390. [Snapshot] Added active snapshots check before returning diff report to client and in diff report generation to fail fast.

Posted by "prashantpogde (via GitHub)" <gi...@apache.org>.
prashantpogde merged PR #4617:
URL: https://github.com/apache/ozone/pull/4617


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


[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4617: HDDS-8390. [Snapshot] Added active snapshots check before returning diff report to client and in diff report generation to fail fast.

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4617:
URL: https://github.com/apache/ozone/pull/4617#discussion_r1186429739


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -666,114 +687,60 @@ private void generateSnapshotDiffReport(final String jobKey,
 
       final BucketLayout bucketLayout = getBucketLayout(volume, bucket,
           fromSnapshot.getMetadataManager());
-      final Table<String, OmKeyInfo> fsKeyTable =
-          fromSnapshot.getMetadataManager().getKeyTable(bucketLayout);
-      final Table<String, OmKeyInfo> tsKeyTable =
-          toSnapshot.getMetadataManager().getKeyTable(bucketLayout);
-
-      boolean useFullDiff = ozoneManager.getConfiguration().getBoolean(
-          OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF,
-          OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF_DEFAULT);
-      if (forceFullDiff) {
-        useFullDiff = true;
-      }
-
       Map<String, String> tablePrefixes =
           getTablePrefixes(toSnapshot.getMetadataManager(), volume, bucket);
-      List<String> tablesToLookUp =
-              Collections.singletonList(fsKeyTable.getName());
-      Set<String> deltaFilesForKeyOrFileTable = getDeltaFiles(fromSnapshot,
-          toSnapshot, tablesToLookUp, fsInfo, tsInfo, useFullDiff,
-          tablePrefixes, path.toString());
-
-      // Workaround to handle deletes if native rocksDb tool for reading
-      // tombstone is not loaded.
-      // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
-      if (!isNativeRocksToolsLoaded) {
-        deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
-                fromSnapshot, tablesToLookUp));
-      }
-      try {
-        addToObjectIdMap(fsKeyTable, tsKeyTable,
-            Pair.of(isNativeRocksToolsLoaded, deltaFilesForKeyOrFileTable),
-            objectIdToKeyNameMapForFromSnapshot,
-            objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
-            tablePrefixes);
-      } catch (NativeLibraryNotLoadedException e) {
-        // Workaround to handle deletes if use of native rocksDb tool for
-        // reading tombstone fails.
-        // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
-        deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
-                fromSnapshot, tablesToLookUp));
-        try {
-          addToObjectIdMap(fsKeyTable, tsKeyTable,
-              Pair.of(false, deltaFilesForKeyOrFileTable),
-              objectIdToKeyNameMapForFromSnapshot,
-              objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
-              tablePrefixes);
-        } catch (NativeLibraryNotLoadedException ex) {
-          // This code should be never executed.
-          throw new IllegalStateException(ex);
-        }
-      }
+
+      boolean useFullDiff = snapshotForceFullDiff || forceFullDiff;
+
+      validateSnapshotsAreActive(volume, bucket, fromSnapshotName,
+          toSnapshotName);
+      Table<String, OmKeyInfo> fsKeyTable = fromSnapshot.getMetadataManager()
+          .getKeyTable(bucketLayout);
+      Table<String, OmKeyInfo> tsKeyTable = toSnapshot.getMetadataManager()
+          .getKeyTable(bucketLayout);
+
+      getDeltaFilesAndDiffKeysToObjectIdToKeyMap(fsKeyTable, tsKeyTable,
+          fromSnapshot, toSnapshot, fsInfo, tsInfo, useFullDiff,
+          tablePrefixes, objectIdToKeyNameMapForFromSnapshot,
+          objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
+          path.toString());
 
       if (bucketLayout.isFileSystemOptimized()) {
-        final Table<String, OmDirectoryInfo> fsDirTable =
+        validateSnapshotsAreActive(volume, bucket, fromSnapshotName,
+            toSnapshotName);
+
+        Table<String, OmDirectoryInfo> fsDirTable =
             fromSnapshot.getMetadataManager().getDirectoryTable();
-        final Table<String, OmDirectoryInfo> tsDirTable =
+        Table<String, OmDirectoryInfo> tsDirTable =
             toSnapshot.getMetadataManager().getDirectoryTable();
-        tablesToLookUp = Collections.singletonList(fsDirTable.getName());
-        final Set<String> deltaFilesForDirTable =
-            getDeltaFiles(fromSnapshot, toSnapshot, tablesToLookUp, fsInfo,
-                    tsInfo, useFullDiff, tablePrefixes, path.toString());
-        if (!isNativeRocksToolsLoaded) {
-          deltaFilesForDirTable.addAll(getSSTFileListForSnapshot(
-                  fromSnapshot, tablesToLookUp));
-        }
-        try {
-          addToObjectIdMap(fsDirTable, tsDirTable,
-              Pair.of(isNativeRocksToolsLoaded, deltaFilesForDirTable),
-              objectIdToKeyNameMapForFromSnapshot,
-              objectIdToKeyNameMapForToSnapshot,
-              objectIDsToCheckMap,
-              tablePrefixes);
-        } catch (NativeLibraryNotLoadedException e) {
-          try {
-            // Workaround to handle deletes if use of native rockstools for
-            // reading tombstone fails.
-            // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
-            deltaFilesForDirTable.addAll(getSSTFileListForSnapshot(
-                    fromSnapshot, tablesToLookUp));
-            addToObjectIdMap(fsDirTable, tsDirTable,
-                Pair.of(false, deltaFilesForDirTable),
-                objectIdToKeyNameMapForFromSnapshot,
-                objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
-                tablePrefixes);
-          } catch (NativeLibraryNotLoadedException ex) {
-            // This code should be never executed.
-            throw new IllegalStateException(ex);
-          }
-        }
+
+        getDeltaFilesAndDiffKeysToObjectIdToKeyMap(fsDirTable, tsDirTable,
+            fromSnapshot, toSnapshot, fsInfo, tsInfo, useFullDiff,
+            tablePrefixes, objectIdToKeyNameMapForFromSnapshot,
+            objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
+            path.toString());
       }
 
+      validateSnapshotsAreActive(volume, bucket, fromSnapshotName,
+          toSnapshotName);
       long totalDiffEntries = generateDiffReport(jobId,
           objectIDsToCheckMap,
           objectIdToKeyNameMapForFromSnapshot,
           objectIdToKeyNameMapForToSnapshot);
 
       updateJobStatusToDone(jobKey, totalDiffEntries);
-    } catch (IOException | RocksDBException exception) {
+    } catch (ExecutionException | IOException | RocksDBException exception) {
       updateJobStatus(jobKey, IN_PROGRESS, FAILED);
-      LOG.error("Caught checked exception during diff report generation for " +
+      LOG.info("Caught checked exception during diff report generation for " +

Review Comment:
   My understanding was that checked exceptions (known exceptions) should be logged at `INFO` level and handle properly. But then giving it more thoughts, checked exceptions do not they are expected, especially in this case. It make sense to keep log level `ERROR`, Hence reverted the log level. 



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


[GitHub] [ozone] hemantk-12 commented on a diff in pull request #4617: HDDS-8390. [Snapshot] Added active snapshots check before returning diff report to client and in diff report generation to fail fast.

Posted by "hemantk-12 (via GitHub)" <gi...@apache.org>.
hemantk-12 commented on code in PR #4617:
URL: https://github.com/apache/ozone/pull/4617#discussion_r1186425155


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -786,29 +753,88 @@ private void generateSnapshotDiffReport(final String jobKey,
     }
   }
 
+  @SuppressWarnings("checkstyle:ParameterNumber")
+  private void getDeltaFilesAndDiffKeysToObjectIdToKeyMap(
+      final Table<String, ? extends WithObjectID> fsTable,
+      final Table<String, ? extends WithObjectID> tsTable,
+      final OmSnapshot fromSnapshot,
+      final OmSnapshot toSnapshot,
+      final SnapshotInfo fsInfo,
+      final SnapshotInfo tsInfo,
+      final boolean useFullDiff,
+      final Map<String, String> tablePrefixes,
+      final PersistentMap<byte[], byte[]> oldObjIdToKeyMap,
+      final PersistentMap<byte[], byte[]> newObjIdToKeyMap,
+      final PersistentSet<byte[]> objectIDsToCheck,
+      final String diffDir 
+  ) throws IOException, RocksDBException {
+
+    List<String> tablesToLookUp = Collections.singletonList(fsTable.getName());
+
+    Set<String> deltaFiles = getDeltaFiles(fromSnapshot, toSnapshot,
+        tablesToLookUp, fsInfo, tsInfo, useFullDiff, tablePrefixes, diffDir);
+
+    // Workaround to handle deletes if native rocksDb tool for reading
+    // tombstone is not loaded.
+    // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
+    if (!sstDumpTool.isPresent()) {
+      deltaFiles.addAll(getSSTFileListForSnapshot(fromSnapshot,
+          tablesToLookUp));
+    }
+
+    try {
+      addToObjectIdMap(fsTable,
+          tsTable,
+          deltaFiles,
+          sstDumpTool.isPresent(),
+          oldObjIdToKeyMap,
+          newObjIdToKeyMap,
+          objectIDsToCheck,
+          tablePrefixes);
+    } catch (NativeLibraryNotLoadedException e) {
+      try {

Review Comment:
   Added the log statement.



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


[GitHub] [ozone] smengcl commented on pull request #4617: HDDS-8390. [Snapshot] Added active snapshots check before returning diff report to client and in diff report generation to fail fast.

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #4617:
URL: https://github.com/apache/ozone/pull/4617#issuecomment-1526693418

   There are a few conflicts in `SnapshotDiffManager`.


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


[GitHub] [ozone] smengcl commented on a diff in pull request #4617: HDDS-8390. [Snapshot] Added active snapshots check before returning diff report to client and in diff report generation to fail fast.

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #4617:
URL: https://github.com/apache/ozone/pull/4617#discussion_r1185775695


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -666,114 +687,60 @@ private void generateSnapshotDiffReport(final String jobKey,
 
       final BucketLayout bucketLayout = getBucketLayout(volume, bucket,
           fromSnapshot.getMetadataManager());
-      final Table<String, OmKeyInfo> fsKeyTable =
-          fromSnapshot.getMetadataManager().getKeyTable(bucketLayout);
-      final Table<String, OmKeyInfo> tsKeyTable =
-          toSnapshot.getMetadataManager().getKeyTable(bucketLayout);
-
-      boolean useFullDiff = ozoneManager.getConfiguration().getBoolean(
-          OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF,
-          OZONE_OM_SNAPSHOT_FORCE_FULL_DIFF_DEFAULT);
-      if (forceFullDiff) {
-        useFullDiff = true;
-      }
-
       Map<String, String> tablePrefixes =
           getTablePrefixes(toSnapshot.getMetadataManager(), volume, bucket);
-      List<String> tablesToLookUp =
-              Collections.singletonList(fsKeyTable.getName());
-      Set<String> deltaFilesForKeyOrFileTable = getDeltaFiles(fromSnapshot,
-          toSnapshot, tablesToLookUp, fsInfo, tsInfo, useFullDiff,
-          tablePrefixes, path.toString());
-
-      // Workaround to handle deletes if native rocksDb tool for reading
-      // tombstone is not loaded.
-      // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
-      if (!isNativeRocksToolsLoaded) {
-        deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
-                fromSnapshot, tablesToLookUp));
-      }
-      try {
-        addToObjectIdMap(fsKeyTable, tsKeyTable,
-            Pair.of(isNativeRocksToolsLoaded, deltaFilesForKeyOrFileTable),
-            objectIdToKeyNameMapForFromSnapshot,
-            objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
-            tablePrefixes);
-      } catch (NativeLibraryNotLoadedException e) {
-        // Workaround to handle deletes if use of native rocksDb tool for
-        // reading tombstone fails.
-        // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
-        deltaFilesForKeyOrFileTable.addAll(getSSTFileListForSnapshot(
-                fromSnapshot, tablesToLookUp));
-        try {
-          addToObjectIdMap(fsKeyTable, tsKeyTable,
-              Pair.of(false, deltaFilesForKeyOrFileTable),
-              objectIdToKeyNameMapForFromSnapshot,
-              objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
-              tablePrefixes);
-        } catch (NativeLibraryNotLoadedException ex) {
-          // This code should be never executed.
-          throw new IllegalStateException(ex);
-        }
-      }
+
+      boolean useFullDiff = snapshotForceFullDiff || forceFullDiff;
+
+      validateSnapshotsAreActive(volume, bucket, fromSnapshotName,
+          toSnapshotName);
+      Table<String, OmKeyInfo> fsKeyTable = fromSnapshot.getMetadataManager()
+          .getKeyTable(bucketLayout);
+      Table<String, OmKeyInfo> tsKeyTable = toSnapshot.getMetadataManager()
+          .getKeyTable(bucketLayout);
+
+      getDeltaFilesAndDiffKeysToObjectIdToKeyMap(fsKeyTable, tsKeyTable,
+          fromSnapshot, toSnapshot, fsInfo, tsInfo, useFullDiff,
+          tablePrefixes, objectIdToKeyNameMapForFromSnapshot,
+          objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
+          path.toString());
 
       if (bucketLayout.isFileSystemOptimized()) {
-        final Table<String, OmDirectoryInfo> fsDirTable =
+        validateSnapshotsAreActive(volume, bucket, fromSnapshotName,
+            toSnapshotName);
+
+        Table<String, OmDirectoryInfo> fsDirTable =
             fromSnapshot.getMetadataManager().getDirectoryTable();
-        final Table<String, OmDirectoryInfo> tsDirTable =
+        Table<String, OmDirectoryInfo> tsDirTable =
             toSnapshot.getMetadataManager().getDirectoryTable();
-        tablesToLookUp = Collections.singletonList(fsDirTable.getName());
-        final Set<String> deltaFilesForDirTable =
-            getDeltaFiles(fromSnapshot, toSnapshot, tablesToLookUp, fsInfo,
-                    tsInfo, useFullDiff, tablePrefixes, path.toString());
-        if (!isNativeRocksToolsLoaded) {
-          deltaFilesForDirTable.addAll(getSSTFileListForSnapshot(
-                  fromSnapshot, tablesToLookUp));
-        }
-        try {
-          addToObjectIdMap(fsDirTable, tsDirTable,
-              Pair.of(isNativeRocksToolsLoaded, deltaFilesForDirTable),
-              objectIdToKeyNameMapForFromSnapshot,
-              objectIdToKeyNameMapForToSnapshot,
-              objectIDsToCheckMap,
-              tablePrefixes);
-        } catch (NativeLibraryNotLoadedException e) {
-          try {
-            // Workaround to handle deletes if use of native rockstools for
-            // reading tombstone fails.
-            // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
-            deltaFilesForDirTable.addAll(getSSTFileListForSnapshot(
-                    fromSnapshot, tablesToLookUp));
-            addToObjectIdMap(fsDirTable, tsDirTable,
-                Pair.of(false, deltaFilesForDirTable),
-                objectIdToKeyNameMapForFromSnapshot,
-                objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
-                tablePrefixes);
-          } catch (NativeLibraryNotLoadedException ex) {
-            // This code should be never executed.
-            throw new IllegalStateException(ex);
-          }
-        }
+
+        getDeltaFilesAndDiffKeysToObjectIdToKeyMap(fsDirTable, tsDirTable,
+            fromSnapshot, toSnapshot, fsInfo, tsInfo, useFullDiff,
+            tablePrefixes, objectIdToKeyNameMapForFromSnapshot,
+            objectIdToKeyNameMapForToSnapshot, objectIDsToCheckMap,
+            path.toString());
       }
 
+      validateSnapshotsAreActive(volume, bucket, fromSnapshotName,
+          toSnapshotName);
       long totalDiffEntries = generateDiffReport(jobId,
           objectIDsToCheckMap,
           objectIdToKeyNameMapForFromSnapshot,
           objectIdToKeyNameMapForToSnapshot);
 
       updateJobStatusToDone(jobKey, totalDiffEntries);
-    } catch (IOException | RocksDBException exception) {
+    } catch (ExecutionException | IOException | RocksDBException exception) {
       updateJobStatus(jobKey, IN_PROGRESS, FAILED);
-      LOG.error("Caught checked exception during diff report generation for " +
+      LOG.info("Caught checked exception during diff report generation for " +

Review Comment:
   Why is log level changed to `info` for exception message?



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -360,49 +364,51 @@ private Set<String> getSSTFileListForSnapshot(OmSnapshot snapshot,
         .getPath(), tablesToLookUp);
   }
 
-  @SuppressWarnings("parameternumber")
-  public SnapshotDiffResponse getSnapshotDiffReport(
-      final String volume,
-      final String bucket,
-      final OmSnapshot fromSnapshot,
-      final OmSnapshot toSnapshot,
-      final SnapshotInfo fsInfo,
-      final SnapshotInfo tsInfo,
-      final int index,
-      final int pageSize,
-      final boolean forceFullDiff
+  public SnapshotDiffResponse getSnapshotDiffReport(final String volume,
+                                                    final String bucket,
+                                                    final String fromSnapshot,
+                                                    final String toSnapshot,
+                                                    final int index,
+                                                    final int pageSize,
+                                                    final boolean forceFullDiff
   ) throws IOException {

Review Comment:
   Thx for refactoring the parameter list!



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -786,29 +753,88 @@ private void generateSnapshotDiffReport(final String jobKey,
     }
   }
 
+  @SuppressWarnings("checkstyle:ParameterNumber")
+  private void getDeltaFilesAndDiffKeysToObjectIdToKeyMap(
+      final Table<String, ? extends WithObjectID> fsTable,
+      final Table<String, ? extends WithObjectID> tsTable,
+      final OmSnapshot fromSnapshot,
+      final OmSnapshot toSnapshot,
+      final SnapshotInfo fsInfo,
+      final SnapshotInfo tsInfo,
+      final boolean useFullDiff,
+      final Map<String, String> tablePrefixes,
+      final PersistentMap<byte[], byte[]> oldObjIdToKeyMap,
+      final PersistentMap<byte[], byte[]> newObjIdToKeyMap,
+      final PersistentSet<byte[]> objectIDsToCheck,
+      final String diffDir 
+  ) throws IOException, RocksDBException {
+
+    List<String> tablesToLookUp = Collections.singletonList(fsTable.getName());
+
+    Set<String> deltaFiles = getDeltaFiles(fromSnapshot, toSnapshot,
+        tablesToLookUp, fsInfo, tsInfo, useFullDiff, tablePrefixes, diffDir);
+
+    // Workaround to handle deletes if native rocksDb tool for reading
+    // tombstone is not loaded.
+    // TODO: [SNAPSHOT] Update Rocksdb SSTFileIterator to read tombstone
+    if (!sstDumpTool.isPresent()) {
+      deltaFiles.addAll(getSSTFileListForSnapshot(fromSnapshot,
+          tablesToLookUp));
+    }
+
+    try {
+      addToObjectIdMap(fsTable,
+          tsTable,
+          deltaFiles,
+          sstDumpTool.isPresent(),
+          oldObjIdToKeyMap,
+          newObjIdToKeyMap,
+          objectIDsToCheck,
+          tablePrefixes);
+    } catch (NativeLibraryNotLoadedException e) {
+      try {

Review Comment:
   Not related. But good to have a log message here just in case we would catch some bugs with it.
   
   ```suggestion
       } catch (NativeLibraryNotLoadedException e) {
         LOG.warn("SSTDumpTool load failure, retrying without it", e);
         try {
   ```



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