swamirishi commented on code in PR #6226:
URL: https://github.com/apache/ozone/pull/6226#discussion_r1498625651
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java:
##########
@@ -137,13 +139,31 @@ public static void createHardLinks(Path dbPath) throws
IOException {
}
}
+ private static void createHardLinksOrCopy(Path fullFromPath, Path
fullToPath) throws IOException {
+ if (isSamePartition(fullFromPath.getParent(), fullToPath.getParent())) {
+ Files.createLink(fullToPath, fullFromPath);
+ } else {
+ Files.copy(fullFromPath, fullToPath,
StandardCopyOption.REPLACE_EXISTING);
Review Comment:
Can we do some kind of a checksum verification if the fullToPath exists.
1) copy to tmp file
2) Verify checksum if the files exist
3) if checksums don't match raise exception.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java:
##########
@@ -137,13 +139,31 @@ public static void createHardLinks(Path dbPath) throws
IOException {
}
}
+ private static void createHardLinksOrCopy(Path fullFromPath, Path
fullToPath) throws IOException {
+ if (isSamePartition(fullFromPath.getParent(), fullToPath.getParent())) {
+ Files.createLink(fullToPath, fullFromPath);
+ } else {
+ Files.copy(fullFromPath, fullToPath,
StandardCopyOption.REPLACE_EXISTING);
Review Comment:
Is replace existing correct? Can this kind of a situation occur where we are
re-copying stuff? We shouldn't inadvertently delete data.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java:
##########
@@ -66,7 +66,7 @@ public void testLinkFiles(@TempDir File tempDir) throws
Exception {
assertFalse(tree2.exists());
assertFalse(f1Link.exists());
- OmSnapshotUtils.linkFiles(tree1, tree2);
+ OmSnapshotUtils.linkFilesOrCopy(tree1, tree2);
Review Comment:
Mocking this isSamePartition function should help. From what I know you
cannot mock Files interface through mockito but PowerMockito can do it.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotUtils.java:
##########
@@ -66,7 +66,7 @@ public void testLinkFiles(@TempDir File tempDir) throws
Exception {
assertFalse(tree2.exists());
assertFalse(f1Link.exists());
- OmSnapshotUtils.linkFiles(tree1, tree2);
+ OmSnapshotUtils.linkFilesOrCopy(tree1, tree2);
Review Comment:
Please add similar test case for copy case, when both files are in different
partitions. You can achieve this by mocking the layer.
--
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]
