From b9f4ccf1d7c69f2e0e0dbd72ece63bf42149a825 Mon Sep 17 00:00:00 2001 From: Sadanand Shenoy Date: Wed, 25 Jun 2025 23:21:09 +0530 Subject: [PATCH 1/4] HDDS-13228. Take snapshot cache lock during the last iteration of tarball transfer. --- .../org/apache/hadoop/ozone/OzoneConsts.java | 6 + .../hadoop/ozone/om/helpers/SnapshotInfo.java | 2 +- ...stOMDbCheckpointServletInodeBasedXfer.java | 178 +++++++++++++++--- .../OMDBCheckpointServletInodeBasedXfer.java | 14 ++ .../hadoop/ozone/om/OmSnapshotManager.java | 10 + .../hadoop/ozone/om/codec/OMDBDefinition.java | 10 + .../ozone/om/snapshot/SnapshotCache.java | 1 + 7 files changed, 196 insertions(+), 25 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index 6bee926336b8..7c9134da4c08 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -533,6 +533,12 @@ public final class OzoneConsts { */ public static final String ETAG = "ETag"; + /** + * A constant string used as a separator in various contexts within + * the OMDBCheckpoint functions. + */ + public static final String SEPARATOR = "-"; + private OzoneConsts() { // Never Constructed } diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java index 680e80bfd7a2..f74fbbadc19d 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java @@ -20,6 +20,7 @@ import static org.apache.hadoop.hdds.HddsUtils.fromProtobuf; import static org.apache.hadoop.hdds.HddsUtils.toProtobuf; import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX; +import static org.apache.hadoop.ozone.OzoneConsts.SEPARATOR; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -57,7 +58,6 @@ public final class SnapshotInfo implements Auditable, CopyObject { SnapshotInfo::getProtobuf, SnapshotInfo.class); - private static final String SEPARATOR = "-"; private static final long INVALID_TIMESTAMP = -1; private static final UUID INITIAL_SNAPSHOT_ID = UUID.randomUUID(); diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java index 37086b2c8404..3fd1907a108f 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java @@ -17,6 +17,7 @@ package org.apache.hadoop.ozone.om; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ACL_ENABLED; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS_WILDCARD; @@ -28,8 +29,10 @@ import static org.apache.hadoop.ozone.OzoneConsts.OZONE_DB_CHECKPOINT_REQUEST_FLUSH; 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.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyBoolean; import static org.mockito.Mockito.doCallRealMethod; @@ -54,6 +57,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; @@ -68,6 +72,7 @@ import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationFactor; import org.apache.hadoop.hdds.client.ReplicationType; +import org.apache.hadoop.hdds.client.StandaloneReplicationConfig; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.utils.IOUtils; import org.apache.hadoop.hdds.utils.db.DBCheckpoint; @@ -77,13 +82,22 @@ import org.apache.hadoop.ozone.TestDataUtil; import org.apache.hadoop.ozone.client.OzoneBucket; import org.apache.hadoop.ozone.client.OzoneClient; +import org.apache.hadoop.ozone.client.OzoneSnapshot; import org.apache.hadoop.ozone.lock.BootstrapStateHandler; +import org.apache.hadoop.ozone.om.codec.OMDBDefinition; +import org.apache.hadoop.ozone.om.helpers.OmKeyInfo; +import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo; import org.apache.hadoop.ozone.om.snapshot.OmSnapshotUtils; import org.apache.hadoop.security.UserGroupInformation; +import org.apache.ratis.util.function.UncheckedAutoCloseableSupplier; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.rocksdb.ColumnFamilyDescriptor; +import org.rocksdb.ColumnFamilyHandle; +import org.rocksdb.DBOptions; +import org.rocksdb.RocksDB; /** * Class used for testing the OM DB Checkpoint provider servlet using inode based transfer logic. @@ -106,6 +120,8 @@ public class TestOMDbCheckpointServletInodeBasedXfer { @BeforeEach void init() throws Exception { conf = new OzoneConfiguration(); + conf.setTimeDuration(OMConfigKeys.OZONE_OM_SNAPSHOT_CACHE_CLEANUP_SERVICE_RUN_INTERVAL, + 100, TimeUnit.MINUTES); } @AfterEach @@ -193,33 +209,12 @@ public void write(int b) throws IOException { @Test void testContentsOfTarballWithSnapshot() throws Exception { - setupCluster(); - setupMocks(); - when(requestMock.getParameter(OZONE_DB_CHECKPOINT_INCLUDE_SNAPSHOT_DATA)).thenReturn("true"); String volumeName = "vol" + RandomStringUtils.secure().nextNumeric(5); String bucketName = "buck" + RandomStringUtils.secure().nextNumeric(5); - // Create a "spy" dbstore keep track of the checkpoint. - writeData(volumeName, bucketName, true); - DBStore dbStore = om.getMetadataManager().getStore(); - DBStore spyDbStore = spy(dbStore); AtomicReference realCheckpoint = new AtomicReference<>(); - when(spyDbStore.getCheckpoint(true)).thenAnswer(b -> { - DBCheckpoint checkpoint = spy(dbStore.getCheckpoint(true)); - // Don't delete the checkpoint, because we need to compare it - // with the snapshot data. - doNothing().when(checkpoint).cleanupCheckpoint(); - realCheckpoint.set(checkpoint); - return checkpoint; - }); - // Init the mock with the spyDbstore - doCallRealMethod().when(omDbCheckpointServletMock).initialize(any(), any(), - eq(false), any(), any(), eq(false)); - omDbCheckpointServletMock.initialize(spyDbStore, om.getMetrics().getDBCheckpointMetrics(), - false, - om.getOmAdminUsernames(), om.getOmAdminGroups(), false); - + setupClusterAndMocks(volumeName, bucketName, realCheckpoint); + DBStore dbStore = om.getMetadataManager().getStore(); // Get the tarball. - when(responseMock.getOutputStream()).thenReturn(servletOutputStream); omDbCheckpointServletMock.doGet(requestMock, responseMock); String testDirName = folder.resolve("testDir").toString(); String newDbDirName = testDirName + OM_KEY_PREFIX + OM_DB_NAME; @@ -252,6 +247,8 @@ void testContentsOfTarballWithSnapshot() throws Exception { populateInodesOfFilesInDirectory(dbStore, Paths.get(snapshotPath), inodesFromOmDataDir, hardLinkMapFromOmData); } + populateInodesOfFilesInDirectory(dbStore, Paths.get(dbStore.getRocksDBCheckpointDiffer().getSSTBackupDir()), + inodesFromOmDataDir, hardLinkMapFromOmData); Path hardlinkFilePath = newDbDir.toPath().resolve(OmSnapshotManager.OM_HARDLINK_FILE); Map> hardlinkMapFromTarball = readFileToMap(hardlinkFilePath.toString()); @@ -278,13 +275,146 @@ void testContentsOfTarballWithSnapshot() throws Exception { assertFalse(hardlinkFilePath.toFile().exists()); } + @Test + public void testSnapshotDBConsistency() throws Exception { + String volumeName = "vol" + RandomStringUtils.secure().nextNumeric(5); + String bucketName = "buck" + RandomStringUtils.secure().nextNumeric(5); + AtomicReference realCheckpoint = new AtomicReference<>(); + setupClusterAndMocks(volumeName, bucketName, realCheckpoint); + List snapshots = new ArrayList<>(); + client.getObjectStore().listSnapshot(volumeName, bucketName, "", null) + .forEachRemaining(snapshots::add); + OzoneSnapshot snapshotToModify = snapshots.get(0); + String dummyKey = "dummyKey"; + writeDummyKeyToDeleteTableOfSnapshotDB(snapshotToModify, bucketName, volumeName, dummyKey); + // Get the tarball. + omDbCheckpointServletMock.doGet(requestMock, responseMock); + String testDirName = folder.resolve("testDir").toString(); + String newDbDirName = testDirName + OM_KEY_PREFIX + OM_DB_NAME; + File newDbDir = new File(newDbDirName); + assertTrue(newDbDir.mkdirs()); + FileUtil.unTar(tempFile, newDbDir); + Set allPathsInTarball = getAllPathsInTarball(newDbDir); + // create hardlinks now + OmSnapshotUtils.createHardLinks(newDbDir.toPath()); + for (Path old : allPathsInTarball) { + assertTrue(old.toFile().delete()); + } + Path snapshotDbDir = Paths.get(newDbDir.toPath().toString(), OM_SNAPSHOT_CHECKPOINT_DIR, + OM_DB_NAME + "-" + snapshotToModify.getSnapshotId()); + deleteWalFiles(snapshotDbDir); + assertTrue(Files.exists(snapshotDbDir)); + String value = getValueFromSnapshotDeleteTable(dummyKey, snapshotDbDir.toString()); + assertNotNull(value); + } + + private static void deleteWalFiles(Path snapshotDbDir) throws IOException { + try (Stream filesInTarball = Files.list(snapshotDbDir)) { + List files = filesInTarball.filter(p -> p.toString().contains(".log")) + .collect(Collectors.toList()); + for (Path p : files) { + Files.delete(p); + } + } + } + + private static Set getAllPathsInTarball(File newDbDir) throws IOException { + Set allPathsInTarball = new HashSet<>(); + try (Stream filesInTarball = Files.list(newDbDir.toPath())) { + List files = filesInTarball.collect(Collectors.toList()); + for (Path p : files) { + File file = p.toFile(); + if (file.getName().equals(OmSnapshotManager.OM_HARDLINK_FILE)) { + continue; + } + allPathsInTarball.add(p); + } + } + return allPathsInTarball; + } + + private void writeDummyKeyToDeleteTableOfSnapshotDB(OzoneSnapshot snapshotToModify, String bucketName, + String volumeName, String keyName) + throws IOException { + try (UncheckedAutoCloseableSupplier supplier = om.getOmSnapshotManager() + .getSnapshot(snapshotToModify.getSnapshotId())) { + OmSnapshot omSnapshot = supplier.get(); + OmKeyInfo dummyOmKeyInfo = + new OmKeyInfo.Builder().setBucketName(bucketName).setVolumeName(volumeName).setKeyName(keyName) + .setReplicationConfig(StandaloneReplicationConfig.getInstance(ONE)).build(); + RepeatedOmKeyInfo dummyRepeatedKeyInfo = + new RepeatedOmKeyInfo.Builder().setOmKeyInfos(Collections.singletonList(dummyOmKeyInfo)).build(); + omSnapshot.getMetadataManager().getDeletedTable().put(dummyOmKeyInfo.getKeyName(), dummyRepeatedKeyInfo); + } + } + + private void setupClusterAndMocks(String volumeName, String bucketName, + AtomicReference realCheckpoint) throws Exception { + setupCluster(); + setupMocks(); + om.getKeyManager().getSnapshotSstFilteringService().pause(); + when(requestMock.getParameter(OZONE_DB_CHECKPOINT_INCLUDE_SNAPSHOT_DATA)).thenReturn("true"); + // Create a "spy" dbstore keep track of the checkpoint. + writeData(volumeName, bucketName, true); + DBStore dbStore = om.getMetadataManager().getStore(); + DBStore spyDbStore = spy(dbStore); + when(spyDbStore.getCheckpoint(true)).thenAnswer(b -> { + DBCheckpoint checkpoint = spy(dbStore.getCheckpoint(true)); + // Don't delete the checkpoint, because we need to compare it + // with the snapshot data. + doNothing().when(checkpoint).cleanupCheckpoint(); + realCheckpoint.set(checkpoint); + return checkpoint; + }); + // Init the mock with the spyDbstore + doCallRealMethod().when(omDbCheckpointServletMock).initialize(any(), any(), + eq(false), any(), any(), eq(false)); + omDbCheckpointServletMock.initialize(spyDbStore, om.getMetrics().getDBCheckpointMetrics(), + false, + om.getOmAdminUsernames(), om.getOmAdminGroups(), false); + when(responseMock.getOutputStream()).thenReturn(servletOutputStream); + } + + String getValueFromSnapshotDeleteTable(String key, String snapshotDB) { + String result = null; + List cfDescriptors = new ArrayList<>(); + int count = 1; + int deletedTableCFIndex = 0; + cfDescriptors.add(new ColumnFamilyDescriptor("default".getBytes(StandardCharsets.UTF_8))); + for (String cfName : OMDBDefinition.getAllColumnFamilies()) { + if (cfName.equals(OMDBDefinition.DELETED_TABLE)) { + deletedTableCFIndex = count; + } + cfDescriptors.add(new ColumnFamilyDescriptor(cfName.getBytes(StandardCharsets.UTF_8))); + count++; + } + // For holding handles + List cfHandles = new ArrayList<>(); + try (DBOptions options = new DBOptions().setCreateIfMissing(false).setCreateMissingColumnFamilies(true); + RocksDB db = RocksDB.openReadOnly(options, snapshotDB, cfDescriptors, cfHandles)) { + + ColumnFamilyHandle deletedTableCF = cfHandles.get(deletedTableCFIndex); // 0 is default + byte[] value = db.get(deletedTableCF, key.getBytes(StandardCharsets.UTF_8)); + if (value != null) { + result = new String(value, StandardCharsets.UTF_8); + } + } catch (Exception e) { + fail("Exception while reading from snapshot DB " + e.getMessage()); + } finally { + for (ColumnFamilyHandle handle : cfHandles) { + handle.close(); + } + } + return result; + } + public static Map> readFileToMap(String filePath) throws IOException { Map> dataMap = new HashMap<>(); try (BufferedReader reader = Files.newBufferedReader(Paths.get(filePath), StandardCharsets.UTF_8)) { String line; while ((line = reader.readLine()) != null) { String trimmedLine = line.trim(); - if (trimmedLine.isEmpty() || !trimmedLine.contains("\t")) { + if (!trimmedLine.contains("\t")) { continue; } int tabIndex = trimmedLine.indexOf("\t"); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java index 3fe5aca7a919..7b881a3fb1e0 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java @@ -26,6 +26,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.OZONE_DB_CHECKPOINT_REQUEST_TO_EXCLUDE_SST; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_RATIS_SNAPSHOT_MAX_TOTAL_SST_SIZE_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_RATIS_SNAPSHOT_MAX_TOTAL_SST_SIZE_KEY; +import static org.apache.hadoop.ozone.om.lock.OzoneManagerLock.FlatResource.SNAPSHOT_DB_LOCK; import static org.apache.hadoop.ozone.om.snapshot.OMDBCheckpointUtils.includeSnapshotData; import static org.apache.hadoop.ozone.om.snapshot.OMDBCheckpointUtils.logEstimatedTarballSize; import static org.apache.hadoop.ozone.om.snapshot.OmSnapshotUtils.DATA_PREFIX; @@ -50,6 +51,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.UUID; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Stream; import javax.servlet.ServletException; @@ -255,6 +257,18 @@ public void writeDbDataToStream(HttpServletRequest request, OutputStream destina hardLinkFileMap, getCompactionLogDir()); writeDBToArchive(sstFilesToExclude, tmpSstBackupDir, maxTotalSstSize, archiveOutputStream, tmpdir, hardLinkFileMap, getSstBackupDir()); + + // This is done to ensure all data to be copied correctly is flushed in the snapshot DB + for (Path snapshotDir : snapshotPaths) { + String snapshotId = OmSnapshotManager.extractSnapshotIDFromCheckpointDirName(snapshotDir.toString()); + omMetadataManager.getLock().acquireReadLock(SNAPSHOT_DB_LOCK, snapshotId); + // invalidate closes the snapshot DB + om.getOmSnapshotManager().invalidateCacheEntry(UUID.fromString(snapshotId)); + writeDBToArchive(sstFilesToExclude, snapshotDir, + maxTotalSstSize, archiveOutputStream, tmpdir, hardLinkFileMap); + omMetadataManager.getLock().releaseReadLock(SNAPSHOT_DB_LOCK, snapshotId); + } + } writeHardlinkFile(getConf(), hardLinkFileMap, archiveOutputStream); includeRatisSnapshotCompleteFlag(archiveOutputStream); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java index 93070fcbe052..0ce838c20cc2 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java @@ -24,6 +24,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_CHECKPOINT_DIR; import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_DIFF_DB_NAME; import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_INDICATOR; +import static org.apache.hadoop.ozone.OzoneConsts.SEPARATOR; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_FS_SNAPSHOT_MAX_LIMIT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_FS_SNAPSHOT_MAX_LIMIT_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_CACHE_CLEANUP_SERVICE_RUN_INTERVAL; @@ -769,6 +770,15 @@ public static String getSnapshotPath(OzoneConfiguration conf, OM_DB_NAME + checkpointDirName; } + public static String extractSnapshotIDFromCheckpointDirName(String snapshotPath) { + // Find "om.db-" in the path and return whatever comes after + int index = snapshotPath.lastIndexOf(OM_DB_NAME); + if (index == -1 || index + OM_DB_NAME.length() >= snapshotPath.length()) { + throw new IllegalArgumentException("Invalid snapshot path " + snapshotPath); + } + return snapshotPath.substring(index + OM_DB_NAME.length() + SEPARATOR.length()); + } + public static boolean isSnapshotKey(String[] keyParts) { return (keyParts.length > 1) && (keyParts[0].compareTo(OM_SNAPSHOT_INDICATOR) == 0); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java index 6d053e1e5e05..9894e8f5d6bf 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/codec/OMDBDefinition.java @@ -17,6 +17,8 @@ package org.apache.hadoop.ozone.om.codec; +import java.util.ArrayList; +import java.util.List; import java.util.Map; import org.apache.hadoop.hdds.utils.TransactionInfo; import org.apache.hadoop.hdds.utils.db.DBColumnFamilyDefinition; @@ -358,5 +360,13 @@ public String getName() { public String getLocationConfigKey() { return OMConfigKeys.OZONE_OM_DB_DIRS; } + + public static List getAllColumnFamilies() { + List columnFamilies = new ArrayList<>(); + COLUMN_FAMILIES.values().forEach(cf -> { + columnFamilies.add(cf.getName()); + }); + return columnFamilies; + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java index b465956f35e6..eedf18f6534a 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotCache.java @@ -139,6 +139,7 @@ public void invalidate(UUID key) { LOG.warn("SnapshotId: '{}' does not exist in snapshot cache.", k); } else { try { + v.get().getMetadataManager().getStore().flushDB(); v.get().close(); } catch (IOException e) { throw new IllegalStateException("Failed to close snapshotId: " + key, e); From 9f5c4382d0dca48b60f435e4ecfbf594272c754e Mon Sep 17 00:00:00 2001 From: Sadanand Shenoy Date: Wed, 25 Jun 2025 23:30:43 +0530 Subject: [PATCH 2/4] add try-finally --- .../om/TestOMDbCheckpointServletInodeBasedXfer.java | 1 + .../om/OMDBCheckpointServletInodeBasedXfer.java | 13 ++++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java index 3fd1907a108f..6398dadf2872 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java @@ -120,6 +120,7 @@ public class TestOMDbCheckpointServletInodeBasedXfer { @BeforeEach void init() throws Exception { conf = new OzoneConfiguration(); + // ensure cache entries are not evicted thereby snapshot db's are not closed conf.setTimeDuration(OMConfigKeys.OZONE_OM_SNAPSHOT_CACHE_CLEANUP_SERVICE_RUN_INTERVAL, 100, TimeUnit.MINUTES); } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java index 7b881a3fb1e0..46f1a2835048 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java @@ -262,11 +262,14 @@ public void writeDbDataToStream(HttpServletRequest request, OutputStream destina for (Path snapshotDir : snapshotPaths) { String snapshotId = OmSnapshotManager.extractSnapshotIDFromCheckpointDirName(snapshotDir.toString()); omMetadataManager.getLock().acquireReadLock(SNAPSHOT_DB_LOCK, snapshotId); - // invalidate closes the snapshot DB - om.getOmSnapshotManager().invalidateCacheEntry(UUID.fromString(snapshotId)); - writeDBToArchive(sstFilesToExclude, snapshotDir, - maxTotalSstSize, archiveOutputStream, tmpdir, hardLinkFileMap); - omMetadataManager.getLock().releaseReadLock(SNAPSHOT_DB_LOCK, snapshotId); + try { + // invalidate closes the snapshot DB + om.getOmSnapshotManager().invalidateCacheEntry(UUID.fromString(snapshotId)); + writeDBToArchive(sstFilesToExclude, snapshotDir, maxTotalSstSize, archiveOutputStream, tmpdir, + hardLinkFileMap); + } finally { + omMetadataManager.getLock().releaseReadLock(SNAPSHOT_DB_LOCK, snapshotId); + } } } From 9ec93e5525ac0b444945e84bac7b49e96b25d4d4 Mon Sep 17 00:00:00 2001 From: Sadanand Shenoy Date: Tue, 8 Jul 2025 13:38:23 +0530 Subject: [PATCH 3/4] address comment --- .../src/main/java/org/apache/hadoop/ozone/OzoneConsts.java | 2 +- .../java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java | 4 ++-- .../java/org/apache/hadoop/ozone/om/OmSnapshotManager.java | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index 7c9134da4c08..22e402d3af90 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -537,7 +537,7 @@ public final class OzoneConsts { * A constant string used as a separator in various contexts within * the OMDBCheckpoint functions. */ - public static final String SEPARATOR = "-"; + public static final String OM_SNAPSHOT_SEPARATOR = "-"; private OzoneConsts() { // Never Constructed diff --git a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java index f74fbbadc19d..688aba5ee2ea 100644 --- a/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java +++ b/hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/SnapshotInfo.java @@ -20,7 +20,7 @@ import static org.apache.hadoop.hdds.HddsUtils.fromProtobuf; import static org.apache.hadoop.hdds.HddsUtils.toProtobuf; import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX; -import static org.apache.hadoop.ozone.OzoneConsts.SEPARATOR; +import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_SEPARATOR; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; @@ -548,7 +548,7 @@ public Map toAuditMap() { public static String getCheckpointDirName(UUID snapshotId) { Objects.requireNonNull(snapshotId, "SnapshotId is needed to create checkpoint directory"); - return SEPARATOR + snapshotId; + return OM_SNAPSHOT_SEPARATOR + snapshotId; } /** diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java index 0ce838c20cc2..a0f1b0064a55 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java @@ -24,7 +24,7 @@ import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_CHECKPOINT_DIR; import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_DIFF_DB_NAME; import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_INDICATOR; -import static org.apache.hadoop.ozone.OzoneConsts.SEPARATOR; +import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_SEPARATOR; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_FS_SNAPSHOT_MAX_LIMIT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_FS_SNAPSHOT_MAX_LIMIT_DEFAULT; import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_CACHE_CLEANUP_SERVICE_RUN_INTERVAL; @@ -776,7 +776,7 @@ public static String extractSnapshotIDFromCheckpointDirName(String snapshotPath) if (index == -1 || index + OM_DB_NAME.length() >= snapshotPath.length()) { throw new IllegalArgumentException("Invalid snapshot path " + snapshotPath); } - return snapshotPath.substring(index + OM_DB_NAME.length() + SEPARATOR.length()); + return snapshotPath.substring(index + OM_DB_NAME.length() + OM_SNAPSHOT_SEPARATOR.length()); } public static boolean isSnapshotKey(String[] keyParts) { From 9732e2c2264c8d125d6038e6497d3dcecdafbfb9 Mon Sep 17 00:00:00 2001 From: Sadanand Shenoy Date: Tue, 8 Jul 2025 22:41:07 +0530 Subject: [PATCH 4/4] address comment --- ...stOMDbCheckpointServletInodeBasedXfer.java | 4 ++ .../OMDBCheckpointServletInodeBasedXfer.java | 46 +++++++++++++------ .../hadoop/ozone/om/OmSnapshotManager.java | 2 +- 3 files changed, 37 insertions(+), 15 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java index 6398dadf2872..f9c5ffa878d5 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java @@ -276,6 +276,10 @@ void testContentsOfTarballWithSnapshot() throws Exception { assertFalse(hardlinkFilePath.toFile().exists()); } + /** + * Verifies that a manually added entry to the snapshot's delete table + * is persisted and can be retrieved from snapshot db loaded from OM DB checkpoint. + */ @Test public void testSnapshotDBConsistency() throws Exception { String volumeName = "vol" + RandomStringUtils.secure().nextNumeric(5); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java index 46f1a2835048..fe2d36e5f966 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMDBCheckpointServletInodeBasedXfer.java @@ -257,21 +257,9 @@ public void writeDbDataToStream(HttpServletRequest request, OutputStream destina hardLinkFileMap, getCompactionLogDir()); writeDBToArchive(sstFilesToExclude, tmpSstBackupDir, maxTotalSstSize, archiveOutputStream, tmpdir, hardLinkFileMap, getSstBackupDir()); - // This is done to ensure all data to be copied correctly is flushed in the snapshot DB - for (Path snapshotDir : snapshotPaths) { - String snapshotId = OmSnapshotManager.extractSnapshotIDFromCheckpointDirName(snapshotDir.toString()); - omMetadataManager.getLock().acquireReadLock(SNAPSHOT_DB_LOCK, snapshotId); - try { - // invalidate closes the snapshot DB - om.getOmSnapshotManager().invalidateCacheEntry(UUID.fromString(snapshotId)); - writeDBToArchive(sstFilesToExclude, snapshotDir, maxTotalSstSize, archiveOutputStream, tmpdir, - hardLinkFileMap); - } finally { - omMetadataManager.getLock().releaseReadLock(SNAPSHOT_DB_LOCK, snapshotId); - } - } - + transferSnapshotData(sstFilesToExclude, tmpdir, snapshotPaths, maxTotalSstSize, + archiveOutputStream, hardLinkFileMap); } writeHardlinkFile(getConf(), hardLinkFileMap, archiveOutputStream); includeRatisSnapshotCompleteFlag(archiveOutputStream); @@ -285,6 +273,36 @@ public void writeDbDataToStream(HttpServletRequest request, OutputStream destina } } + /** + * Transfers the snapshot data from the specified snapshot directories into the archive output stream, + * handling deduplication and managing resource locking. + * + * @param sstFilesToExclude Set of SST file identifiers to exclude from the archive. + * @param tmpdir Temporary directory for intermediate processing. + * @param snapshotPaths Set of paths to snapshot directories to be processed. + * @param maxTotalSstSize AtomicLong to track the cumulative size of SST files included. + * @param archiveOutputStream Archive output stream to write the snapshot data. + * @param hardLinkFileMap Map of hardlink file paths to their unique identifiers for deduplication. + * @throws IOException if an I/O error occurs during processing. + */ + private void transferSnapshotData(Set sstFilesToExclude, Path tmpdir, Set snapshotPaths, + AtomicLong maxTotalSstSize, ArchiveOutputStream archiveOutputStream, + Map hardLinkFileMap) throws IOException { + OzoneManager om = (OzoneManager) getServletContext().getAttribute(OzoneConsts.OM_CONTEXT_ATTRIBUTE); + OMMetadataManager omMetadataManager = om.getMetadataManager(); + for (Path snapshotDir : snapshotPaths) { + String snapshotId = OmSnapshotManager.extractSnapshotIDFromCheckpointDirName(snapshotDir.toString()); + omMetadataManager.getLock().acquireReadLock(SNAPSHOT_DB_LOCK, snapshotId); + try { + // invalidate closes the snapshot DB + om.getOmSnapshotManager().invalidateCacheEntry(UUID.fromString(snapshotId)); + writeDBToArchive(sstFilesToExclude, snapshotDir, maxTotalSstSize, archiveOutputStream, tmpdir, hardLinkFileMap); + } finally { + omMetadataManager.getLock().releaseReadLock(SNAPSHOT_DB_LOCK, snapshotId); + } + } + } + private boolean writeDBToArchive(Set sstFilesToExclude, Path dir, AtomicLong maxTotalSstSize, ArchiveOutputStream archiveOutputStream, Path tmpdir, Map hardLinkFileMap) throws IOException { diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java index a0f1b0064a55..f9e314a51d86 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java @@ -773,7 +773,7 @@ public static String getSnapshotPath(OzoneConfiguration conf, public static String extractSnapshotIDFromCheckpointDirName(String snapshotPath) { // Find "om.db-" in the path and return whatever comes after int index = snapshotPath.lastIndexOf(OM_DB_NAME); - if (index == -1 || index + OM_DB_NAME.length() >= snapshotPath.length()) { + if (index == -1 || index + OM_DB_NAME.length() + OM_SNAPSHOT_SEPARATOR.length() >= snapshotPath.length()) { throw new IllegalArgumentException("Invalid snapshot path " + snapshotPath); } return snapshotPath.substring(index + OM_DB_NAME.length() + OM_SNAPSHOT_SEPARATOR.length());