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


##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java:
##########
@@ -214,4 +215,47 @@ public static void linkFiles(File oldDir, File newDir) 
throws IOException {
       }
     }
   }
+
+  static String getSSTFullPath(String sstFilename, String sstBackupDir,

Review Comment:
   ```suggestion
     static private String getSSTFullPath(String sstFilename, String 
sstBackupDir,
   ```



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -1180,6 +1192,36 @@ Set<String> getDeltaFiles(OmSnapshot fromSnapshot,
     // TODO: [SNAPSHOT] Refactor the parameter list
     Optional<Set<String>> deltaFiles = Optional.empty();
 
+    OmSnapshotLocalData fromSnapshotLocalData = getSnapshotLocalData(fsInfo);
+    int fromSnapshotVersion = fromSnapshotLocalData.getVersion();
+    OmSnapshotLocalData toSnapshotLocalData = getSnapshotLocalData(tsInfo);
+    int toSnapshotVersion = toSnapshotLocalData.getVersion();
+
+    try {
+      if (fromSnapshotVersion > 0 && toSnapshotVersion > 0) {
+        // both snapshots are defragmented, To calculate snap-diff, we can 
simply compare the
+        // SST files contained in OmSnapshotLocalData instances of both these 
and get the delta files
+        OmSnapshotLocalData.VersionMeta toSnapVersionMeta =
+            
toSnapshotLocalData.getVersionSstFileInfos().get(toSnapshotVersion);
+        // get the source snapshot SST files (versionMeta)  corresponding to 
target snapshot
+        OmSnapshotLocalData.VersionMeta fromSnapVersionMeta =
+            resolveBaseVersionMeta(toSnapshotLocalData, 
fromSnapshot.getSnapshotID());
+        // Calculate diff files using helper method
+        if (toSnapVersionMeta == null) {
+          String errMsg =
+              "Cannot find corresponding version of from snapshot " + 
fromSnapshotVersion + " from " + tsInfo;
+          LOG.error(errMsg);
+          throw new IOException(errMsg);
+        }
+        List<String> diffFiles = calculateDiffFiles(fromSnapVersionMeta, 
toSnapVersionMeta);
+        return OmSnapshotUtils.getSSTDiffListWithFullPath(diffFiles,
+            
OmSnapshotManager.getSnapshotPath(ozoneManager.getMetadataManager(), 
fsInfo).toString(),
+            
OmSnapshotManager.getSnapshotPath(ozoneManager.getMetadataManager(), 
tsInfo).toString(), diffDir);
+      }
+    } catch (Exception e) {
+      LOG.error("Failed to calculate snap-diff via optimal method, Falling 
back to other methods", e);

Review Comment:
   if it does happen, it's better to log fromSnapshot, toSnapshot and fsInfo so 
we know where to troubleshoote to



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -1223,6 +1265,70 @@ Set<String> getDeltaFiles(OmSnapshot fromSnapshot,
             toSnapshot.getSnapshotTableKey()));
   }
 
+  /**
+   * Resolve the VersionMeta of the ancestor snapshot (fromSnapshotId)
+   * that the given snapshot (toSnapshot) was built on.
+   *
+   * Traverses the snapshot chain backwards using prevSnapId.
+   */
+  private OmSnapshotLocalData.VersionMeta resolveBaseVersionMeta(
+      OmSnapshotLocalData toSnapshot,
+      UUID fromSnapshotId) throws IOException {
+    OmMetadataManagerImpl metadataManager =
+        (OmMetadataManagerImpl) ozoneManager.getMetadataManager();
+    // Start walking back from the child snapshot
+    OmSnapshotLocalData child = toSnapshot;
+    while (!child.getPreviousSnapshotId().equals(fromSnapshotId)) {
+      UUID parentId = child.getPreviousSnapshotId();
+      // Load the parent snapshot in the chain
+      child = getSnapshotLocalData(
+          getSnapshotInfo(ozoneManager,
+              metadataManager.getSnapshotChainManager(),
+              parentId));
+    }
+    SnapshotInfo snapshotInfo =
+        getSnapshotInfo(ozoneManager, 
metadataManager.getSnapshotChainManager(), fromSnapshotId);
+    OmSnapshotLocalData fromSnapshot = getSnapshotLocalData(snapshotInfo);
+    // Get the version that the child was built from
+    OmSnapshotLocalData.VersionMeta childVersionMeta = 
child.getVersionSstFileInfos().get(child.getVersion());
+    int versionUsedWhenBuildingChild = 
childVersionMeta.getPreviousSnapshotVersion();
+    return 
fromSnapshot.getVersionSstFileInfos().get(versionUsedWhenBuildingChild);

Review Comment:
   what if versionUsedWhenBuildingChild is not found?



##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java:
##########
@@ -1223,6 +1265,70 @@ Set<String> getDeltaFiles(OmSnapshot fromSnapshot,
             toSnapshot.getSnapshotTableKey()));
   }
 
+  /**
+   * Resolve the VersionMeta of the ancestor snapshot (fromSnapshotId)
+   * that the given snapshot (toSnapshot) was built on.
+   *
+   * Traverses the snapshot chain backwards using prevSnapId.
+   */
+  private OmSnapshotLocalData.VersionMeta resolveBaseVersionMeta(
+      OmSnapshotLocalData toSnapshot,
+      UUID fromSnapshotId) throws IOException {
+    OmMetadataManagerImpl metadataManager =
+        (OmMetadataManagerImpl) ozoneManager.getMetadataManager();
+    // Start walking back from the child snapshot
+    OmSnapshotLocalData child = toSnapshot;
+    while (!child.getPreviousSnapshotId().equals(fromSnapshotId)) {

Review Comment:
   better to have a stop condition otherwise it could end up in an infinite 
loop.



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