Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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 @@ -3711,7 +3711,7 @@ public synchronized TermIndex installSnapshotFromLeader(String leaderId) {
TermIndex termIndex = null;
try {
// Install hard links.
OmSnapshotUtils.createHardLinks(omDBCheckpoint.getCheckpointLocation());
OmSnapshotUtils.createHardLinksOrCopyFromHardlinkFile(omDBCheckpoint.getCheckpointLocation());
termIndex = installCheckpoint(leaderId, omDBCheckpoint);
} catch (Exception ex) {
LOG.error("Failed to install snapshot from Leader OM.", ex);
Expand Down 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 @@ -106,7 +108,7 @@ public static Path createHardLinkList(int truncateLength,
*
* @param dbPath Path to db to have links created.
*/
public static void createHardLinks(Path dbPath) throws IOException {
public static void createHardLinksOrCopyFromHardlinkFile(Path dbPath) throws IOException {
File hardLinkFile =
new File(dbPath.toString(), OmSnapshotManager.OM_HARDLINK_FILE);
if (hardLinkFile.exists()) {
Expand All @@ -128,7 +130,7 @@ public static void createHardLinks(Path dbPath) throws IOException {
"Failed to create directory: " + parent.toString());
}
}
Files.createLink(fullToPath, fullFromPath);
createHardLinksOrCopy(fullFromPath, fullToPath);
}
if (!hardLinkFile.delete()) {
throw new IOException("Failed to delete: " + hardLinkFile);
Expand All @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is replace existing correct? Can this kind of a situation occur where we are re-copying stuff? We shouldn't inadvertently delete data.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

The file shouldn't exist at all, I'm doing a replace just as a precaution. I'm following the normal OM bootstrap process, We don't do a checksum check for that either.

}
}

private static boolean isSamePartition(Path fromPathParent, Path toPathParent) throws IOException {
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);
}

/**
* 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 @@ -169,7 +189,7 @@ public static void linkFiles(File oldDir, File newDir) throws IOException {
throw new IOException("Directory create fails: " + newFile);
}
} else {
Files.createLink(newFile.toPath(), oldFile.toPath());
createHardLinksOrCopy(oldFile.toPath(), newFile.toPath());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ public void testHardLinkCreation() throws IOException {
File s1FileLink = new File(followerSnapDir2, "s1.sst");

// Create links on the follower from list.
OmSnapshotUtils.createHardLinks(candidateDir.toPath());
OmSnapshotUtils.createHardLinksOrCopyFromHardlinkFile(candidateDir.toPath());

// Confirm expected follower links.
assertTrue(s1FileLink.exists());
Expand Down
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