Copilot commented on code in PR #9335:
URL: https://github.com/apache/ozone/pull/9335#discussion_r2566231743
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java:
##########
@@ -309,6 +317,36 @@ public void writeDbDataToStream(HttpServletRequest
request, OutputStream destina
}
}
+ /**
+ * Retrieves the paths to the local property YAML files for the specified
snapshot IDs.
+ * This method resolves the chain of previous snapshot references for each
snapshot ID
+ * and gathers their corresponding local property YAML file paths.
+ *
+ * @param localDataManager The OmSnapshotLocalDataManager instance
responsible for managing
+ * snapshot data and metadata.
+ * @param snapshotIds A set of snapshot IDs for which the local
property YAML file
+ * paths should be resolved.
+ * @return A collection of paths to the local property YAML files for the
specified
+ * snapshot IDs.
+ */
+ private Collection<Path>
getSnapshotLocalDataPaths(OmSnapshotLocalDataManager localDataManager,
+ Set<UUID> snapshotIds) {
+ Set<UUID> snapshotLocalDataIds = new HashSet<>();
+ Map<UUID, OmSnapshotLocalDataManager.SnapshotVersionsMeta> versionNodeMap =
+ localDataManager.getVersionNodeMapUnmodifiable();
+ for (UUID snapshot : snapshotIds) {
+ UUID id = snapshot;
+ // Get the previous snapshot id for the current snapshot id until we
reach null or the first snapshot id which
+ // is already in the snapshotLocalDataIds set.
+ while (id != null && !snapshotLocalDataIds.contains(id)) {
+ snapshotLocalDataIds.add(id);
+ id = versionNodeMap.get(id).getPreviousSnapshotId();
+ }
+ }
+ return
snapshotLocalDataIds.stream().map(localDataManager::getSnapshotLocalPropertyYamlPath)
+ .map(Paths::get).collect(Collectors.toList());
Review Comment:
Access of [element](1) annotated with VisibleForTesting found in production
code.
```suggestion
// Avoid using @VisibleForTesting method. Construct the path manually
using public API.
// Assume the local property YAML file is named "local_property.yaml" in
the snapshot directory.
return snapshotLocalDataIds.stream()
.map(id -> Paths.get(localDataManager.getSnapshotDir(id).toString(),
"local_property.yaml"))
.collect(Collectors.toList());
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java:
##########
@@ -309,6 +317,36 @@ public void writeDbDataToStream(HttpServletRequest
request, OutputStream destina
}
}
+ /**
+ * Retrieves the paths to the local property YAML files for the specified
snapshot IDs.
+ * This method resolves the chain of previous snapshot references for each
snapshot ID
+ * and gathers their corresponding local property YAML file paths.
+ *
+ * @param localDataManager The OmSnapshotLocalDataManager instance
responsible for managing
+ * snapshot data and metadata.
+ * @param snapshotIds A set of snapshot IDs for which the local
property YAML file
+ * paths should be resolved.
+ * @return A collection of paths to the local property YAML files for the
specified
+ * snapshot IDs.
+ */
+ private Collection<Path>
getSnapshotLocalDataPaths(OmSnapshotLocalDataManager localDataManager,
+ Set<UUID> snapshotIds) {
+ Set<UUID> snapshotLocalDataIds = new HashSet<>();
+ Map<UUID, OmSnapshotLocalDataManager.SnapshotVersionsMeta> versionNodeMap =
+ localDataManager.getVersionNodeMapUnmodifiable();
+ for (UUID snapshot : snapshotIds) {
+ UUID id = snapshot;
+ // Get the previous snapshot id for the current snapshot id until we
reach null or the first snapshot id which
+ // is already in the snapshotLocalDataIds set.
+ while (id != null && !snapshotLocalDataIds.contains(id)) {
+ snapshotLocalDataIds.add(id);
+ id = versionNodeMap.get(id).getPreviousSnapshotId();
Review Comment:
Potential NullPointerException: `versionNodeMap.get(id)` can return null if
the snapshot ID is not present in the version node map. This will cause a NPE
when calling `.getPreviousSnapshotId()`. Consider adding a null check:
```java
OmSnapshotLocalDataManager.SnapshotVersionsMeta meta =
versionNodeMap.get(id);
if (meta == null) {
break; // or handle appropriately
}
id = meta.getPreviousSnapshotId();
```
```suggestion
OmSnapshotLocalDataManager.SnapshotVersionsMeta meta =
versionNodeMap.get(id);
if (meta == null) {
break;
}
id = meta.getPreviousSnapshotId();
```
--
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]
