Copilot commented on code in PR #9079:
URL: https://github.com/apache/ozone/pull/9079#discussion_r2408821405
##########
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 {
Review Comment:
The method documentation is missing parameters and return value
descriptions. Add @param and @return annotations to explain the fromSnapshotId
parameter and the returned VersionMeta object.
##########
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,
+ String... dbPaths) {
+
+ // Try to locate the SST in the backup dir first
+ final Path sstPathInBackupDir = Paths.get(sstBackupDir,
+ sstFilename);
+ if (Files.exists(sstPathInBackupDir)) {
+ return sstPathInBackupDir.toString();
+ }
+
+ // SST file does not exist in the SST backup dir, this means the SST file
+ // has not gone through any compactions yet and is only available in the
+ // src DB directory or destDB directory
+ for (String dbPath : dbPaths) {
+ final Path sstPathInDBDir = Paths.get(dbPath,
+ sstFilename);
+ if (Files.exists(sstPathInDBDir)) {
+ return sstPathInDBDir.toString();
+ }
+ }
+
+ // TODO: More graceful error handling?
+ throw new RuntimeException("Unable to locate SST file: " +
+ sstFilename);
+ }
+
Review Comment:
This method lacks Javadoc documentation. Add documentation explaining the
method's purpose, parameters, return value, and potential IOException handling.
```suggestion
/**
* Creates hard links for the specified SST files from the source or
destination
* DB paths into the provided directory for SnapDiff jobs, and returns the
set
* of full paths to the linked SST files.
*
* @param diffFiles List of SST file names to be linked.
* @param srcDbPath Path to the source DB directory.
* @param dstDbPath Path to the destination DB directory.
* @param sstFilesDirForSnapDiffJob Directory where hard links will be
created.
* @return Set of full paths to the hard-linked SST files.
* @throws RuntimeException if a hard link cannot be created due to an
IOException.
*/
```
##########
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 {
}
}
}
+
Review Comment:
This method lacks Javadoc documentation. Add documentation explaining the
method's purpose, parameters, return value, and the exception it throws when
SST file is not found.
```suggestion
/**
* Locates the full path of the specified SST file by searching the backup
directory
* first, followed by the provided database directories.
*
* @param sstFilename The name of the SST file to locate.
* @param sstBackupDir The directory where SST backups are stored.
* @param dbPaths One or more database directories to search for the
SST file.
* @return The full path to the SST file as a string.
* @throws RuntimeException if the SST file cannot be found in any of the
specified directories.
*/
```
##########
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);
+ }
+
+ /**
+ * Calculates the symmetric difference of SST files between two version
metadata.
+ * Returns files that are present in one snapshot but not in the other.
+ *
+ * @param fromSnapVersionMeta Version metadata from the first snapshot
+ * @param toSnapVersionMeta Version metadata from the second snapshot
+ * @return List of SST file names that differ between the snapshots
Review Comment:
The method documentation is incomplete. The @param annotations are missing
for both parameters, and the method description should clarify that it returns
a symmetric difference.
```suggestion
* Returns a list of SST file names that are present in one snapshot but
not in the other,
* i.e., the symmetric difference between the two sets of SST files.
*
* @param fromSnapVersionMeta Version metadata from the first snapshot;
provides the set of SST files for comparison.
* @param toSnapVersionMeta Version metadata from the second snapshot;
provides the set of SST files for comparison.
* @return List of SST file names that are present in only one of the two
snapshots (symmetric difference).
```
--
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]
