This is an automated email from the ASF dual-hosted git repository.
swamirishi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 6c41a9aa3c HDDS-12064. Optimize bootstrap logic to reduce loop while
checking file links (#7676)
6c41a9aa3c is described below
commit 6c41a9aa3c35a519f1c536e18ce15b6becf2891b
Author: Swaminathan Balachandran <[email protected]>
AuthorDate: Wed Jan 15 11:12:27 2025 -0800
HDDS-12064. Optimize bootstrap logic to reduce loop while checking file
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
---
.../hadoop/ozone/om/OMDBCheckpointServlet.java | 46 ++++++++++-------
.../hadoop/ozone/om/TestOmSnapshotManager.java | 60 +++++++++++++---------
2 files changed, 63 insertions(+), 43 deletions(-)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java
index c8237b7967..ee8633ae3f 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java
@@ -51,6 +51,7 @@ import java.nio.file.Files;
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 class OMDBCheckpointServlet extends
DBCheckpointServlet {
// 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 class OMDBCheckpointServlet extends
DBCheckpointServlet {
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 @@ public class OMDBCheckpointServlet extends
DBCheckpointServlet {
* 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 class OMDBCheckpointServlet extends
DBCheckpointServlet {
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 class OMDBCheckpointServlet extends
DBCheckpointServlet {
@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 @@ public class OMDBCheckpointServlet extends
DBCheckpointServlet {
}
@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 @@ public class OMDBCheckpointServlet extends
DBCheckpointServlet {
* @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 class OMDBCheckpointServlet extends
DBCheckpointServlet {
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 class OMDBCheckpointServlet extends
DBCheckpointServlet {
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 class OMDBCheckpointServlet extends
DBCheckpointServlet {
* @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 @@ public class OMDBCheckpointServlet extends
DBCheckpointServlet {
}
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)) {
diff --git
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
index 1d00ec614c..3a98b4f629 100644
---
a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
+++
b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
@@ -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.apache.hadoop.ozone.om.snapshot.OmSnapshotUtils.truncateFileNa
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 @@ class TestOmSnapshotManager {
"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 @@ class TestOmSnapshotManager {
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 @@ class TestOmSnapshotManager {
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,10 +469,10 @@ class TestOmSnapshotManager {
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,
@@ -472,7 +480,7 @@ class TestOmSnapshotManager {
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 @@ class TestOmSnapshotManager {
// 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 @@ class TestOmSnapshotManager {
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 @@ class TestOmSnapshotManager {
"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 @@ class TestOmSnapshotManager {
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 @@ class TestOmSnapshotManager {
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,11 +623,11 @@ class TestOmSnapshotManager {
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,
@@ -623,7 +635,7 @@ class TestOmSnapshotManager {
assertEquals(excluded.size(), 0);
assertEquals(copyFiles.size(), 2);
assertEquals(fileSize, 0);
- assertEquals(copyFiles.get(addNonSstToCopiedFiles),
+
assertEquals(copyFiles.get(addNonSstToCopiedFiles.getFileName().toString()).get(addNonSstToCopiedFiles),
destAddNonSstToCopiedFiles);
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
