HDDS-15201. Record lastDefragTime when snapshot defrag is successfully run

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`

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;
   }
 

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<Integer, VersionMeta> versionMetaMap =
             (Map<Integer, VersionMeta>) nodes.get(OzoneConsts.OM_SLD_VERSION_SST_FILE_INFO);

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) {

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<OmSnapshotLocalData> omSnapshotLocalDataSerializer;
 
-  private static final Instant NOW = Instant.now();
-
   @BeforeAll
   public static void setupSerializer() throws IOException {
     omSnapshotLocalDataSerializer = new YamlSerializer<OmSnapshotLocalData>(YAML_FACTORY) {
@@ -126,7 +125,7 @@ private Pair<File, UUID> 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<Integer, VersionMeta> 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<File, UUID> 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<File, UUID> 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());
+  }
 }

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<SstFileInfo> expectedLiveFileMetaData =
           newVersionSstFiles.subList(0, 3).stream().map(SstFileInfo::new).collect(Collectors.toList());