Copilot commented on code in PR #10679:
URL: https://github.com/apache/ozone/pull/10679#discussion_r3532945936
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java:
##########
@@ -360,16 +414,23 @@ private void init(OzoneConfiguration configuration,
SnapshotChainManager chainMa
throw new IOException("Error while listing yaml files inside directory:
" + snapshotDir.getAbsolutePath());
}
Arrays.sort(localDataFiles, Comparator.comparing(File::getName));
+ Set<String> failedFilePaths = new HashSet<>();
for (File localDataFile : localDataFiles) {
- OmSnapshotLocalData snapshotLocalData =
snapshotLocalDataSerializer.load(localDataFile);
+ Optional<OmSnapshotLocalData> loadedLocalData =
tryLoadSnapshotLocalData(localDataFile, failedFilePaths);
+ if (!loadedLocalData.isPresent()) {
+ continue;
+ }
+ OmSnapshotLocalData snapshotLocalData = loadedLocalData.get();
File file = new
File(getSnapshotLocalPropertyYamlPath(snapshotLocalData.getSnapshotId()));
String expectedPath = file.getAbsolutePath();
String actualPath = localDataFile.getAbsolutePath();
if (!expectedPath.equals(actualPath)) {
- throw new IOException("Unexpected path for local data file with
snapshotId:" + snapshotLocalData.getSnapshotId()
- + " : " + actualPath + ". " + "Expected: " + expectedPath);
+ failedFilePaths.add(actualPath);
+ LOG.error("Skipping snapshot local data file {} because its stored
snapshotId {} does not match its path. " +
+ "Expected path: {}.", actualPath,
snapshotLocalData.getSnapshotId(), expectedPath);
+ continue;
}
- addVersionNodeWithDependents(snapshotLocalData);
+ addVersionNodeWithDependents(snapshotLocalData, failedFilePaths);
}
Review Comment:
In init(), the return value of addVersionNodeWithDependents(...) is ignored.
When it returns false (eg due to an unloadable/mismatched previous snapshot
YAML), the current snapshot YAML will remain reloadable and may be parsed
repeatedly as a dependency of later snapshots. With large YAMLs this can lead
to unnecessary repeated IO/parse work during OM startup. Consider marking the
current YAML path as failed/skipped when addVersionNodeWithDependents returns
false so subsequent dependency walks can short-circuit without reparsing it.
--
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]
