Skip to content

Commit

Permalink
HDDS-12064. Optimize bootstrap logic to reduce loop while checking fi…
Browse files Browse the repository at this point in the history
…le links (#7676)

* HDDS-12064. Optimize bootstrap logic to reduce loop while checking file links

Change-Id: I6871db471adc1790ac3a0ff295a4db6eeb7608ad

* HDDS-12064. Fix findbugs

Change-Id: If6f300d6068c4be2c8da99fdef3ae8495680d5ea

* HDDS-12064. Address review comments

Change-Id: Ic2b623cdb5ea6cbdcfad2b82ebb11bad62caa6d2

* HDDS-12064. Address review comments

Change-Id: I03befbcab5d08add580c44cc7ee52dbfaeb101ba
swamirishi authored Jan 15, 2025
1 parent 85e7521 commit 6c41a9a
Showing 2 changed files with 63 additions and 43 deletions.
Original file line number Diff line number Diff line change
@@ -51,6 +51,7 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
@@ -149,7 +150,7 @@ public void writeDbDataToStream(DBCheckpoint checkpoint,
// the same. For synchronization purposes, some files are copied
// to a temp directory on the leader. In those cases the source
// and dest won't be the same.
Map<Path, Path> copyFiles = new HashMap<>();
Map<String, Map<Path, Path>> copyFiles = new HashMap<>();

// Map of link to path.
Map<Path, Path> hardLinkFiles = new HashMap<>();
@@ -168,12 +169,14 @@ public void writeDbDataToStream(DBCheckpoint checkpoint,
differ.getCompactionLogDir());

// Files to be excluded from tarball
Map<Path, Path> sstFilesToExclude = normalizeExcludeList(toExcludeList,
Map<String, Map<Path, Path>> sstFilesToExclude = normalizeExcludeList(toExcludeList,
checkpoint.getCheckpointLocation(), sstBackupDir);
boolean completed = getFilesForArchive(checkpoint, copyFiles,
hardLinkFiles, sstFilesToExclude, includeSnapshotData(request),
excludedList, sstBackupDir, compactionLogDir);
writeFilesToArchive(copyFiles, hardLinkFiles, archiveOutputStream,
Map<Path, Path> flatCopyFiles = copyFiles.values().stream().flatMap(map -> map.entrySet().stream())
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
writeFilesToArchive(flatCopyFiles, hardLinkFiles, archiveOutputStream,
completed, checkpoint.getCheckpointLocation());
} catch (Exception e) {
LOG.error("got exception writing to archive " + e);
@@ -194,14 +197,19 @@ hardLinkFiles, sstFilesToExclude, includeSnapshotData(request),
* include sst files.)
*/
@VisibleForTesting
public static Map<Path, Path> normalizeExcludeList(
public static Map<String, Map<Path, Path>> normalizeExcludeList(
List<String> toExcludeList,
Path checkpointLocation,
DirectoryData sstBackupDir) {
Map<Path, Path> paths = new HashMap<>();
Map<String, Map<Path, Path>> paths = new HashMap<>();
Path metaDirPath = getMetaDirPath(checkpointLocation);
for (String s : toExcludeList) {
Path fileName = Paths.get(s).getFileName();
if (fileName == null) {
continue;
}
Path destPath = Paths.get(metaDirPath.toString(), s);
Map<Path, Path> fileMap = paths.computeIfAbsent(fileName.toString(), (k) -> new HashMap<>());
if (destPath.toString().startsWith(
sstBackupDir.getOriginalDir().toString())) {
// The source of the sstBackupDir is a temporary directory and needs
@@ -210,12 +218,12 @@ public static Map<Path, Path> normalizeExcludeList(
sstBackupDir.getOriginalDir().toString().length() + 1;
Path srcPath = Paths.get(sstBackupDir.getTmpDir().toString(),
truncateFileName(truncateLength, destPath));
paths.put(srcPath, destPath);
fileMap.put(srcPath, destPath);
} else if (!s.startsWith(OM_SNAPSHOT_DIR)) {
Path fixedPath = Paths.get(checkpointLocation.toString(), s);
paths.put(fixedPath, fixedPath);
fileMap.put(fixedPath, fixedPath);
} else {
paths.put(destPath, destPath);
fileMap.put(destPath, destPath);
}
}
return paths;
@@ -266,9 +274,9 @@ public File getTmpDir() {

@SuppressWarnings("checkstyle:ParameterNumber")
private boolean getFilesForArchive(DBCheckpoint checkpoint,
Map<Path, Path> copyFiles,
Map<String, Map<Path, Path>> copyFiles,
Map<Path, Path> hardLinkFiles,
Map<Path, Path> sstFilesToExclude,
Map<String, Map<Path, Path>> sstFilesToExclude,
boolean includeSnapshotData,
List<String> excluded,
DirectoryData sstBackupDir,
@@ -360,9 +368,9 @@ private void waitForDirToExist(Path dir) throws IOException {
}

@SuppressWarnings("checkstyle:ParameterNumber")
private boolean processDir(Path dir, Map<Path, Path> copyFiles,
private boolean processDir(Path dir, Map<String, Map<Path, Path>> copyFiles,
Map<Path, Path> hardLinkFiles,
Map<Path, Path> sstFilesToExclude,
Map<String, Map<Path, Path>> sstFilesToExclude,
Set<Path> snapshotPaths,
List<String> excluded,
AtomicLong copySize,
@@ -437,9 +445,9 @@ private boolean processDir(Path dir, Map<Path, Path> copyFiles,
* @param excluded The list of db files that actually were excluded.
*/
@VisibleForTesting
public static long processFile(Path file, Map<Path, Path> copyFiles,
public static long processFile(Path file, Map<String, Map<Path, Path>> copyFiles,
Map<Path, Path> hardLinkFiles,
Map<Path, Path> sstFilesToExclude,
Map<String, Map<Path, Path>> sstFilesToExclude,
List<String> excluded,
Path destDir)
throws IOException {
@@ -458,7 +466,7 @@ public static long processFile(Path file, Map<Path, Path> copyFiles,
if (destDir != null) {
destFile = Paths.get(destDir.toString(), fileName);
}
if (sstFilesToExclude.containsKey(file)) {
if (sstFilesToExclude.getOrDefault(fileNamePath.toString(), Collections.emptyMap()).containsKey(file)) {
excluded.add(destFile.toString());
} else {
if (fileName.endsWith(ROCKSDB_SST_SUFFIX)) {
@@ -473,13 +481,13 @@ public static long processFile(Path file, Map<Path, Path> copyFiles,
hardLinkFiles.put(destFile, linkPath);
} else {
// Add to tarball.
copyFiles.put(file, destFile);
copyFiles.computeIfAbsent(fileNamePath.toString(), (k) -> new HashMap<>()).put(file, destFile);
fileSize = Files.size(file);
}
}
} else {
// Not sst file.
copyFiles.put(file, destFile);
copyFiles.computeIfAbsent(fileNamePath.toString(), (k) -> new HashMap<>()).put(file, destFile);
}
}
return fileSize;
@@ -494,7 +502,7 @@ public static long processFile(Path file, Map<Path, Path> copyFiles,
* @param file - File to be linked.
* @return dest path of file to be linked to.
*/
private static Path findLinkPath(Map<Path, Path> files, Path file)
private static Path findLinkPath(Map<String, Map<Path, Path>> files, Path file)
throws IOException {
// findbugs nonsense
Path fileNamePath = file.getFileName();
@@ -503,7 +511,7 @@ private static Path findLinkPath(Map<Path, Path> files, Path file)
}
String fileName = fileNamePath.toString();

for (Map.Entry<Path, Path> entry: files.entrySet()) {
for (Map.Entry<Path, Path> entry : files.getOrDefault(fileName, Collections.emptyMap()).entrySet()) {
Path srcPath = entry.getKey();
Path destPath = entry.getValue();
if (!srcPath.toString().endsWith(fileName)) {
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@

package org.apache.hadoop.ozone.om;

import com.google.common.collect.ImmutableMap;
import org.apache.hadoop.hdds.HddsConfigKeys;
import org.apache.hadoop.hdds.conf.OzoneConfiguration;
import org.apache.hadoop.hdds.scm.HddsWhiteboxTestUtils;
@@ -72,6 +73,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.Mockito.mock;
@@ -371,17 +373,17 @@ public void testExcludeUtilities() throws IOException {
"backup.sst");
truncateLength = leaderDir.toString().length() + 1;
existingSstList.add(truncateFileName(truncateLength, destSstBackup));
Map<Path, Path> normalizedMap =
Map<String, Map<Path, Path>> normalizedMap =
OMDBCheckpointServlet.normalizeExcludeList(existingSstList,
leaderCheckpointDir.toPath(), sstBackupDir);
Map<Path, Path> expectedMap = new TreeMap<>();
Map<String, Map<Path, Path>> expectedMap = new TreeMap<>();
Path s1 = Paths.get(leaderSnapDir1.toString(), "s1.sst");
Path noLink = Paths.get(leaderSnapDir2.toString(), "noLink.sst");
Path f1 = Paths.get(leaderCheckpointDir.toString(), "f1.sst");
expectedMap.put(s1, s1);
expectedMap.put(noLink, noLink);
expectedMap.put(f1, f1);
expectedMap.put(srcSstBackup, destSstBackup);
expectedMap.put("s1.sst", ImmutableMap.of(s1, s1));
expectedMap.put("noLink.sst", ImmutableMap.of(noLink, noLink));
expectedMap.put("f1.sst", ImmutableMap.of(f1, f1));
expectedMap.put("backup.sst", ImmutableMap.of(srcSstBackup, destSstBackup));
assertEquals(expectedMap, new TreeMap<>(normalizedMap));
}

@@ -396,11 +398,15 @@ void testProcessFileWithNullDestDirParameter(@TempDir File testDir) throws IOExc
assertTrue(new File(testDir, "snap2").mkdirs());
Path copyFile = Paths.get(testDir.toString(),
"snap1/copyfile.sst");
Path copyFileName = copyFile.getFileName();
assertNotNull(copyFileName);
Files.write(copyFile,
"dummyData".getBytes(StandardCharsets.UTF_8));
long expectedFileSize = Files.size(copyFile);
Path excludeFile = Paths.get(testDir.toString(),
"snap1/excludeFile.sst");
Path excludeFileName = excludeFile.getFileName();
assertNotNull(excludeFileName);
Files.write(excludeFile,
"dummyData".getBytes(StandardCharsets.UTF_8));
Path linkToExcludedFile = Paths.get(testDir.toString(),
@@ -418,10 +424,12 @@ void testProcessFileWithNullDestDirParameter(@TempDir File testDir) throws IOExc
Files.write(addNonSstToCopiedFiles,
"dummyData".getBytes(StandardCharsets.UTF_8));

Map<Path, Path> toExcludeFiles = new HashMap<>();
toExcludeFiles.put(excludeFile, excludeFile);
Map<Path, Path> copyFiles = new HashMap<>();
copyFiles.put(copyFile, copyFile);
Map<String, Map<Path, Path>> toExcludeFiles = new HashMap<>();
toExcludeFiles.computeIfAbsent(excludeFileName.toString(), (k) -> new HashMap<>()).put(excludeFile,
excludeFile);
Map<String, Map<Path, Path>> copyFiles = new HashMap<>();
copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile,
copyFile);
List<String> excluded = new ArrayList<>();
Map<Path, Path> hardLinkFiles = new HashMap<>();
long fileSize;
@@ -461,18 +469,18 @@ void testProcessFileWithNullDestDirParameter(@TempDir File testDir) throws IOExc
toExcludeFiles, excluded, null);
assertEquals(excluded.size(), 0);
assertEquals(copyFiles.size(), 2);
assertEquals(copyFiles.get(addToCopiedFiles), addToCopiedFiles);
assertEquals(copyFiles.get(addToCopiedFiles.getFileName().toString()).get(addToCopiedFiles), addToCopiedFiles);
assertEquals(fileSize, expectedFileSize);
copyFiles = new HashMap<>();
copyFiles.put(copyFile, copyFile);
copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile, copyFile);

// Confirm the addNonSstToCopiedFiles gets added to list of copied files
fileSize = processFile(addNonSstToCopiedFiles, copyFiles, hardLinkFiles,
toExcludeFiles, excluded, null);
assertEquals(excluded.size(), 0);
assertEquals(copyFiles.size(), 2);
assertEquals(fileSize, 0);
assertEquals(copyFiles.get(addNonSstToCopiedFiles),
assertEquals(copyFiles.get(addNonSstToCopiedFiles.getFileName().toString()).get(addNonSstToCopiedFiles),
addNonSstToCopiedFiles);
}

@@ -492,6 +500,8 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti
// Create test files.
Path copyFile = Paths.get(testDir.toString(),
"snap1/copyfile.sst");
Path copyFileName = copyFile.getFileName();
assertNotNull(copyFileName);
Path destCopyFile = Paths.get(destDir.toString(),
"snap1/copyfile.sst");
Files.write(copyFile,
@@ -505,6 +515,8 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti
long expectedFileSize = Files.size(copyFile);
Path excludeFile = Paths.get(testDir.toString(),
"snap1/excludeFile.sst");
Path excludeFileName = excludeFile.getFileName();
assertNotNull(excludeFileName);
Path destExcludeFile = Paths.get(destDir.toString(),
"snap1/excludeFile.sst");
Files.write(excludeFile,
@@ -539,10 +551,10 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti
"dummyData".getBytes(StandardCharsets.UTF_8));

// Create test data structures.
Map<Path, Path> toExcludeFiles = new HashMap<>();
toExcludeFiles.put(excludeFile, destExcludeFile);
Map<Path, Path> copyFiles = new HashMap<>();
copyFiles.put(copyFile, destCopyFile);
Map<String, Map<Path, Path>> toExcludeFiles = new HashMap<>();
toExcludeFiles.put(excludeFileName.toString(), ImmutableMap.of(excludeFile, destExcludeFile));
Map<String, Map<Path, Path>> copyFiles = new HashMap<>();
copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile);
List<String> excluded = new ArrayList<>();
Map<Path, Path> hardLinkFiles = new HashMap<>();
long fileSize;
@@ -575,11 +587,11 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti
assertEquals(excluded.size(), 0);
assertEquals(copyFiles.size(), 2);
assertEquals(hardLinkFiles.size(), 0);
assertEquals(copyFiles.get(sameNameAsExcludeFile),
assertEquals(copyFiles.get(sameNameAsExcludeFile.getFileName().toString()).get(sameNameAsExcludeFile),
destSameNameAsExcludeFile);
assertEquals(fileSize, expectedFileSize);
copyFiles = new HashMap<>();
copyFiles.put(copyFile, destCopyFile);
copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile);


// Confirm the file with same name as copy file gets copied.
@@ -588,11 +600,11 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti
assertEquals(excluded.size(), 0);
assertEquals(copyFiles.size(), 2);
assertEquals(hardLinkFiles.size(), 0);
assertEquals(copyFiles.get(sameNameAsCopyFile),
assertEquals(copyFiles.get(sameNameAsCopyFile.getFileName().toString()).get(sameNameAsCopyFile),
destSameNameAsCopyFile);
assertEquals(fileSize, expectedFileSize);
copyFiles = new HashMap<>();
copyFiles.put(copyFile, destCopyFile);
copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile);


// Confirm the linkToCopiedFile gets added as a link.
@@ -611,19 +623,19 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti
toExcludeFiles, excluded, destAddToCopiedFiles.getParent());
assertEquals(excluded.size(), 0);
assertEquals(copyFiles.size(), 2);
assertEquals(copyFiles.get(addToCopiedFiles),
assertEquals(copyFiles.get(addToCopiedFiles.getFileName().toString()).get(addToCopiedFiles),
destAddToCopiedFiles);
assertEquals(fileSize, expectedFileSize);
copyFiles = new HashMap<>();
copyFiles.put(copyFile, destCopyFile);
copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile);

// Confirm the addNonSstToCopiedFiles gets added to list of copied files
fileSize = processFile(addNonSstToCopiedFiles, copyFiles, hardLinkFiles,
toExcludeFiles, excluded, destAddNonSstToCopiedFiles.getParent());
assertEquals(excluded.size(), 0);
assertEquals(copyFiles.size(), 2);
assertEquals(fileSize, 0);
assertEquals(copyFiles.get(addNonSstToCopiedFiles),
assertEquals(copyFiles.get(addNonSstToCopiedFiles.getFileName().toString()).get(addNonSstToCopiedFiles),
destAddNonSstToCopiedFiles);
}

0 comments on commit 6c41a9a

Please sign in to comment.