Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ public DBCheckpoint getCheckpoint(Path tmpdir, boolean flush)
differ.incrementTarballRequestCount();
FileUtils.copyDirectory(compactionLogDir.getOriginalDir(),
compactionLogDir.getTmpDir());
OmSnapshotUtils.linkFiles(sstBackupDir.getOriginalDir(),
OmSnapshotUtils.linkFilesOrCopy(sstBackupDir.getOriginalDir(),
sstBackupDir.getTmpDir());
checkpoint = getDbStore().getCheckpoint(flush);
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3946,7 +3946,7 @@ private void moveCheckpointFiles(File oldDB, Path checkpointPath, File dbDir,
// Link each of the candidate DB files to real DB directory. This
// preserves the links that already exist between files in the
// candidate db.
OmSnapshotUtils.linkFiles(checkpointPath.toFile(),
OmSnapshotUtils.linkFilesOrCopy(checkpointPath.toFile(),
oldDB);
moveOmSnapshotData(oldDB.toPath(), dbSnapshotsDir.toPath());
Files.deleteIfExists(markerFile);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@
import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileStore;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -128,7 +130,12 @@ public static void createHardLinks(Path dbPath) throws IOException {
"Failed to create directory: " + parent.toString());
}
}
Files.createLink(fullToPath, fullFromPath);

if (isSamePartition(fullFromPath.getParent(), fullToPath.getParent())) {
Files.createLink(fullToPath, fullFromPath);
} else {
Files.copy(fullFromPath, fullToPath, StandardCopyOption.REPLACE_EXISTING);
}
}
if (!hardLinkFile.delete()) {
throw new IOException("Failed to delete: " + hardLinkFile);
Expand All @@ -137,13 +144,28 @@ public static void createHardLinks(Path dbPath) throws IOException {
}
}

private static boolean isSamePartition(Path fromPathParent, Path toPathParent) throws IOException {
try {
if (fromPathParent == null || toPathParent == null) {
throw new IOException("From path: " + fromPathParent + " or To path: " + toPathParent + " is null");
}

FileStore fromPathStore = Files.getFileStore(fromPathParent);
FileStore toPathStore = Files.getFileStore(toPathParent);
return fromPathStore.equals(toPathStore);
} catch (IOException ex) {
throw new IOException("Failed to get the stores, fromPath: " + fromPathParent + " toPath: " + toPathParent, ex);
}
}

/**
* Link each of the files in oldDir to newDir.
* Link each of the files in oldDir to newDir if they are in the same partition
* else copy the files in oldDir to newDir.
*
* @param oldDir The dir to create links from.
* @param newDir The dir to create links to.
*/
public static void linkFiles(File oldDir, File newDir) throws IOException {
public static void linkFilesOrCopy(File oldDir, File newDir) throws IOException {
int truncateLength = oldDir.toString().length() + 1;
List<String> oldDirList;
try (Stream<Path> files = Files.walk(oldDir.toPath())) {
Expand All @@ -168,8 +190,10 @@ public static void linkFiles(File oldDir, File newDir) throws IOException {
if (!newFile.mkdirs()) {
throw new IOException("Directory create fails: " + newFile);
}
} else {
} else if (isSamePartition(newFile.toPath().getParent(), oldFile.toPath().getParent())) {
Files.createLink(newFile.toPath(), oldFile.toPath());
} else {
Files.copy(oldFile.toPath(), newFile.toPath(), StandardCopyOption.REPLACE_EXISTING);
}
}
}
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write a test case where we assert for the new functionality. (New file is in a different partition than the old file)

Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add similar test case for copy case, when both files are in different partitions. You can achieve this by mocking the layer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mocking this isSamePartition function should help. From what I know you cannot mock Files interface through mockito but PowerMockito can do it.


// Expected files/links should exist now.
assertTrue(tree2.exists());
Expand Down