From 0ea3331446065b000c0ad452aff72ad8bc23a68c Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Fri, 10 Jan 2025 02:35:56 -0800 Subject: [PATCH 1/4] HDDS-12064. Optimize bootstrap logic to reduce loop while checking file links Change-Id: I6871db471adc1790ac3a0ff295a4db6eeb7608ad --- .../ozone/om/OMDBCheckpointServlet.java | 43 ++++++++-------- .../ozone/om/TestOmSnapshotManager.java | 50 ++++++++++--------- 2 files changed, 49 insertions(+), 44 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 c8237b79673..015b1d501b7 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.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 copyFiles = new HashMap<>(); + Map> copyFiles = new HashMap<>(); // Map of link to path. Map hardLinkFiles = new HashMap<>(); @@ -168,13 +169,14 @@ public void writeDbDataToStream(DBCheckpoint checkpoint, differ.getCompactionLogDir()); // Files to be excluded from tarball - Map sstFilesToExclude = normalizeExcludeList(toExcludeList, + Map> sstFilesToExclude = normalizeExcludeList(toExcludeList, checkpoint.getCheckpointLocation(), sstBackupDir); boolean completed = getFilesForArchive(checkpoint, copyFiles, hardLinkFiles, sstFilesToExclude, includeSnapshotData(request), excludedList, sstBackupDir, compactionLogDir); - writeFilesToArchive(copyFiles, hardLinkFiles, archiveOutputStream, - completed, checkpoint.getCheckpointLocation()); + writeFilesToArchive(copyFiles.values().stream().flatMap(map -> map.entrySet().stream()) + .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)), + hardLinkFiles, archiveOutputStream, completed, checkpoint.getCheckpointLocation()); } catch (Exception e) { LOG.error("got exception writing to archive " + e); throw e; @@ -194,11 +196,11 @@ hardLinkFiles, sstFilesToExclude, includeSnapshotData(request), * include sst files.) */ @VisibleForTesting - public static Map normalizeExcludeList( + public static Map> normalizeExcludeList( List toExcludeList, Path checkpointLocation, DirectoryData sstBackupDir) { - Map paths = new HashMap<>(); + Map> paths = new HashMap<>(); Path metaDirPath = getMetaDirPath(checkpointLocation); for (String s : toExcludeList) { Path destPath = Paths.get(metaDirPath.toString(), s); @@ -210,12 +212,12 @@ public static Map normalizeExcludeList( sstBackupDir.getOriginalDir().toString().length() + 1; Path srcPath = Paths.get(sstBackupDir.getTmpDir().toString(), truncateFileName(truncateLength, destPath)); - paths.put(srcPath, destPath); + paths.computeIfAbsent(srcPath.getFileName().toString(), (k) -> new HashMap<>()).put(srcPath, destPath); } else if (!s.startsWith(OM_SNAPSHOT_DIR)) { Path fixedPath = Paths.get(checkpointLocation.toString(), s); - paths.put(fixedPath, fixedPath); + paths.computeIfAbsent(fixedPath.getFileName().toString(), (k) -> new HashMap<>()).put(fixedPath, fixedPath); } else { - paths.put(destPath, destPath); + paths.computeIfAbsent(destPath.getFileName().toString(), (k) -> new HashMap<>()).put(destPath, destPath); } } return paths; @@ -266,9 +268,9 @@ public File getTmpDir() { @SuppressWarnings("checkstyle:ParameterNumber") private boolean getFilesForArchive(DBCheckpoint checkpoint, - Map copyFiles, + Map> copyFiles, Map hardLinkFiles, - Map sstFilesToExclude, + Map> sstFilesToExclude, boolean includeSnapshotData, List excluded, DirectoryData sstBackupDir, @@ -360,9 +362,9 @@ private void waitForDirToExist(Path dir) throws IOException { } @SuppressWarnings("checkstyle:ParameterNumber") - private boolean processDir(Path dir, Map copyFiles, + private boolean processDir(Path dir, Map> copyFiles, Map hardLinkFiles, - Map sstFilesToExclude, + Map> sstFilesToExclude, Set snapshotPaths, List excluded, AtomicLong copySize, @@ -437,9 +439,9 @@ private boolean processDir(Path dir, Map copyFiles, * @param excluded The list of db files that actually were excluded. */ @VisibleForTesting - public static long processFile(Path file, Map copyFiles, + public static long processFile(Path file, Map> copyFiles, Map hardLinkFiles, - Map sstFilesToExclude, + Map> sstFilesToExclude, List excluded, Path destDir) throws IOException { @@ -458,7 +460,8 @@ public static long processFile(Path file, Map copyFiles, if (destDir != null) { destFile = Paths.get(destDir.toString(), fileName); } - if (sstFilesToExclude.containsKey(file)) { + if (sstFilesToExclude.getOrDefault(file.getFileName().getFileName().toString(), Collections.emptyMap()) + .containsKey(file)) { excluded.add(destFile.toString()); } else { if (fileName.endsWith(ROCKSDB_SST_SUFFIX)) { @@ -473,13 +476,13 @@ public static long processFile(Path file, Map copyFiles, hardLinkFiles.put(destFile, linkPath); } else { // Add to tarball. - copyFiles.put(file, destFile); + copyFiles.computeIfAbsent(file.getFileName().toString(), (k) -> new HashMap<>()).put(file, destFile); fileSize = Files.size(file); } } } else { // Not sst file. - copyFiles.put(file, destFile); + copyFiles.computeIfAbsent(file.getFileName().toString(), (k) -> new HashMap<>()).put(file, destFile); } } return fileSize; @@ -494,7 +497,7 @@ public static long processFile(Path file, Map copyFiles, * @param file - File to be linked. * @return dest path of file to be linked to. */ - private static Path findLinkPath(Map files, Path file) + private static Path findLinkPath(Map> files, Path file) throws IOException { // findbugs nonsense Path fileNamePath = file.getFileName(); @@ -503,7 +506,7 @@ private static Path findLinkPath(Map files, Path file) } String fileName = fileNamePath.toString(); - for (Map.Entry entry: files.entrySet()) { + for (Map.Entry 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 1d00ec614cd..21904d7b459 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; @@ -371,17 +372,17 @@ public void testExcludeUtilities() throws IOException { "backup.sst"); truncateLength = leaderDir.toString().length() + 1; existingSstList.add(truncateFileName(truncateLength, destSstBackup)); - Map normalizedMap = + Map> normalizedMap = OMDBCheckpointServlet.normalizeExcludeList(existingSstList, leaderCheckpointDir.toPath(), sstBackupDir); - Map expectedMap = new TreeMap<>(); + Map> 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)); } @@ -418,10 +419,11 @@ void testProcessFileWithNullDestDirParameter(@TempDir File testDir) throws IOExc Files.write(addNonSstToCopiedFiles, "dummyData".getBytes(StandardCharsets.UTF_8)); - Map toExcludeFiles = new HashMap<>(); - toExcludeFiles.put(excludeFile, excludeFile); - Map copyFiles = new HashMap<>(); - copyFiles.put(copyFile, copyFile); + Map> toExcludeFiles = new HashMap<>(); + toExcludeFiles.computeIfAbsent(excludeFile.getFileName().toString(), (k) -> new HashMap<>()).put(excludeFile, + excludeFile); + Map> copyFiles = new HashMap<>(); + copyFiles.computeIfAbsent(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, copyFile); List excluded = new ArrayList<>(); Map hardLinkFiles = new HashMap<>(); long fileSize; @@ -461,10 +463,10 @@ 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(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, copyFile); // Confirm the addNonSstToCopiedFiles gets added to list of copied files fileSize = processFile(addNonSstToCopiedFiles, copyFiles, hardLinkFiles, @@ -472,7 +474,7 @@ void testProcessFileWithNullDestDirParameter(@TempDir File testDir) throws IOExc assertEquals(excluded.size(), 0); assertEquals(copyFiles.size(), 2); assertEquals(fileSize, 0); - assertEquals(copyFiles.get(addNonSstToCopiedFiles), + assertEquals(copyFiles.get(addNonSstToCopiedFiles.getFileName().toString()).get(addNonSstToCopiedFiles), addNonSstToCopiedFiles); } @@ -539,10 +541,10 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti "dummyData".getBytes(StandardCharsets.UTF_8)); // Create test data structures. - Map toExcludeFiles = new HashMap<>(); - toExcludeFiles.put(excludeFile, destExcludeFile); - Map copyFiles = new HashMap<>(); - copyFiles.put(copyFile, destCopyFile); + Map> toExcludeFiles = new HashMap<>(); + toExcludeFiles.put(excludeFile.getFileName().toString(), ImmutableMap.of(excludeFile, destExcludeFile)); + Map> copyFiles = new HashMap<>(); + copyFiles.computeIfAbsent(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); List excluded = new ArrayList<>(); Map hardLinkFiles = new HashMap<>(); long fileSize; @@ -575,11 +577,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(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); // Confirm the file with same name as copy file gets copied. @@ -588,11 +590,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(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); // Confirm the linkToCopiedFile gets added as a link. @@ -611,11 +613,11 @@ 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(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); // Confirm the addNonSstToCopiedFiles gets added to list of copied files fileSize = processFile(addNonSstToCopiedFiles, copyFiles, hardLinkFiles, @@ -623,7 +625,7 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti assertEquals(excluded.size(), 0); assertEquals(copyFiles.size(), 2); assertEquals(fileSize, 0); - assertEquals(copyFiles.get(addNonSstToCopiedFiles), + assertEquals(copyFiles.get(addNonSstToCopiedFiles.getFileName().toString()).get(addNonSstToCopiedFiles), destAddNonSstToCopiedFiles); } From c1ade201bb9add4cc59308b417faf8b66f7be6b8 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Fri, 10 Jan 2025 09:46:52 -0800 Subject: [PATCH 2/4] HDDS-12064. Fix findbugs Change-Id: If6f300d6068c4be2c8da99fdef3ae8495680d5ea --- .../ozone/om/OMDBCheckpointServlet.java | 17 +++++++----- .../ozone/om/TestOmSnapshotManager.java | 26 +++++++++++++------ 2 files changed, 28 insertions(+), 15 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 015b1d501b7..20e63ba3a8f 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 @@ -203,6 +203,10 @@ public static Map> normalizeExcludeList( Map> 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); if (destPath.toString().startsWith( sstBackupDir.getOriginalDir().toString())) { @@ -212,12 +216,12 @@ public static Map> normalizeExcludeList( sstBackupDir.getOriginalDir().toString().length() + 1; Path srcPath = Paths.get(sstBackupDir.getTmpDir().toString(), truncateFileName(truncateLength, destPath)); - paths.computeIfAbsent(srcPath.getFileName().toString(), (k) -> new HashMap<>()).put(srcPath, destPath); + paths.computeIfAbsent(fileName.toString(), (k) -> new HashMap<>()).put(srcPath, destPath); } else if (!s.startsWith(OM_SNAPSHOT_DIR)) { Path fixedPath = Paths.get(checkpointLocation.toString(), s); - paths.computeIfAbsent(fixedPath.getFileName().toString(), (k) -> new HashMap<>()).put(fixedPath, fixedPath); + paths.computeIfAbsent(fileName.toString(), (k) -> new HashMap<>()).put(fixedPath, fixedPath); } else { - paths.computeIfAbsent(destPath.getFileName().toString(), (k) -> new HashMap<>()).put(destPath, destPath); + paths.computeIfAbsent(fileName.toString(), (k) -> new HashMap<>()).put(destPath, destPath); } } return paths; @@ -460,8 +464,7 @@ public static long processFile(Path file, Map> copyFiles if (destDir != null) { destFile = Paths.get(destDir.toString(), fileName); } - if (sstFilesToExclude.getOrDefault(file.getFileName().getFileName().toString(), Collections.emptyMap()) - .containsKey(file)) { + if (sstFilesToExclude.getOrDefault(fileNamePath.toString(), Collections.emptyMap()).containsKey(file)) { excluded.add(destFile.toString()); } else { if (fileName.endsWith(ROCKSDB_SST_SUFFIX)) { @@ -476,13 +479,13 @@ public static long processFile(Path file, Map> copyFiles hardLinkFiles.put(destFile, linkPath); } else { // Add to tarball. - copyFiles.computeIfAbsent(file.getFileName().toString(), (k) -> new HashMap<>()).put(file, destFile); + copyFiles.computeIfAbsent(fileNamePath.toString(), (k) -> new HashMap<>()).put(file, destFile); fileSize = Files.size(file); } } } else { // Not sst file. - copyFiles.computeIfAbsent(file.getFileName().toString(), (k) -> new HashMap<>()).put(file, destFile); + copyFiles.computeIfAbsent(fileNamePath.toString(), (k) -> new HashMap<>()).put(file, destFile); } } return fileSize; 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 21904d7b459..3a98b4f6298 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 @@ -73,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; @@ -397,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(), @@ -420,10 +425,11 @@ void testProcessFileWithNullDestDirParameter(@TempDir File testDir) throws IOExc "dummyData".getBytes(StandardCharsets.UTF_8)); Map> toExcludeFiles = new HashMap<>(); - toExcludeFiles.computeIfAbsent(excludeFile.getFileName().toString(), (k) -> new HashMap<>()).put(excludeFile, + toExcludeFiles.computeIfAbsent(excludeFileName.toString(), (k) -> new HashMap<>()).put(excludeFile, excludeFile); Map> copyFiles = new HashMap<>(); - copyFiles.computeIfAbsent(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, copyFile); + copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile, + copyFile); List excluded = new ArrayList<>(); Map hardLinkFiles = new HashMap<>(); long fileSize; @@ -466,7 +472,7 @@ void testProcessFileWithNullDestDirParameter(@TempDir File testDir) throws IOExc assertEquals(copyFiles.get(addToCopiedFiles.getFileName().toString()).get(addToCopiedFiles), addToCopiedFiles); assertEquals(fileSize, expectedFileSize); copyFiles = new HashMap<>(); - copyFiles.computeIfAbsent(copyFile.getFileName().toString(), (k) -> new HashMap<>()).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, @@ -494,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, @@ -507,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, @@ -542,9 +552,9 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti // Create test data structures. Map> toExcludeFiles = new HashMap<>(); - toExcludeFiles.put(excludeFile.getFileName().toString(), ImmutableMap.of(excludeFile, destExcludeFile)); + toExcludeFiles.put(excludeFileName.toString(), ImmutableMap.of(excludeFile, destExcludeFile)); Map> copyFiles = new HashMap<>(); - copyFiles.computeIfAbsent(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); + copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); List excluded = new ArrayList<>(); Map hardLinkFiles = new HashMap<>(); long fileSize; @@ -581,7 +591,7 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti destSameNameAsExcludeFile); assertEquals(fileSize, expectedFileSize); copyFiles = new HashMap<>(); - copyFiles.computeIfAbsent(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); + copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); // Confirm the file with same name as copy file gets copied. @@ -594,7 +604,7 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti destSameNameAsCopyFile); assertEquals(fileSize, expectedFileSize); copyFiles = new HashMap<>(); - copyFiles.computeIfAbsent(copyFile.getFileName().toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); + copyFiles.computeIfAbsent(copyFileName.toString(), (k) -> new HashMap<>()).put(copyFile, destCopyFile); // Confirm the linkToCopiedFile gets added as a link. @@ -617,7 +627,7 @@ void testProcessFileWithDestDirParameter(@TempDir File testDir) throws IOExcepti destAddToCopiedFiles); assertEquals(fileSize, expectedFileSize); copyFiles = new HashMap<>(); - copyFiles.computeIfAbsent(copyFile.getFileName().toString(), (k) -> new HashMap<>()).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, From 111dcad2ad4cea74cc9325e420c1e257241f1322 Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 14 Jan 2025 00:27:58 -0800 Subject: [PATCH 3/4] HDDS-12064. Address review comments Change-Id: Ic2b623cdb5ea6cbdcfad2b82ebb11bad62caa6d2 --- .../hadoop/ozone/om/OMDBCheckpointServlet.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 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 20e63ba3a8f..6ab17cdd9e9 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 @@ -174,9 +174,10 @@ public void writeDbDataToStream(DBCheckpoint checkpoint, boolean completed = getFilesForArchive(checkpoint, copyFiles, hardLinkFiles, sstFilesToExclude, includeSnapshotData(request), excludedList, sstBackupDir, compactionLogDir); - writeFilesToArchive(copyFiles.values().stream().flatMap(map -> map.entrySet().stream()) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)), - hardLinkFiles, archiveOutputStream, completed, checkpoint.getCheckpointLocation()); + Map 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); throw e; @@ -208,6 +209,7 @@ public static Map> normalizeExcludeList( continue; } Path destPath = Paths.get(metaDirPath.toString(), s); + 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 @@ -216,12 +218,12 @@ public static Map> normalizeExcludeList( sstBackupDir.getOriginalDir().toString().length() + 1; Path srcPath = Paths.get(sstBackupDir.getTmpDir().toString(), truncateFileName(truncateLength, destPath)); - paths.computeIfAbsent(fileName.toString(), (k) -> new HashMap<>()).put(srcPath, destPath); + paths.get(fileName.toString()).put(srcPath, destPath); } else if (!s.startsWith(OM_SNAPSHOT_DIR)) { Path fixedPath = Paths.get(checkpointLocation.toString(), s); - paths.computeIfAbsent(fileName.toString(), (k) -> new HashMap<>()).put(fixedPath, fixedPath); + paths.get(fileName.toString()).put(fixedPath, fixedPath); } else { - paths.computeIfAbsent(fileName.toString(), (k) -> new HashMap<>()).put(destPath, destPath); + paths.get(fileName.toString()).put(destPath, destPath); } } return paths; From 01ab2d845197f117f6700e82148212965cb8502e Mon Sep 17 00:00:00 2001 From: Swaminathan Balachandran Date: Tue, 14 Jan 2025 07:49:40 -0800 Subject: [PATCH 4/4] HDDS-12064. Address review comments Change-Id: I03befbcab5d08add580c44cc7ee52dbfaeb101ba --- .../org/apache/hadoop/ozone/om/OMDBCheckpointServlet.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 6ab17cdd9e9..ee8633ae3f1 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 @@ -209,7 +209,7 @@ public static Map> normalizeExcludeList( continue; } Path destPath = Paths.get(metaDirPath.toString(), s); - paths.computeIfAbsent(fileName.toString(), (k) -> new HashMap<>()); + Map 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 @@ -218,12 +218,12 @@ public static Map> normalizeExcludeList( sstBackupDir.getOriginalDir().toString().length() + 1; Path srcPath = Paths.get(sstBackupDir.getTmpDir().toString(), truncateFileName(truncateLength, destPath)); - paths.get(fileName.toString()).put(srcPath, destPath); + fileMap.put(srcPath, destPath); } else if (!s.startsWith(OM_SNAPSHOT_DIR)) { Path fixedPath = Paths.get(checkpointLocation.toString(), s); - paths.get(fileName.toString()).put(fixedPath, fixedPath); + fileMap.put(fixedPath, fixedPath); } else { - paths.get(fileName.toString()).put(destPath, destPath); + fileMap.put(destPath, destPath); } } return paths;