You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "arunsarin85 (via GitHub)" <gi...@apache.org> on 2026/05/11 09:47:06 UTC

[PR] HDDS-13217. Add integration test comparing OM snapshot checkpoint metadata to live OM for a bucket [ozone]

arunsarin85 opened a new pull request, #10235:
URL: https://github.com/apache/ozone/pull/10235

   ## What changes were proposed in this pull request?
   This PR adds a new integration test class,TestOmSnapshotCheckpointDbContent. It builds a small MiniOzone cluster, writes keys for OBJECT_STORE, FILE_SYSTEM_OPTIMIZED, and LEGACY buckets, creates a snapshot, waits for the on-disk checkpoint to appear, opens that checkpoint read-only, and compares bucket-scoped rows between the live OM metadata manager and the checkpoint for the tables the test covers.
   
   Please describe your PR in detail:
   Snapshot correctness depends on the OM checkpoint on disk reflecting the same bucket metadata as the active OM for the data paths clients care about. 
   
   For each bucket layout, the flow is: create a volume and bucket, write a few keys, call createSnapshot, load SnapshotInfo from the live OM, wait until the checkpoint CURRENT file exists, open the checkpoint with RocksDBCheckpoint and OmMetadataManagerImpl.createCheckpointMetadataManager, then scan matching key prefixes on both sides.
   
   Compared tables include volume and bucket rows, the key table for object store and legacy layouts, file and directory tables for FSO.
   
   The new snapshot row in snapshotInfoTable commits in the same batch as the checkpoint step, so it appears on the live OM after commit but not inside the checkpoint taken mid-batch. After the checkpoint, OM also scrubs deleted and snapshot-renamed prefix ranges on the active DB, so live and checkpoint diverge there on purpose. Those tables are not part of the equality check.
   
   Three datanodes for RATIS/THREE, snapshot defrag disabled for stability, and a null check on SnapshotInfo for clearer failures.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-13217
   
   ## How was this patch tested?
   
   Locally with Maven, scoped to the new class:
   
   mvn -pl hadoop-ozone/integration-test -Dtest=TestOmSnapshotCheckpointDbContent -DfailIfNoTests=false test
   <img width="790" height="275" alt="image" src="https://github.com/user-attachments/assets/9e89cff6-02c5-4297-8262-0a14487c3a09" />


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl closed pull request #10235: HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket
URL: https://github.com/apache/ozone/pull/10235


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "arunsarin85 (via GitHub)" <gi...@apache.org>.
arunsarin85 commented on code in PR #10235:
URL: https://github.com/apache/ozone/pull/10235#discussion_r3228323552


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotCheckpointDbContent.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DIRECTORY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.MULTIPART_INFO_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.OPEN_KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VOLUME_TABLE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.concurrent.TimeoutException;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.hdds.utils.db.RocksDBCheckpoint;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValueIterator;
+import org.apache.hadoop.hdds.utils.db.TablePrefixInfo;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * HDDS-13217: when an OM snapshot is created, the on-disk RocksDB checkpoint for that snapshot
+ * should contain the same bucket-scoped metadata as the live OM for the tables exercised here.
+ *
+ * <p>Flow: write some keys, call createSnapshot, wait until the checkpoint is materialized, open
+ * it read-only, then for each relevant table scan the key prefix for this volume/bucket on both
+ * the live metadata manager and the checkpoint and assert the maps match.
+ *
+ * <p>Tables compared: volume and bucket rows; key table for OBJECT_STORE and LEGACY layouts;
+ * file and directory tables for FILE_SYSTEM_OPTIMIZED; openKeyTable and multipartInfoTable (these
+ * stay empty in these tests but we still compare to catch stray open writes).
+ *
+ * <p>Omitted on purpose: snapshotInfoTable, because the new snapshot row is part of the same DB
+ * batch as the checkpoint step, so it appears on the live OM after the batch commits but not
+ * inside the checkpoint taken mid-batch. deletedTable, deletedDirTable, snapshotRenamedTable:
+ * the leader deletes those prefix ranges on the active DB immediately after the checkpoint, so
+ * live and checkpoint intentionally differ. We also do not scan cluster-wide tables (tokens,
+ * secrets, compaction log, other tenants/volumes).
+ *
+ * <p>Later tests worth adding: a second snapshot on the same bucket; writes after the snapshot to
+ * prove the checkpoint slice does not change; multipart uploads with real state; linked buckets
+ * resolving to a source bucket.
+ */
+public class TestOmSnapshotCheckpointDbContent {
+
+  private static final byte[] TEST_KEY_CONTENT = new byte[] {0x61, 0x62, 0x63};
+
+  private static MiniOzoneCluster cluster;
+  private static OzoneConfiguration conf;
+  private static OzoneClient client;
+  private static ObjectStore store;
+
+  /** Mini cluster shared by all tests: three datanodes, snapshots enabled; defrag disabled. */
+  @BeforeAll
+  public static void init()
+      throws IOException, InterruptedException, TimeoutException {
+    conf = new OzoneConfiguration();
+    conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+    conf.setInt(OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL, -1);
+
+    cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(3).build();
+    cluster.waitForClusterToBeReady();

Review Comment:
   Added a patch .



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #10235:
URL: https://github.com/apache/ozone/pull/10235#discussion_r3223644399


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotCheckpointDbContent.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DIRECTORY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.MULTIPART_INFO_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.OPEN_KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VOLUME_TABLE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.concurrent.TimeoutException;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.hdds.utils.db.RocksDBCheckpoint;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValueIterator;
+import org.apache.hadoop.hdds.utils.db.TablePrefixInfo;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * HDDS-13217: when an OM snapshot is created, the on-disk RocksDB checkpoint for that snapshot
+ * should contain the same bucket-scoped metadata as the live OM for the tables exercised here.
+ *
+ * <p>Flow: write some keys, call createSnapshot, wait until the checkpoint is materialized, open
+ * it read-only, then for each relevant table scan the key prefix for this volume/bucket on both
+ * the live metadata manager and the checkpoint and assert the maps match.
+ *
+ * <p>Tables compared: volume and bucket rows; key table for OBJECT_STORE and LEGACY layouts;
+ * file and directory tables for FILE_SYSTEM_OPTIMIZED; openKeyTable and multipartInfoTable (these
+ * stay empty in these tests but we still compare to catch stray open writes).
+ *
+ * <p>Omitted on purpose: snapshotInfoTable, because the new snapshot row is part of the same DB
+ * batch as the checkpoint step, so it appears on the live OM after the batch commits but not
+ * inside the checkpoint taken mid-batch. deletedTable, deletedDirTable, snapshotRenamedTable:
+ * the leader deletes those prefix ranges on the active DB immediately after the checkpoint, so
+ * live and checkpoint intentionally differ. We also do not scan cluster-wide tables (tokens,
+ * secrets, compaction log, other tenants/volumes).
+ *
+ * <p>Later tests worth adding: a second snapshot on the same bucket; writes after the snapshot to
+ * prove the checkpoint slice does not change; multipart uploads with real state; linked buckets
+ * resolving to a source bucket.
+ */
+public class TestOmSnapshotCheckpointDbContent {
+
+  private static final byte[] TEST_KEY_CONTENT = new byte[] {0x61, 0x62, 0x63};
+
+  private static MiniOzoneCluster cluster;
+  private static OzoneConfiguration conf;
+  private static OzoneClient client;
+  private static ObjectStore store;
+
+  /** Mini cluster shared by all tests: three datanodes, snapshots enabled; defrag disabled. */
+  @BeforeAll
+  public static void init()
+      throws IOException, InterruptedException, TimeoutException {
+    conf = new OzoneConfiguration();
+    conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+    conf.setInt(OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL, -1);
+
+    cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(3).build();
+    cluster.waitForClusterToBeReady();
+    client = cluster.newClient();
+    store = client.getObjectStore();
+  }
+
+  @AfterAll
+  public static void shutdown() {
+    IOUtils.closeQuietly(client);
+    if (cluster != null) {
+      cluster.shutdown();
+    }
+  }
+
+  /**
+   * Object store layout: mixed key shapes (flat and path-like), then snapshot and compare tables
+   * for this bucket.
+   */
+  @Test
+  public void testSnapshotCheckpointMatchesLiveObsBucket()
+      throws IOException, InterruptedException, TimeoutException {
+    runLayoutScenario(BucketLayout.OBJECT_STORE,
+        Arrays.asList("k1", "prefix/k2", "prefix/k3"));
+  }
+
+  /**
+   * File-system-optimized layout: data lives in file/dir tables; includes keys under a short path and
+   * one at the bucket root.
+   */
+  @Test
+  public void testSnapshotCheckpointMatchesLiveFsoBucket()
+      throws IOException, InterruptedException, TimeoutException {
+    runLayoutScenario(BucketLayout.FILE_SYSTEM_OPTIMIZED,
+        Arrays.asList("d/k1", "d/k2", "k-root"));
+  }
+
+  /**
+   * Legacy layout: still uses the key table only; flat key names to match default path behavior on
+   * this cluster.
+   */
+  @Test
+  public void testSnapshotCheckpointMatchesLiveLegacyBucket()
+      throws IOException, InterruptedException, TimeoutException {
+    runLayoutScenario(BucketLayout.LEGACY,
+        Arrays.asList("key-a", "key-b", "key-c"));
+  }
+
+  /** Create bucket, write keys, snapshot, wait for checkpoint files, open checkpoint DB, compare. */
+  private void runLayoutScenario(BucketLayout layout, List<String> keyNames)
+      throws IOException, InterruptedException, TimeoutException {
+    OzoneBucket bucket =
+        TestDataUtil.createVolumeAndBucket(client, layout);
+    String volumeName = bucket.getVolumeName();
+    String bucketName = bucket.getName();
+
+    for (String keyName : keyNames) {
+      TestDataUtil.createKey(bucket, keyName, TEST_KEY_CONTENT);
+    }
+
+    String snapshotName = "snap-a";
+    store.createSnapshot(volumeName, bucketName, snapshotName);
+
+    OzoneManager om = cluster.getOzoneManager();
+    OMMetadataManager liveMm = om.getMetadataManager();
+    SnapshotInfo snapshotInfo = liveMm.getSnapshotInfoTable().get(
+        SnapshotInfo.getTableKey(volumeName, bucketName, snapshotName));
+    assertNotNull(snapshotInfo, "Snapshot row should exist after createSnapshot");
+    assertEquals(snapshotName, snapshotInfo.getName());
+
+    String currentPath =
+        OmSnapshotManager.getSnapshotPath(conf, snapshotInfo, 0)
+            + OM_KEY_PREFIX + "CURRENT";
+    GenericTestUtils.waitFor(() -> new File(currentPath).exists(), 1000, 120_000);
+
+    RocksDBCheckpoint checkpoint = new RocksDBCheckpoint(
+        Paths.get(OmSnapshotManager.getSnapshotPath(conf, snapshotInfo, 0)));
+    try (OmMetadataManagerImpl checkpointMm =
+             OmMetadataManagerImpl.createCheckpointMetadataManager(conf, checkpoint)) {
+      TablePrefixInfo prefixes = liveMm.getTableBucketPrefix(volumeName, bucketName);
+      assertBucketPrefixTablesMatch(liveMm, checkpointMm, prefixes, layout);
+    }
+  }
+
+  /** Load every row under the bucket prefix from live vs checkpoint for the tables this test cares about. */
+  private void assertBucketPrefixTablesMatch(
+      OMMetadataManager live,
+      OMMetadataManager checkpoint,
+      TablePrefixInfo prefixes,
+      BucketLayout layout) throws IOException {
+
+    assertPrefixEquals(VOLUME_TABLE, live.getVolumeTable(),
+        checkpoint.getVolumeTable(), prefixes);
+    assertPrefixEquals(BUCKET_TABLE, live.getBucketTable(),
+        checkpoint.getBucketTable(), prefixes);
+
+    if (layout.isFileSystemOptimized()) {
+      assertPrefixEquals(FILE_TABLE, live.getFileTable(),
+          checkpoint.getFileTable(), prefixes);
+      assertPrefixEquals(DIRECTORY_TABLE, live.getDirectoryTable(),
+          checkpoint.getDirectoryTable(), prefixes);
+    } else {
+      assertPrefixEquals(KEY_TABLE,
+          live.getKeyTable(layout),
+          checkpoint.getKeyTable(layout), prefixes);
+    }
+
+    assertPrefixEquals(OPEN_KEY_TABLE,
+        live.getOpenKeyTable(layout),
+        checkpoint.getOpenKeyTable(layout), prefixes);
+    assertPrefixEquals(MULTIPART_INFO_TABLE, live.getMultipartInfoTable(),
+        checkpoint.getMultipartInfoTable(), prefixes);
+  }
+
+  private static <V> void assertPrefixEquals(
+      String tableName,
+      Table<String, V> live,
+      Table<String, V> checkpoint,
+      TablePrefixInfo prefixes) throws IOException {
+    String prefix = prefixes.getTablePrefix(tableName);
+    assertEquals(readPrefix(live, prefix), readPrefix(checkpoint, prefix),
+        tableName);

Review Comment:
   If `prefixes.getTablePrefix(tableName)` returns `null` or empty, `readPrefix(...)` returns an empty map, and the assertion will pass even though the test didn’t validate anything for that table. Consider asserting the prefix is present (at least for the tables that are expected to be bucket-prefixed in this test) so missing/incorrect prefix computation fails loudly.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotCheckpointDbContent.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DIRECTORY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.MULTIPART_INFO_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.OPEN_KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VOLUME_TABLE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.concurrent.TimeoutException;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.hdds.utils.db.RocksDBCheckpoint;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValueIterator;
+import org.apache.hadoop.hdds.utils.db.TablePrefixInfo;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * HDDS-13217: when an OM snapshot is created, the on-disk RocksDB checkpoint for that snapshot
+ * should contain the same bucket-scoped metadata as the live OM for the tables exercised here.
+ *
+ * <p>Flow: write some keys, call createSnapshot, wait until the checkpoint is materialized, open
+ * it read-only, then for each relevant table scan the key prefix for this volume/bucket on both
+ * the live metadata manager and the checkpoint and assert the maps match.
+ *
+ * <p>Tables compared: volume and bucket rows; key table for OBJECT_STORE and LEGACY layouts;
+ * file and directory tables for FILE_SYSTEM_OPTIMIZED; openKeyTable and multipartInfoTable (these
+ * stay empty in these tests but we still compare to catch stray open writes).
+ *
+ * <p>Omitted on purpose: snapshotInfoTable, because the new snapshot row is part of the same DB
+ * batch as the checkpoint step, so it appears on the live OM after the batch commits but not
+ * inside the checkpoint taken mid-batch. deletedTable, deletedDirTable, snapshotRenamedTable:
+ * the leader deletes those prefix ranges on the active DB immediately after the checkpoint, so
+ * live and checkpoint intentionally differ. We also do not scan cluster-wide tables (tokens,
+ * secrets, compaction log, other tenants/volumes).
+ *
+ * <p>Later tests worth adding: a second snapshot on the same bucket; writes after the snapshot to
+ * prove the checkpoint slice does not change; multipart uploads with real state; linked buckets
+ * resolving to a source bucket.
+ */
+public class TestOmSnapshotCheckpointDbContent {
+
+  private static final byte[] TEST_KEY_CONTENT = new byte[] {0x61, 0x62, 0x63};
+
+  private static MiniOzoneCluster cluster;
+  private static OzoneConfiguration conf;
+  private static OzoneClient client;
+  private static ObjectStore store;
+
+  /** Mini cluster shared by all tests: three datanodes, snapshots enabled; defrag disabled. */
+  @BeforeAll
+  public static void init()
+      throws IOException, InterruptedException, TimeoutException {
+    conf = new OzoneConfiguration();
+    conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+    conf.setInt(OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL, -1);
+
+    cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(3).build();
+    cluster.waitForClusterToBeReady();
+    client = cluster.newClient();
+    store = client.getObjectStore();
+  }
+
+  @AfterAll
+  public static void shutdown() {
+    IOUtils.closeQuietly(client);
+    if (cluster != null) {
+      cluster.shutdown();
+    }
+  }
+
+  /**
+   * Object store layout: mixed key shapes (flat and path-like), then snapshot and compare tables
+   * for this bucket.
+   */
+  @Test
+  public void testSnapshotCheckpointMatchesLiveObsBucket()
+      throws IOException, InterruptedException, TimeoutException {
+    runLayoutScenario(BucketLayout.OBJECT_STORE,
+        Arrays.asList("k1", "prefix/k2", "prefix/k3"));
+  }
+
+  /**
+   * File-system-optimized layout: data lives in file/dir tables; includes keys under a short path and
+   * one at the bucket root.
+   */
+  @Test
+  public void testSnapshotCheckpointMatchesLiveFsoBucket()
+      throws IOException, InterruptedException, TimeoutException {
+    runLayoutScenario(BucketLayout.FILE_SYSTEM_OPTIMIZED,
+        Arrays.asList("d/k1", "d/k2", "k-root"));
+  }
+
+  /**
+   * Legacy layout: still uses the key table only; flat key names to match default path behavior on
+   * this cluster.
+   */
+  @Test
+  public void testSnapshotCheckpointMatchesLiveLegacyBucket()
+      throws IOException, InterruptedException, TimeoutException {
+    runLayoutScenario(BucketLayout.LEGACY,
+        Arrays.asList("key-a", "key-b", "key-c"));
+  }
+
+  /** Create bucket, write keys, snapshot, wait for checkpoint files, open checkpoint DB, compare. */
+  private void runLayoutScenario(BucketLayout layout, List<String> keyNames)
+      throws IOException, InterruptedException, TimeoutException {
+    OzoneBucket bucket =
+        TestDataUtil.createVolumeAndBucket(client, layout);
+    String volumeName = bucket.getVolumeName();
+    String bucketName = bucket.getName();
+
+    for (String keyName : keyNames) {
+      TestDataUtil.createKey(bucket, keyName, TEST_KEY_CONTENT);
+    }
+
+    String snapshotName = "snap-a";
+    store.createSnapshot(volumeName, bucketName, snapshotName);
+
+    OzoneManager om = cluster.getOzoneManager();
+    OMMetadataManager liveMm = om.getMetadataManager();
+    SnapshotInfo snapshotInfo = liveMm.getSnapshotInfoTable().get(
+        SnapshotInfo.getTableKey(volumeName, bucketName, snapshotName));
+    assertNotNull(snapshotInfo, "Snapshot row should exist after createSnapshot");
+    assertEquals(snapshotName, snapshotInfo.getName());
+
+    String currentPath =
+        OmSnapshotManager.getSnapshotPath(conf, snapshotInfo, 0)
+            + OM_KEY_PREFIX + "CURRENT";
+    GenericTestUtils.waitFor(() -> new File(currentPath).exists(), 1000, 120_000);
+
+    RocksDBCheckpoint checkpoint = new RocksDBCheckpoint(
+        Paths.get(OmSnapshotManager.getSnapshotPath(conf, snapshotInfo, 0)));

Review Comment:
   Building `currentPath` via string concatenation is brittle (depends on whether `getSnapshotPath(...)` and `OM_KEY_PREFIX` include path separators) and repeats `getSnapshotPath(...)`. Prefer constructing a `Path` once (e.g., `Path snapshotPath = ...`) and resolving `OM_KEY_PREFIX` and `CURRENT` via `Path.resolve(...)` (or `Paths.get(...)`) to avoid separator issues and duplication.



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotCheckpointDbContent.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DIRECTORY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.MULTIPART_INFO_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.OPEN_KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VOLUME_TABLE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.concurrent.TimeoutException;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.hdds.utils.db.RocksDBCheckpoint;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValueIterator;
+import org.apache.hadoop.hdds.utils.db.TablePrefixInfo;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * HDDS-13217: when an OM snapshot is created, the on-disk RocksDB checkpoint for that snapshot
+ * should contain the same bucket-scoped metadata as the live OM for the tables exercised here.
+ *
+ * <p>Flow: write some keys, call createSnapshot, wait until the checkpoint is materialized, open
+ * it read-only, then for each relevant table scan the key prefix for this volume/bucket on both
+ * the live metadata manager and the checkpoint and assert the maps match.
+ *
+ * <p>Tables compared: volume and bucket rows; key table for OBJECT_STORE and LEGACY layouts;
+ * file and directory tables for FILE_SYSTEM_OPTIMIZED; openKeyTable and multipartInfoTable (these
+ * stay empty in these tests but we still compare to catch stray open writes).
+ *
+ * <p>Omitted on purpose: snapshotInfoTable, because the new snapshot row is part of the same DB
+ * batch as the checkpoint step, so it appears on the live OM after the batch commits but not
+ * inside the checkpoint taken mid-batch. deletedTable, deletedDirTable, snapshotRenamedTable:
+ * the leader deletes those prefix ranges on the active DB immediately after the checkpoint, so
+ * live and checkpoint intentionally differ. We also do not scan cluster-wide tables (tokens,
+ * secrets, compaction log, other tenants/volumes).
+ *
+ * <p>Later tests worth adding: a second snapshot on the same bucket; writes after the snapshot to
+ * prove the checkpoint slice does not change; multipart uploads with real state; linked buckets
+ * resolving to a source bucket.
+ */
+public class TestOmSnapshotCheckpointDbContent {
+
+  private static final byte[] TEST_KEY_CONTENT = new byte[] {0x61, 0x62, 0x63};
+
+  private static MiniOzoneCluster cluster;
+  private static OzoneConfiguration conf;
+  private static OzoneClient client;
+  private static ObjectStore store;
+
+  /** Mini cluster shared by all tests: three datanodes, snapshots enabled; defrag disabled. */
+  @BeforeAll
+  public static void init()
+      throws IOException, InterruptedException, TimeoutException {
+    conf = new OzoneConfiguration();
+    conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+    conf.setInt(OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL, -1);
+
+    cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(3).build();
+    cluster.waitForClusterToBeReady();
+    client = cluster.newClient();
+    store = client.getObjectStore();
+  }
+
+  @AfterAll
+  public static void shutdown() {
+    IOUtils.closeQuietly(client);
+    if (cluster != null) {
+      cluster.shutdown();
+    }
+  }
+
+  /**
+   * Object store layout: mixed key shapes (flat and path-like), then snapshot and compare tables
+   * for this bucket.
+   */
+  @Test
+  public void testSnapshotCheckpointMatchesLiveObsBucket()
+      throws IOException, InterruptedException, TimeoutException {
+    runLayoutScenario(BucketLayout.OBJECT_STORE,
+        Arrays.asList("k1", "prefix/k2", "prefix/k3"));
+  }
+
+  /**
+   * File-system-optimized layout: data lives in file/dir tables; includes keys under a short path and
+   * one at the bucket root.
+   */
+  @Test
+  public void testSnapshotCheckpointMatchesLiveFsoBucket()
+      throws IOException, InterruptedException, TimeoutException {
+    runLayoutScenario(BucketLayout.FILE_SYSTEM_OPTIMIZED,
+        Arrays.asList("d/k1", "d/k2", "k-root"));
+  }
+
+  /**
+   * Legacy layout: still uses the key table only; flat key names to match default path behavior on
+   * this cluster.
+   */
+  @Test
+  public void testSnapshotCheckpointMatchesLiveLegacyBucket()
+      throws IOException, InterruptedException, TimeoutException {
+    runLayoutScenario(BucketLayout.LEGACY,
+        Arrays.asList("key-a", "key-b", "key-c"));
+  }
+
+  /** Create bucket, write keys, snapshot, wait for checkpoint files, open checkpoint DB, compare. */
+  private void runLayoutScenario(BucketLayout layout, List<String> keyNames)
+      throws IOException, InterruptedException, TimeoutException {
+    OzoneBucket bucket =
+        TestDataUtil.createVolumeAndBucket(client, layout);
+    String volumeName = bucket.getVolumeName();
+    String bucketName = bucket.getName();
+
+    for (String keyName : keyNames) {
+      TestDataUtil.createKey(bucket, keyName, TEST_KEY_CONTENT);
+    }
+
+    String snapshotName = "snap-a";
+    store.createSnapshot(volumeName, bucketName, snapshotName);
+
+    OzoneManager om = cluster.getOzoneManager();
+    OMMetadataManager liveMm = om.getMetadataManager();
+    SnapshotInfo snapshotInfo = liveMm.getSnapshotInfoTable().get(
+        SnapshotInfo.getTableKey(volumeName, bucketName, snapshotName));
+    assertNotNull(snapshotInfo, "Snapshot row should exist after createSnapshot");
+    assertEquals(snapshotName, snapshotInfo.getName());
+
+    String currentPath =
+        OmSnapshotManager.getSnapshotPath(conf, snapshotInfo, 0)
+            + OM_KEY_PREFIX + "CURRENT";
+    GenericTestUtils.waitFor(() -> new File(currentPath).exists(), 1000, 120_000);
+
+    RocksDBCheckpoint checkpoint = new RocksDBCheckpoint(
+        Paths.get(OmSnapshotManager.getSnapshotPath(conf, snapshotInfo, 0)));
+    try (OmMetadataManagerImpl checkpointMm =
+             OmMetadataManagerImpl.createCheckpointMetadataManager(conf, checkpoint)) {
+      TablePrefixInfo prefixes = liveMm.getTableBucketPrefix(volumeName, bucketName);
+      assertBucketPrefixTablesMatch(liveMm, checkpointMm, prefixes, layout);
+    }
+  }
+
+  /** Load every row under the bucket prefix from live vs checkpoint for the tables this test cares about. */
+  private void assertBucketPrefixTablesMatch(
+      OMMetadataManager live,
+      OMMetadataManager checkpoint,
+      TablePrefixInfo prefixes,
+      BucketLayout layout) throws IOException {
+
+    assertPrefixEquals(VOLUME_TABLE, live.getVolumeTable(),
+        checkpoint.getVolumeTable(), prefixes);
+    assertPrefixEquals(BUCKET_TABLE, live.getBucketTable(),
+        checkpoint.getBucketTable(), prefixes);
+
+    if (layout.isFileSystemOptimized()) {
+      assertPrefixEquals(FILE_TABLE, live.getFileTable(),
+          checkpoint.getFileTable(), prefixes);
+      assertPrefixEquals(DIRECTORY_TABLE, live.getDirectoryTable(),
+          checkpoint.getDirectoryTable(), prefixes);
+    } else {
+      assertPrefixEquals(KEY_TABLE,
+          live.getKeyTable(layout),
+          checkpoint.getKeyTable(layout), prefixes);
+    }
+
+    assertPrefixEquals(OPEN_KEY_TABLE,
+        live.getOpenKeyTable(layout),
+        checkpoint.getOpenKeyTable(layout), prefixes);
+    assertPrefixEquals(MULTIPART_INFO_TABLE, live.getMultipartInfoTable(),
+        checkpoint.getMultipartInfoTable(), prefixes);
+  }
+
+  private static <V> void assertPrefixEquals(
+      String tableName,
+      Table<String, V> live,
+      Table<String, V> checkpoint,
+      TablePrefixInfo prefixes) throws IOException {
+    String prefix = prefixes.getTablePrefix(tableName);
+    assertEquals(readPrefix(live, prefix), readPrefix(checkpoint, prefix),
+        tableName);
+  }
+
+  private static <V> SortedMap<String, V> readPrefix(
+      Table<String, V> table, String prefix) throws IOException {
+    SortedMap<String, V> map = new TreeMap<>();
+    if (prefix == null || prefix.isEmpty()) {
+      return map;
+    }
+    try (KeyValueIterator<String, V> it = table.iterator(prefix)) {
+      while (it.hasNext()) {
+        KeyValue<String, V> kv = it.next();

Review Comment:
   `table.iterator(prefix)` commonly seeks to `prefix` but is not necessarily bounded to that prefix range. As written, this can read past the prefix and pull in unrelated rows (potentially the rest of the table), causing false failures and unnecessary work. Fix by stopping iteration once `kv.getKey()` no longer starts with `prefix` (or use an API that iterates over a bounded prefix/range if available).
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10235:
URL: https://github.com/apache/ozone/pull/10235#issuecomment-4747250504

   Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Add integration test comparing OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10235:
URL: https://github.com/apache/ozone/pull/10235#discussion_r3217849205


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotCheckpointDbContent.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DIRECTORY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.MULTIPART_INFO_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.OPEN_KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VOLUME_TABLE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.concurrent.TimeoutException;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.hdds.utils.db.RocksDBCheckpoint;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValueIterator;
+import org.apache.hadoop.hdds.utils.db.TablePrefixInfo;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * HDDS-13217: when an OM snapshot is created, the on-disk RocksDB checkpoint for that snapshot
+ * should contain the same bucket-scoped metadata as the live OM for the tables exercised here.
+ *
+ * <p>Flow: write some keys, call createSnapshot, wait until the checkpoint is materialized, open
+ * it read-only, then for each relevant table scan the key prefix for this volume/bucket on both
+ * the live metadata manager and the checkpoint and assert the maps match.
+ *
+ * <p>Tables compared: volume and bucket rows; key table for OBJECT_STORE and LEGACY layouts;
+ * file and directory tables for FILE_SYSTEM_OPTIMIZED; openKeyTable and multipartInfoTable (these
+ * stay empty in these tests but we still compare to catch stray open writes).
+ *
+ * <p>Omitted on purpose: snapshotInfoTable, because the new snapshot row is part of the same DB
+ * batch as the checkpoint step, so it appears on the live OM after the batch commits but not
+ * inside the checkpoint taken mid-batch. deletedTable, deletedDirTable, snapshotRenamedTable:
+ * the leader deletes those prefix ranges on the active DB immediately after the checkpoint, so
+ * live and checkpoint intentionally differ. We also do not scan cluster-wide tables (tokens,
+ * secrets, compaction log, other tenants/volumes).
+ *
+ * <p>Later tests worth adding: a second snapshot on the same bucket; writes after the snapshot to
+ * prove the checkpoint slice does not change; multipart uploads with real state; linked buckets
+ * resolving to a source bucket.
+ */
+public class TestOmSnapshotCheckpointDbContent {
+
+  private static final byte[] TEST_KEY_CONTENT = new byte[] {0x61, 0x62, 0x63};
+
+  private static MiniOzoneCluster cluster;
+  private static OzoneConfiguration conf;
+  private static OzoneClient client;
+  private static ObjectStore store;
+
+  /** Mini cluster shared by all tests: three datanodes, snapshots enabled; defrag disabled. */
+  @BeforeAll
+  public static void init()
+      throws IOException, InterruptedException, TimeoutException {
+    conf = new OzoneConfiguration();
+    conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+    conf.setInt(OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL, -1);
+
+    cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(3).build();
+    cluster.waitForClusterToBeReady();

Review Comment:
   New cluster creation take ~30 seconds.  If the test is not destructive, can it be added to an existing test class?  Or can it use shared cluster (see HDDS-12183 for example).



##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotCheckpointDbContent.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DIRECTORY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.MULTIPART_INFO_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.OPEN_KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VOLUME_TABLE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.concurrent.TimeoutException;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.hdds.utils.db.RocksDBCheckpoint;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValueIterator;
+import org.apache.hadoop.hdds.utils.db.TablePrefixInfo;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * HDDS-13217: when an OM snapshot is created, the on-disk RocksDB checkpoint for that snapshot
+ * should contain the same bucket-scoped metadata as the live OM for the tables exercised here.
+ *
+ * <p>Flow: write some keys, call createSnapshot, wait until the checkpoint is materialized, open
+ * it read-only, then for each relevant table scan the key prefix for this volume/bucket on both
+ * the live metadata manager and the checkpoint and assert the maps match.
+ *
+ * <p>Tables compared: volume and bucket rows; key table for OBJECT_STORE and LEGACY layouts;
+ * file and directory tables for FILE_SYSTEM_OPTIMIZED; openKeyTable and multipartInfoTable (these
+ * stay empty in these tests but we still compare to catch stray open writes).
+ *
+ * <p>Omitted on purpose: snapshotInfoTable, because the new snapshot row is part of the same DB
+ * batch as the checkpoint step, so it appears on the live OM after the batch commits but not
+ * inside the checkpoint taken mid-batch. deletedTable, deletedDirTable, snapshotRenamedTable:
+ * the leader deletes those prefix ranges on the active DB immediately after the checkpoint, so
+ * live and checkpoint intentionally differ. We also do not scan cluster-wide tables (tokens,
+ * secrets, compaction log, other tenants/volumes).
+ *
+ * <p>Later tests worth adding: a second snapshot on the same bucket; writes after the snapshot to
+ * prove the checkpoint slice does not change; multipart uploads with real state; linked buckets
+ * resolving to a source bucket.
+ */
+public class TestOmSnapshotCheckpointDbContent {
+
+  private static final byte[] TEST_KEY_CONTENT = new byte[] {0x61, 0x62, 0x63};
+
+  private static MiniOzoneCluster cluster;
+  private static OzoneConfiguration conf;
+  private static OzoneClient client;
+  private static ObjectStore store;
+
+  /** Mini cluster shared by all tests: three datanodes, snapshots enabled; defrag disabled. */
+  @BeforeAll
+  public static void init()
+      throws IOException, InterruptedException, TimeoutException {
+    conf = new OzoneConfiguration();
+    conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+    conf.setInt(OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL, -1);
+
+    cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(3).build();
+    cluster.waitForClusterToBeReady();

Review Comment:
   New cluster creation take ~30 seconds.  If the test is not destructive, can it be added to an existing test class?  Or can it use shared cluster? (see HDDS-12183 for example)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10235:
URL: https://github.com/apache/ozone/pull/10235#issuecomment-4686274883

   This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "SaketaChalamchala (via GitHub)" <gi...@apache.org>.
SaketaChalamchala commented on PR #10235:
URL: https://github.com/apache/ozone/pull/10235#issuecomment-4435495909

   The current patch seems to compare the contents of a snapshot and live OM. 
   The intent of the JIRA seems to be to verify the integrity of snapshots after defrag iterations.
   Ex.,
   ```
   Setup - Take snapshots :      S1(at T1)                         S2(at T2)                                S3(at T3)
   
   Test#1: 
   Run Defrag Iteration#1:    S1' = compact(S1)              S2' =  S1' + diff(S1, S2)         S3' =  S1' + diff(S1, S2)  +  diff(S2, S3) 
   Verify contents:                 S1' == S1                             S2' == S2                               S3' == S3
   
   Test#2: 
   Delete S2
   Run Defrag Iteration#2:     S2'' =  compact(S2')             S3'' =  S2'' + diff(S2, S3) 
   Verify contents:                  S2'' == S2                               S3'' == S3
   ```
   
   cc @smengcl could you confirm?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #10235: HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket
URL: https://github.com/apache/ozone/pull/10235


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Add integration test comparing OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on code in PR #10235:
URL: https://github.com/apache/ozone/pull/10235#discussion_r3217849205


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshotCheckpointDbContent.java:
##########
@@ -0,0 +1,241 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.ozone.om.snapshot;
+
+import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
+import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.BUCKET_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.DIRECTORY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.FILE_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.MULTIPART_INFO_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.OPEN_KEY_TABLE;
+import static org.apache.hadoop.ozone.om.codec.OMDBDefinition.VOLUME_TABLE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Paths;
+import java.util.Arrays;
+import java.util.List;
+import java.util.SortedMap;
+import java.util.TreeMap;
+import java.util.concurrent.TimeoutException;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.hdds.utils.db.RocksDBCheckpoint;
+import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValue;
+import org.apache.hadoop.hdds.utils.db.Table.KeyValueIterator;
+import org.apache.hadoop.hdds.utils.db.TablePrefixInfo;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.TestDataUtil;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneBucket;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.om.OMConfigKeys;
+import org.apache.hadoop.ozone.om.OMMetadataManager;
+import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.helpers.BucketLayout;
+import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
+import org.apache.ozone.test.GenericTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * HDDS-13217: when an OM snapshot is created, the on-disk RocksDB checkpoint for that snapshot
+ * should contain the same bucket-scoped metadata as the live OM for the tables exercised here.
+ *
+ * <p>Flow: write some keys, call createSnapshot, wait until the checkpoint is materialized, open
+ * it read-only, then for each relevant table scan the key prefix for this volume/bucket on both
+ * the live metadata manager and the checkpoint and assert the maps match.
+ *
+ * <p>Tables compared: volume and bucket rows; key table for OBJECT_STORE and LEGACY layouts;
+ * file and directory tables for FILE_SYSTEM_OPTIMIZED; openKeyTable and multipartInfoTable (these
+ * stay empty in these tests but we still compare to catch stray open writes).
+ *
+ * <p>Omitted on purpose: snapshotInfoTable, because the new snapshot row is part of the same DB
+ * batch as the checkpoint step, so it appears on the live OM after the batch commits but not
+ * inside the checkpoint taken mid-batch. deletedTable, deletedDirTable, snapshotRenamedTable:
+ * the leader deletes those prefix ranges on the active DB immediately after the checkpoint, so
+ * live and checkpoint intentionally differ. We also do not scan cluster-wide tables (tokens,
+ * secrets, compaction log, other tenants/volumes).
+ *
+ * <p>Later tests worth adding: a second snapshot on the same bucket; writes after the snapshot to
+ * prove the checkpoint slice does not change; multipart uploads with real state; linked buckets
+ * resolving to a source bucket.
+ */
+public class TestOmSnapshotCheckpointDbContent {
+
+  private static final byte[] TEST_KEY_CONTENT = new byte[] {0x61, 0x62, 0x63};
+
+  private static MiniOzoneCluster cluster;
+  private static OzoneConfiguration conf;
+  private static OzoneClient client;
+  private static ObjectStore store;
+
+  /** Mini cluster shared by all tests: three datanodes, snapshots enabled; defrag disabled. */
+  @BeforeAll
+  public static void init()
+      throws IOException, InterruptedException, TimeoutException {
+    conf = new OzoneConfiguration();
+    conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+    conf.setInt(OZONE_SNAPSHOT_DEFRAG_SERVICE_INTERVAL, -1);
+
+    cluster = MiniOzoneCluster.newBuilder(conf).setNumDatanodes(3).build();
+    cluster.waitForClusterToBeReady();

Review Comment:
   New cluster creation takes ~30 seconds.  If the test is not destructive, can it be added to an existing test class?  Or can it use shared cluster? (see HDDS-12183 for example)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-13217. Test to compare OM snapshot checkpoint metadata to live OM for a bucket [ozone]

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #10235:
URL: https://github.com/apache/ozone/pull/10235#issuecomment-4513118562

   > The current patch seems to compare the contents of a snapshot and live OM. The intent of the JIRA seems to be to verify the integrity of snapshots after defrag iterations. Ex.,
   > 
   > ```
   > Setup - Take snapshots :      S1(at T1)                         S2(at T2)                                S3(at T3)
   > 
   > Test#1: 
   > Run Defrag Iteration#1:    S1' = compact(S1)              S2' =  S1' + diff(S1, S2)         S3' =  S1' + diff(S1, S2)  +  diff(S2, S3) 
   > Verify contents:                 S1' == S1                             S2' == S2                               S3' == S3
   > 
   > Test#2: 
   > Delete S2
   > Run Defrag Iteration#2:     S2'' =  compact(S2')             S3'' =  S2'' + diff(S2, S3) 
   > Verify contents:                  S2'' == S2                               S3'' == S3
   > ```
   > 
   > cc @smengcl could you confirm?
   
   @SaketaChalamchala you are right. The intention of the jira is to check defrag integrity before and after. Sorry if it the wording was confusing.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org