diff --git a/hadoop-hdds/docs/content/feature/SnapshotDefragmentation.md b/hadoop-hdds/docs/content/feature/SnapshotDefragmentation.md index 34b7adddb1e4..3660da677334 100644 --- a/hadoop-hdds/docs/content/feature/SnapshotDefragmentation.md +++ b/hadoop-hdds/docs/content/feature/SnapshotDefragmentation.md @@ -40,7 +40,7 @@ Currently, snapshot RocksDBs has automatic RocksDB compaction disabled intention Note: Snapshot Defragmentation was previously called Snapshot Compaction earlier during the design phase. It is not RocksDB compaction. Thus the rename to avoid such confusion. We are also not going to enable RocksDB auto compaction on snapshot RocksDBs. -1. ### Introducing last defragmentation time +1. ### Snapshot local YAML metadata The implemented metadata is stored in local `OmSnapshotLocalData` YAML files, not in Ratis-replicated `SnapshotInfo`. The YAML is created on every snapshot @@ -49,7 +49,8 @@ Note: Snapshot Defragmentation was previously called Snapshot Compaction earlier The important YAML fields are `snapshotId`, `previousSnapshotId`, `version`, `needsDefrag`, `versionSstFileInfos`, `dbTxSequenceNumber`, `transactionInfo`, `lastDefragTime`, `checksum`, and `isSSTFiltered`. - `lastDefragTime` is serialized, but current defrag decisions are based on + `lastDefragTime` records the local wall-clock time when a new defragged + snapshot version is committed. Current defrag decisions are still based on `version`, `needsDefrag`, and `versionSstFileInfos`. The earlier proposal's `notDefraggedSstFileList` and `defraggedSstFileList` diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalData.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalData.java index f876a9606017..a74416d86902 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalData.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalData.java @@ -155,7 +155,7 @@ public long getLastDefragTime() { * Sets the last defrag time, in epoch milliseconds. * @param lastDefragTime Timestamp of the last defrag */ - public void setLastDefragTime(Long lastDefragTime) { + public void setLastDefragTime(long lastDefragTime) { this.lastDefragTime = lastDefragTime; } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java index b72e74cf4a6b..ed6b3feda7e2 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java @@ -183,18 +183,15 @@ public Object construct(Node node) { // Set other fields from parsed YAML snapshotLocalData.setSstFiltered((Boolean) nodes.getOrDefault(OzoneConsts.OM_SLD_IS_SST_FILTERED, false)); - - // Handle potential Integer/Long type mismatch from YAML parsing - Object lastDefragTimeObj = nodes.getOrDefault(OzoneConsts.OM_SLD_LAST_DEFRAG_TIME, -1L); - long lastDefragTime; - if (lastDefragTimeObj instanceof Number) { - lastDefragTime = ((Number) lastDefragTimeObj).longValue(); - } else { + Object lastDefragTimeObj = nodes.get(OzoneConsts.OM_SLD_LAST_DEFRAG_TIME); + if (lastDefragTimeObj == null) { + snapshotLocalData.setLastDefragTime(0L); + } else if (!(lastDefragTimeObj instanceof Number)) { throw new IllegalArgumentException("Invalid type for lastDefragTime: " + lastDefragTimeObj.getClass().getName() + ". Expected Number type."); + } else { + snapshotLocalData.setLastDefragTime(((Number) lastDefragTimeObj).longValue()); } - snapshotLocalData.setLastDefragTime(lastDefragTime); - snapshotLocalData.setNeedsDefrag((Boolean) nodes.getOrDefault(OzoneConsts.OM_SLD_NEEDS_DEFRAG, false)); Map versionMetaMap = (Map) nodes.get(OzoneConsts.OM_SLD_VERSION_SST_FILE_INFO); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java index d542932909cf..a2203f675d80 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java @@ -73,6 +73,7 @@ import org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager; import org.apache.hadoop.ozone.util.ObjectSerializer; import org.apache.hadoop.ozone.util.YamlSerializer; +import org.apache.hadoop.util.Time; import org.apache.ratis.util.function.CheckedFunction; import org.apache.ratis.util.function.CheckedSupplier; import org.rocksdb.LiveFileMetaData; @@ -895,8 +896,10 @@ public void setTransactionInfo(TransactionInfo transactionInfo) { } public synchronized void commit() throws IOException { + SnapshotVersionsMeta existingVersionsMeta = getVersionNodeMap().get(super.snapshotId); // Validate modification and commit the changes. SnapshotVersionsMeta localDataVersionNodes = validateModification(super.snapshotLocalData); + boolean persistLastDefragTime = shouldUpdateLastDefragTime(existingVersionsMeta, localDataVersionNodes); // Need to update the disk state if and only if the dirty bit is set. if (isDirty()) { String filePath = getSnapshotLocalPropertyYamlPath(super.snapshotId); @@ -911,9 +914,19 @@ public synchronized void commit() throws IOException { if (tmpFileExists) { throw new IOException("Unable to delete tmp file " + tmpFilePath); } - snapshotLocalDataSerializer.save(new File(tmpFilePath), super.snapshotLocalData); + Long committedLastDefragTime = null; + OmSnapshotLocalData snapshotLocalDataToPersist = super.snapshotLocalData; + if (persistLastDefragTime) { + committedLastDefragTime = Time.now(); + snapshotLocalDataToPersist = super.snapshotLocalData.copyObject(); + snapshotLocalDataToPersist.setLastDefragTime(committedLastDefragTime); + } + snapshotLocalDataSerializer.save(new File(tmpFilePath), snapshotLocalDataToPersist); Files.move(tmpFile.toPath(), Paths.get(filePath), StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + if (committedLastDefragTime != null) { + super.snapshotLocalData.setLastDefragTime(committedLastDefragTime); + } } else if (snapshotLocalDataFile.exists()) { LOG.info("Deleting YAML file corresponding to snapshotId: {} in path : {}", super.snapshotId, snapshotLocalDataFile.getAbsolutePath()); @@ -929,6 +942,16 @@ public synchronized void commit() throws IOException { } } + private boolean shouldUpdateLastDefragTime(SnapshotVersionsMeta existingVersionsMeta, + SnapshotVersionsMeta currentVersionsMeta) { + if (currentVersionsMeta.getSnapshotVersions().isEmpty()) { + return false; + } + int currentVersion = currentVersionsMeta.getVersion(); + return currentVersion > 0 && + (existingVersionsMeta == null || currentVersion > existingVersionsMeta.getVersion()); + } + private void checkForOphanVersionsAndIncrementCount(UUID snapshotId, SnapshotVersionsMeta previousVersionsMeta, SnapshotVersionsMeta currentVersionMeta, boolean isPurgeTransactionSet) { if (previousVersionsMeta != null) { diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java index 34b9fbe397ec..e18adae7b406 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java @@ -32,7 +32,6 @@ import java.io.File; import java.io.IOException; import java.nio.charset.Charset; -import java.time.Instant; import java.util.Collections; import java.util.List; import java.util.Map; @@ -62,12 +61,12 @@ */ public class TestOmSnapshotLocalDataYaml { + private static final long LAST_DEFRAG_TIME = 123456789L; + private static String testRoot = new FileSystemTestHelper().getTestRootDir(); private static final OmSnapshotLocalDataYaml.YamlFactory YAML_FACTORY = new OmSnapshotLocalDataYaml.YamlFactory(); private static ObjectSerializer omSnapshotLocalDataSerializer; - private static final Instant NOW = Instant.now(); - @BeforeAll public static void setupSerializer() throws IOException { omSnapshotLocalDataSerializer = new YamlSerializer(YAML_FACTORY) { @@ -126,7 +125,7 @@ private Pair writeToYaml(UUID snapshotId, String snapshotName, Trans dataYaml.setSstFiltered(true); // Set last defrag time - dataYaml.setLastDefragTime(NOW.toEpochMilli()); + dataYaml.setLastDefragTime(LAST_DEFRAG_TIME); // Set needs defrag flag dataYaml.setNeedsDefrag(true); @@ -173,7 +172,7 @@ public void testWriteToYaml() throws IOException { ImmutableList.of(new SstFileInfo("sst1", "k1", "k2", "table1"), new SstFileInfo("sst2", "k3", "k4", "table1"), new SstFileInfo("sst3", "k4", "k5", "table2"))), notDefraggedSSTFiles); - assertEquals(NOW.toEpochMilli(), snapshotData.getLastDefragTime()); + assertEquals(LAST_DEFRAG_TIME, snapshotData.getLastDefragTime()); assertTrue(snapshotData.getNeedsDefrag()); Map defraggedSSTFiles = snapshotData.getVersionSstFileInfos(); @@ -261,7 +260,7 @@ public void testChecksum() throws IOException { } @Test - public void testYamlContainsAllFields() throws IOException { + public void testYamlContainsCurrentFields() throws IOException { UUID snapshotId = UUID.randomUUID(); TransactionInfo transactionInfo = TransactionInfo.valueOf(ThreadLocalRandom.current().nextLong(), ThreadLocalRandom.current().nextLong()); @@ -280,4 +279,43 @@ public void testYamlContainsAllFields() throws IOException { assertThat(content).contains(OzoneConsts.OM_SLD_PREV_SNAP_ID); assertThat(content).contains(OzoneConsts.OM_SLD_TXN_INFO); } + + @Test + public void testLoadYamlWithoutLastDefragTimeDefaultsTo0() throws IOException { + UUID snapshotId = UUID.randomUUID(); + Pair yamlFilePrevIdPair = writeToYaml(snapshotId, "snapshot5", null); + File yamlFile = yamlFilePrevIdPair.getLeft(); + String content = FileUtils.readFileToString(yamlFile, Charset.defaultCharset()); + String legacyContent = content.replace("lastDefragTime: " + LAST_DEFRAG_TIME + "\n", ""); + FileUtils.writeStringToFile(yamlFile, legacyContent, Charset.defaultCharset()); + + OmSnapshotLocalData snapshotData = omSnapshotLocalDataSerializer.load(yamlFile); + + assertEquals(44, snapshotData.getVersion()); + assertEquals(0L, snapshotData.getLastDefragTime()); + assertTrue(snapshotData.getNeedsDefrag()); + assertTrue(snapshotData.getSstFiltered()); + assertEquals(3, snapshotData.getVersionSstFileInfos().size()); + } + + @Test + public void testLoadYamlWithEmptyLastDefragTimeDefaultsTo0() throws IOException { + // Parser compatibility for older/edited YAML: removing this field or + // leaving it empty must not make deserialization fail. + UUID snapshotId = UUID.randomUUID(); + Pair yamlFilePrevIdPair = writeToYaml(snapshotId, "snapshot6", null); + File yamlFile = yamlFilePrevIdPair.getLeft(); + String content = FileUtils.readFileToString(yamlFile, Charset.defaultCharset()); + String legacyContent = content.replace("lastDefragTime: " + LAST_DEFRAG_TIME, + "lastDefragTime:"); + FileUtils.writeStringToFile(yamlFile, legacyContent, Charset.defaultCharset()); + + OmSnapshotLocalData snapshotData = omSnapshotLocalDataSerializer.load(yamlFile); + + assertEquals(44, snapshotData.getVersion()); + assertEquals(0L, snapshotData.getLastDefragTime()); + assertTrue(snapshotData.getNeedsDefrag()); + assertTrue(snapshotData.getSstFiltered()); + assertEquals(3, snapshotData.getVersionSstFileInfos().size()); + } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotLocalDataManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotLocalDataManager.java index a2884af76466..c73304fa1229 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotLocalDataManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotLocalDataManager.java @@ -461,16 +461,26 @@ public void testAddVersionFromRDB() throws IOException { createMockLiveFileMetaData("file6.sst", FILE_TABLE, "key1", "key2"), createMockLiveFileMetaData("file7.sst", KEY_TABLE, "key1", "key2"), createMockLiveFileMetaData("file1.sst", "col1", "key1", "key2")); + long beforeAdd = System.currentTimeMillis(); + long committedLastDefragTime; try (WritableOmSnapshotLocalDataProvider snap = localDataManager.getWritableOmSnapshotLocalData(snapId)) { + assertEquals(0L, snap.getSnapshotLocalData().getLastDefragTime()); mockSnapshotStore(snapId, newVersionSstFiles); snap.addSnapshotVersion(snapshotStore); + assertEquals(0L, snap.getSnapshotLocalData().getLastDefragTime()); snap.commit(); + committedLastDefragTime = snap.getSnapshotLocalData().getLastDefragTime(); + assertTrue(committedLastDefragTime >= beforeAdd); } + long afterAdd = System.currentTimeMillis(); validateVersions(localDataManager, snapId, 1, Sets.newHashSet(0, 1)); try (ReadableOmSnapshotLocalDataProvider snap = localDataManager.getOmSnapshotLocalData(snapId)) { OmSnapshotLocalData snapshotLocalData = snap.getSnapshotLocalData(); OmSnapshotLocalData.VersionMeta versionMeta = snapshotLocalData.getVersionSstFileInfos().get(1); + assertEquals(committedLastDefragTime, snapshotLocalData.getLastDefragTime()); + assertTrue(snapshotLocalData.getLastDefragTime() >= beforeAdd); + assertTrue(snapshotLocalData.getLastDefragTime() <= afterAdd); assertEquals(6, versionMeta.getPreviousSnapshotVersion()); List expectedLiveFileMetaData = newVersionSstFiles.subList(0, 3).stream().map(SstFileInfo::new).collect(Collectors.toList());