HDDS-13867. SnapshotDiff delta file computation should happen based on LocalDataYaml file by swamirishi · Pull Request #9268 · apache/ozone

swamirishi · 2025-11-08T19:14:04Z

What changes were proposed in this pull request?

SnapshotDiff delta file computation should happen based on LocalDataYaml file instead of reading the rocksdb.
Changing rocksDBCheckpointDiffer to get the SstFileInfos from yaml file to compute the diff directly.
Depends on #9235 and #9269

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-13867

How was this patch tested?

Additional unit tests

Change-Id: Iba47aeb21663dfa407ab71339cef02c0d74b49f2

Change-Id: Ifd2feca1fddb144e4955db025f0b15a2ab1f3bfe

Change-Id: I34536ff06efb7d5a4942853f0fd83942ab398b5f

…otLocalDataManager Change-Id: I34536ff06efb7d5a4942853f0fd83942ab398b5f

Change-Id: I32bcaf2a1fb290f1790c02872a0230cd65586636

Change-Id: I105a2e8178c0444d52de41b99801f4ceb6d57ffd # Conflicts: # hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/util/Checksum.java # hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/util/ObjectSerializer.java # hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/util/YamlSerializer.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java

Change-Id: I985170e38fb8beeb784048e85a08a4c79e1aec97

Change-Id: I33e6e6e825bf23c323ad7ed593d800a11720fa4f

Change-Id: If30b2c766db82adde72145c8ecd3e590ef54cc2d

Change-Id: Id3f2c49050bc3476b9e0f5f51dacb6d9acc4c2f7

Change-Id: I432960725b4c6c55aa906b5780cc3027e41e10db

Change-Id: I3c5514e5bbd251a2b5297d8f074cfde5c71fa543

Change-Id: Ib5a9e6c91bdccba17820263c47eaf2c8400e930d

Change-Id: Ica36e0615c7bc6aa9b6a7f6fafafd0f830d4bafb

Change-Id: I26b66f266bb7677e4b1078f5fcd9f2ce3a651a70 # Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java

Change-Id: I1d93dbc048a42cc55ff1f8ffa420e52f967527b8

Change-Id: I34202928a7a367dd0a1e57219317ff34de352b78

Change-Id: Iad6f26cb71ec921c51ee2d138745df1a2663533f

Change-Id: Ic5f7e249cfb9cb3973cbcd4abd36b22a6ff8f5aa

…calDataProvider Change-Id: I3a004b4b435075a4348960aeed642e8da71e7e72

Change-Id: I06990bc9ab8fc7e1eb7bec255646a650bd8c35fe

Change-Id: I4c6c61c83aa9fadab8ecef854b99dcc0a89a2208

Change-Id: I0e476322372a302572f1fe79cbf2e874bfeac2ed

Change-Id: I31004e0c95dad64411c6fe848501a82f2f773cba

Change-Id: Id317c8b56e8b25c122b68eaf96599b9690d08f79

Change-Id: I3849387d064e093634e69cdaf870d27c1934cda5 # Conflicts: # hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/util/ObjectSerializer.java # hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/util/YamlSerializer.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalData.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotLocalDataYaml.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java

Change-Id: Ie5e5f3dab4324103e8855dd15619d7755f0422e6

Change-Id: I55bd5c3ef7fc32910a9111328638de2edffcd541

Change-Id: I880997d3eebdf378f14c203c61c2d63b2d17552e

Change-Id: I13ba8e2fd012a3c964d657e83496c93a4f55a3be

…nable to resolve to a previous snapshot id Change-Id: I9382a816415880596534060a841b2840fc5c2b1f

Change-Id: Icf0be64110f56a095dff6954fe3af281873bfc49

Change-Id: I98a74e0d6815f28792096f0992e1aca933706963

Change-Id: I85d6072d0826ccc7f81a2579af307bfad4f049a7

Change-Id: Ibcc1fbd3e44d8b2f49afd83d5222c8d76ec51c6a

Change-Id: I06301f780ff7fff2ab652722872b1707501d61ec

Change-Id: Ia4eeab3598fa78fc2606e6efb9a30a12b6a944c3

Change-Id: Ibef23601b26e5e7062a9cf3a6f0a9c5af4862bb0 # Conflicts: # hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

Change-Id: Ic5ea1918e91b86a0b2c410bfc1b4c4206858a2f1

Change-Id: Ic1a5ef57f528fa5e6be0a35b603943666d717ac3

Change-Id: I13eb719258714697f875d0b64d38e1185ba75db1

Change-Id: I8e93e6ec779ef027edd8125fe8cd33e84e49ba95

Change-Id: Id465f7d04daeb61f3638a5345d58fb95ebdfb223 # Conflicts: # hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdb/util/RdbUtil.java # hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/DifferSnapshotInfo.java # hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java # hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java # hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java # hadoop-hdds/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDiffUtils.java # hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/freon/TestOMSnapshotDAG.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java

Change-Id: I450d29c5871cc8bb84a067651c6802b74bcde8bd

Change-Id: I83fe0a362530c9aeddee5f959b135a7b282e617f # Conflicts: # hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/db/StringInMemoryTestTable.java # hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

Copilot

Pull Request Overview

This PR refactors the SnapshotDiff delta file computation to use information from LocalDataYaml files instead of directly reading from RocksDB. This architectural change improves efficiency and aligns with the snapshot management design.

Key changes:

Refactored DifferSnapshotInfo to use SST file information from YAML metadata instead of direct RocksDB access
Updated RocksDBCheckpointDiffer to work with SstFileInfo objects and introduced DifferSnapshotVersion wrapper class
Modified SnapshotDiffManager to use OmSnapshotLocalDataManager for retrieving snapshot metadata

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
`SnapshotDiffManager.java`	Updated to retrieve snapshot metadata from LocalDataManager and pass version mapping to differ; refactored delta file computation to use SstFileInfo objects
`DifferSnapshotInfo.java`	Completely refactored to store SST file information from YAML metadata instead of RocksDB references; changed from database path string to version-based path function
`RocksDBCheckpointDiffer.java`	Added `DifferSnapshotVersion` inner class; removed direct RocksDB file reading methods; updated diff computation to work with SstFileInfo maps instead of file name sets
`RocksDiffUtils.java`	Simplified filtering methods to work directly with SstFileInfo objects; removed RocksDB-dependent filtering logic
`RdbUtil.java`	Updated utility methods to return SstFileInfo objects instead of file path strings
`SstFileInfo.java`	Added `getFilePath()` helper method to construct full file paths
`OmSnapshotManager.java`	Updated SnapshotDiffManager instantiation to include LocalDataManager dependency
`TestSnapshotDiffManager.java`	Updated test mocks to work with new SstFileInfo-based APIs and added mock for LocalDataManager
`TestOMSnapshotDAG.java`	Refactored test to use LocalDataManager for snapshot metadata; updated differ API calls with new parameters
`TestRocksDiffUtils.java`	Updated tests to work with Map-based SstFileInfo filtering instead of Set-based string filtering
`TestRocksDBCheckpointDiffer.java`	Extensively refactored tests to work with SstFileInfo objects and DifferSnapshotVersion wrapper; removed RocksDB mocking

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-13T19:59:06Z

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

+      internalGetSSTDiffList(src, dest, fwdDAGSameFiles, fwdDAGDifferentFiles);
+    } else {
+      Set<SstFileInfo> srcSstFileInfos = new HashSet<>(src.getSstFileMap().values());
+      Set<SstFileInfo> destSstFileInfos = new HashSet<>(src.getSstFileMap().values());


The method getSSTDiffList is using src.getSstFileMap().values() for both srcSstFileInfos and destSstFileInfos on line 832. This appears to be a bug as it should be using dest.getSstFileMap().values() for the destination snapshot.

This will cause the diff computation to incorrectly compare the source snapshot against itself, resulting in inaccurate delta files.

Suggested change

Set<SstFileInfo> destSstFileInfos = new HashSet<>(src.getSstFileMap().values());

Set<SstFileInfo> destSstFileInfos = new HashSet<>(dest.getSstFileMap().values());

Copilot · 2025-11-13T19:59:07Z

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

+      return generation;
+    }
+
+    private Set<String> getSstFiles() {


[nitpick] The variable name getSstFiles returns a Set<String> but reads more like it should return file objects. Consider renaming to getSstFileNames for better clarity about what is being returned.

Suggested change

private Set<String> getSstFiles() {

private Set<String> getSstFileNames() {

This is not the sstFileName but just the id

Copilot · 2025-11-13T19:59:07Z

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

-      String sstFilesDirForSnapDiffJob) {
+      DifferSnapshotInfo dest, Map<Integer, Integer> versionMap, TablePrefixInfo prefixInfo,
+      Set<String> tablesToLookup, String sstFilesDirForSnapDiffJob) throws IOException {
+    int srcVersion = src.getMaxVersion();


The method getMaxVersion() on line 75 returns Integer which can be null if the versionSstFiles map is empty. However, the method is called on line 783 without null checking, which could result in a NullPointerException.

Although line 373-375 in SnapshotDiffManager.java includes an empty check, it's better to make getMaxVersion() return int and throw an exception internally if the map is empty, or document that it may return null and handle it at all call sites.

Copilot · 2025-11-13T19:59:08Z

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

+    private Path getDbPath() {
+      return dbPath;
+    }
+
+    private long getGeneration() {
+      return generation;
+    }
+
+    private Set<String> getSstFiles() {
+      return sstFiles.keySet();
+    }
+
+    private Map<String, SstFileInfo> getSstFileMap() {


[nitpick] The newly added DifferSnapshotVersion class has private getter methods (getDbPath(), getGeneration(), getSstFiles(), getSstFileMap()) on lines 907-921. These private methods are only accessed within the RocksDBCheckpointDiffer class. Consider whether these should be package-private or if the encapsulation is appropriate for this inner/nested class design.

Suggested change

private Path getDbPath() {

return dbPath;

}

private long getGeneration() {

return generation;

}

private Set<String> getSstFiles() {

return sstFiles.keySet();

}

private Map<String, SstFileInfo> getSstFileMap() {

Path getDbPath() {

return dbPath;

}

long getGeneration() {

return generation;

}

Set<String> getSstFiles() {

return sstFiles.keySet();

}

Map<String, SstFileInfo> getSstFileMap() {

private functions is good for private class

Copilot · 2025-11-13T19:59:08Z

...s/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/DifferSnapshotInfo.java

-  private final String dbPath;
-  private final UUID snapshotId;
-  private final long snapshotGeneration;
+  private final UUID id;


[nitpick] The variable name id on line 37 in DifferSnapshotInfo is too generic. Since this represents a snapshot identifier, it should be renamed to snapshotId to match the original naming and improve code clarity.

Copilot · 2025-11-13T19:59:11Z

...one/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

-      deltaFiles.addAll(inputFiles);
+      Set<SstFileInfo> inputFiles = filterRelevantSstFiles(getSSTFileListForSnapshot(fromSnapshot, tablesToLookUp),
+          tablesToLookUp, tablePrefixes);
+      Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();


Access of element annotated with VisibleForTesting found in production code.

Suggested change

Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

// Avoid using getDbLocation() annotated with @VisibleForTesting.

// Use a public API to get the DB path, or refactor to pass it in.

Path fromSnapshotPath = Paths.get(fromSnapshot.getSnapshotPath());

Copilot · 2025-11-13T19:59:11Z

...one/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

+    Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();
+    Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();


Access of element annotated with VisibleForTesting found in production code.

Suggested change

Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDatabasePath();

Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDatabasePath();

Copilot · 2025-11-13T19:59:11Z

...one/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

+    Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();
+    Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();


Access of element annotated with VisibleForTesting found in production code.

Suggested change

Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

Path fromSnapshotPath = Paths.get(fromSnapshot.getMetadataManager().getStore().getDbPath());

Path toSnapshotPath = Paths.get(toSnapshot.getMetadataManager().getStore().getDbPath());

Copilot · 2025-11-13T19:59:12Z

...one/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

+    Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();
+    Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();


Access of element annotated with VisibleForTesting found in production code.

Suggested change

Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

Path fromSnapshotPath = ((RDBStore) fromSnapshot.getMetadataManager().getStore()).getDbPath();

Path toSnapshotPath = ((RDBStore) toSnapshot.getMetadataManager().getStore()).getDbPath();

Unnecessary change

Copilot · 2025-11-13T19:59:12Z

...one/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

+    Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();
+    Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();


Access of element annotated with VisibleForTesting found in production code.

Suggested change

Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbPath().toAbsolutePath();

Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbPath().toAbsolutePath();

Change-Id: I1d7aded575ac6daaa5f78b3be9f1821d94cb24c6

jojochuang

I roughly follows what this is for, but the scope of change is very big.
As far as I can tell, it should not have visible change to end users. All changes are needed to convert the use of snapshot rocksdb to LocalDataYaml.

There are also refactors that shouldn't need to come in through the same PR. But apart from a few javadoc suggestion it looks fine from a quick look.

jojochuang · 2025-11-13T19:56:26Z

...one/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java

-    return RdbUtil.getSSTFilesForComparison(((RDBStore)snapshot
-        .getMetadataManager().getStore()).getDb().getManagedRocksDb(),
-        tablesToLookUp);
+  protected Set<SstFileInfo> getSSTFileListForSnapshot(OmSnapshot snapshot, Set<String> tablesToLookUp) {


it's actually a set, not a list.

jojochuang · 2025-11-13T20:02:31Z

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

   */
-  private String getSSTFullPath(String sstFilenameWithoutExtension,
-      String... dbPaths) {
+  private String getSSTFullPath(String sstFilenameWithoutExtension, Path... dbPaths) {


the update to this helper method is not relevant to this PR and it's semantically the same as before.

yeah I just included the same in this PR since this would be really small change for another PR

jojochuang · 2025-11-13T20:46:32Z

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

   *
   * @param src source snapshot
   * @param dest destination snapshot
   * @param tablesToLookup tablesToLookup set of table (column family) names used to restrict which SST files to return.


please update javadocs.

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

jojochuang · 2025-11-13T21:40:24Z

...-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDBCheckpointDiffer.java

+              .map(i -> i.stream().map(SstFileInfo::getFileName).sorted().collect(Collectors.toList())).orElse(null));
+    } catch (RuntimeException rtEx) {
+      if (!expectingException) {
+        rtEx.printStackTrace();


do not print to stdout.

jojochuang · 2025-11-13T21:41:57Z

...s/rocksdb-checkpoint-differ/src/test/java/org/apache/ozone/rocksdiff/TestRocksDiffUtils.java

-
-  @ParameterizedTest
-  @MethodSource("values")
-  public void testFilterRelevantSstFilesFromDB(String validSSTColumnFamilyName,


why remove the test case?

this test case is not valid anymore. There is only a filter function which doesn't read anything from the db anymore

Change-Id: Iba0dd30952d17d11eae900213437aa4c28564448

Change-Id: Idba640c859ed4906842f9fafebf04825409053ef

swamirishi · 2025-11-14T12:08:05Z

I roughly follows what this is for, but the scope of change is very big. As far as I can tell, it should not have visible change to end users. All changes are needed to convert the use of snapshot rocksdb to LocalDataYaml.

There are also refactors that shouldn't need to come in through the same PR. But apart from a few javadoc suggestion it looks fine from a quick look.

Some of the refactoring changes are unavoidable

jojochuang

Looks good to me. Just a few javadoc suggestions. A few copilot comments look legit.
I'm okay to merge once the CI passes.

jojochuang · 2025-11-17T21:59:33Z

...-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDiffUtils.java

+  /**
+   * Filter sst files based on prefixes.
+   */
+  public static <T> Map<T, SstFileInfo> filterRelevantSstFiles(Map<T, SstFileInfo> inputFiles,


note: like the previous method, the parameter inputFiles is not just an input: the map can be mutated.

changed the variable name and added a note in the javadoc as well

jojochuang · 2025-11-17T22:08:52Z

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

+   * @param prefixInfo TablePrefixInfo to filter irrelevant SST files; can be null.
   * @param tablesToLookup tablesToLookup Set of column-family (table) names to include when reading SST files;
   *                       must be non-null.
   * @return A list of SST files without extension. e.g. ["000050", "000060"]


Suggested change

* @return A list of SST files without extension. e.g. ["000050", "000060"]

* @param useCompactionDag whether to consult the compaction DAG to compute the SST diff.

* If true, the method uses compaction history to produce an

* incremental diff; if false, a full (conservative) diff will be returned.

* @return A list of SST files without extension. e.g. ["000050", "000060"]

jojochuang · 2025-11-17T22:13:40Z

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

+    Optional<List<SstFileInfo>> sstDiffList = getSSTDiffList(srcSnapshotVersion, destSnapshotVersion, prefixInfo,
+        tablesToLookup, srcVersion == 0);


Suggested change

Optional<List<SstFileInfo>> sstDiffList = getSSTDiffList(srcSnapshotVersion, destSnapshotVersion, prefixInfo,

tablesToLookup, srcVersion == 0);

// if the source snapshot is the first version (srcVersion == 0), uses the compaction DAG path;

// otherwise, uses the fallback set-comparison path.

boolean useCompactionDag = srcVersion == 0;

Optional<List<SstFileInfo>> sstDiffList = getSSTDiffList(srcSnapshotVersion, destSnapshotVersion, prefixInfo,

tablesToLookup, useCompactionDag);

Change-Id: Icac0512a6b5663cfda6060b876dd9a331352481a

swamirishi · 2025-11-18T02:38:03Z

thank you @jojochuang for reviewing the PR

swamirishi added 30 commits October 8, 2025 09:39

HDDS-13765. SnapshotLocalData yaml should also track snapshotId

6e241f6

Change-Id: Iba47aeb21663dfa407ab71339cef02c0d74b49f2

HDDS-13627. In memory Manager for Snapshot Local Data

a869500

Change-Id: Ifd2feca1fddb144e4955db025f0b15a2ab1f3bfe

HDDS-13627. In memory Manager for Snapshot Local Data

252d338

Change-Id: I34536ff06efb7d5a4942853f0fd83942ab398b5f

HDDS-13767. Refactor SnapshotLocalDataYaml related code into OmSnapsh…

4099bc6

…otLocalDataManager Change-Id: I34536ff06efb7d5a4942853f0fd83942ab398b5f

HDDS-13767. Fix pmd

e02670c

Change-Id: I32bcaf2a1fb290f1790c02872a0230cd65586636

HDDS-13627. Fix checkstyle

79580e9

Change-Id: I985170e38fb8beeb784048e85a08a4c79e1aec97

Merge remote-tracking branch 'apache/master' into HEAD

2a331ef

Change-Id: I33e6e6e825bf23c323ad7ed593d800a11720fa4f

Merge remote-tracking branch 'apache/master' into HEAD

c4f69e2

Change-Id: If30b2c766db82adde72145c8ecd3e590ef54cc2d

HDDS-13627. Add tests

afbc592

Change-Id: Id3f2c49050bc3476b9e0f5f51dacb6d9acc4c2f7

HDDS-13783. Implement locks for OmSnapshotLocalDataManager

70ac2c7

Change-Id: I432960725b4c6c55aa906b5780cc3027e41e10db

HDDS-13783. Implement locks for OmSnapshotLocalDataManager

b554cc7

Change-Id: I3c5514e5bbd251a2b5297d8f074cfde5c71fa543

HDDS-13783. Refactor inline variable

49eccfa

Change-Id: Ib5a9e6c91bdccba17820263c47eaf2c8400e930d

HDDS-13627. Refactor map data structure

51eda04

Change-Id: Ica36e0615c7bc6aa9b6a7f6fafafd0f830d4bafb

Merge remote-tracking branch 'origin/HDDS-13627' into HEAD

25f766c

Change-Id: I26b66f266bb7677e4b1078f5fcd9f2ce3a651a70 # Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotLocalDataManager.java

HDDS-13783. Add more condition to upsert

96689fa

Change-Id: I1d93dbc048a42cc55ff1f8ffa420e52f967527b8

HDDS-13783. Add java doc comment

0674299

Change-Id: I34202928a7a367dd0a1e57219317ff34de352b78

HDDS-13783. Add java doc comment

5d9fc49

Change-Id: Iad6f26cb71ec921c51ee2d138745df1a2663533f

HDDS-13783. Implement full lock

2d88176

Change-Id: Ic5f7e249cfb9cb3973cbcd4abd36b22a6ff8f5aa

HDDS-13783. Refactor and move modify method into WritableOmSnapshotLo…

a3c4c69

…calDataProvider Change-Id: I3a004b4b435075a4348960aeed642e8da71e7e72

HDDS-13783. Make full lock non static

686d0c7

Change-Id: I06990bc9ab8fc7e1eb7bec255646a650bd8c35fe

HDDS-13783. Fix remove

491a54b

Change-Id: I4c6c61c83aa9fadab8ecef854b99dcc0a89a2208

HDDS-13627. Fix findbugs

5e69ee9

Change-Id: I0e476322372a302572f1fe79cbf2e874bfeac2ed

HDDS-13785. Remove orphan versions from SnapshotLocalData Yaml file

d36622a

Change-Id: I31004e0c95dad64411c6fe848501a82f2f773cba

HDDS-13785. Fix findbugs

ee213d1

Change-Id: Id317c8b56e8b25c122b68eaf96599b9690d08f79

HDDS-13627. Fix tests

a95604e

Change-Id: Ie5e5f3dab4324103e8855dd15619d7755f0422e6

HDDS-13627. remove checksum interface

5a90fcf

Change-Id: I55bd5c3ef7fc32910a9111328638de2edffcd541

HDDS-13627. Fix test failures

20d7d6a

Change-Id: I880997d3eebdf378f14c203c61c2d63b2d17552e

HDDS-13785. Set defrag flag on previous snapshotId update

ae655cb

Change-Id: I13ba8e2fd012a3c964d657e83496c93a4f55a3be

swamirishi force-pushed the HDDS-13867 branch from 30b5293 to 151e887 Compare November 9, 2025 02:56

swamirishi added 15 commits November 9, 2025 08:50

HDDS-13901. OmSnaphshotLocalDataManager should throw IOException if u…

3771ff4

…nable to resolve to a previous snapshot id Change-Id: I9382a816415880596534060a841b2840fc5c2b1f

Merge remote-tracking branch 'origin/HDDS-13901' into HEAD

0736d82

Change-Id: Icf0be64110f56a095dff6954fe3af281873bfc49

Merge remote-tracking branch 'apache/master' into HEAD

894a047

Change-Id: I98a74e0d6815f28792096f0992e1aca933706963

Merge remote-tracking branch 'apache/master' into HEAD

5823347

Change-Id: I85d6072d0826ccc7f81a2579af307bfad4f049a7

Merge remote-tracking branch 'apache/master' into HEAD

e3c9a94

Change-Id: Ibcc1fbd3e44d8b2f49afd83d5222c8d76ec51c6a

HDDS-13849. Address review comments

6fbf8c7

Change-Id: I06301f780ff7fff2ab652722872b1707501d61ec

Merge remote-tracking branch 'origin/HDDS-13849' into HEAD

ed007a4

Change-Id: Ia4eeab3598fa78fc2606e6efb9a30a12b6a944c3

Merge remote-tracking branch 'origin/HDDS-13849' into HEAD

43923cb

Change-Id: Ibef23601b26e5e7062a9cf3a6f0a9c5af4862bb0 # Conflicts: # hadoop-hdds/rocksdb-checkpoint-differ/src/main/java/org/apache/ozone/rocksdiff/RocksDBCheckpointDiffer.java

HDDS-13849. Address review comments

ea64602

Change-Id: Ic5ea1918e91b86a0b2c410bfc1b4c4206858a2f1

Merge remote-tracking branch 'origin/HDDS-13849' into HEAD

0b16ef6

Change-Id: Ic1a5ef57f528fa5e6be0a35b603943666d717ac3

HDDS-13849. Address review comments

1507659

Change-Id: I13eb719258714697f875d0b64d38e1185ba75db1

Merge remote-tracking branch 'origin/HDDS-13849' into HEAD

4c2a175

Change-Id: I8e93e6ec779ef027edd8125fe8cd33e84e49ba95

HDDS-13867. Fix Snaphsot dag diff exception handling

9d4dd26

Change-Id: I450d29c5871cc8bb84a067651c6802b74bcde8bd

jojochuang requested a review from Copilot November 13, 2025 19:49

Copilot started reviewing on behalf of jojochuang November 13, 2025 19:50 View session

Copilot finished reviewing on behalf of jojochuang November 13, 2025 19:51

Copilot AI reviewed Nov 13, 2025

View reviewed changes

HDDS-13867. Fix bug

9736247

Change-Id: I1d7aded575ac6daaa5f78b3be9f1821d94cb24c6

jojochuang reviewed Nov 13, 2025

View reviewed changes

swamirishi added 2 commits November 13, 2025 22:34

HDDS-13867. Add test

2167b03

Change-Id: Iba0dd30952d17d11eae900213437aa4c28564448

HDDS-13867. Address review comments

08156e4

Change-Id: Idba640c859ed4906842f9fafebf04825409053ef

jojochuang reviewed Nov 17, 2025

View reviewed changes

jojochuang marked this pull request as ready for review November 17, 2025 22:43

HDDS-13867. Address review comments

1b984d3

Change-Id: Icac0512a6b5663cfda6060b876dd9a331352481a

swamirishi merged commit dc2b558 into apache:master Nov 18, 2025
83 of 84 checks passed

	Set<SstFileInfo> destSstFileInfos = new HashSet<>(src.getSstFileMap().values());
	Set<SstFileInfo> destSstFileInfos = new HashSet<>(dest.getSstFileMap().values());

	private Set<String> getSstFiles() {
	private Set<String> getSstFileNames() {

-      Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();
+      // Avoid using getDbLocation() annotated with @VisibleForTesting.
+      // Use a public API to get the DB path, or refactor to pass it in.
+      Path fromSnapshotPath = Paths.get(fromSnapshot.getSnapshotPath());

		Path fromSnapshotPath = fromSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();
		Path toSnapshotPath = toSnapshot.getMetadataManager().getStore().getDbLocation().getAbsoluteFile().toPath();

-   * @return A list of SST files without extension. e.g. ["000050", "000060"]
+   * @param useCompactionDag whether to consult the compaction DAG to compute the SST diff.
+   *                       If true, the method uses compaction history to produce an
+   *                       incremental diff; if false, a full (conservative) diff will be returned.
+   * @return A list of SST files without extension. e.g. ["000050", "000060"]

		Optional<List<SstFileInfo>> sstDiffList = getSSTDiffList(srcSnapshotVersion, destSnapshotVersion, prefixInfo,
		tablesToLookup, srcVersion == 0);

-    Optional<List<SstFileInfo>> sstDiffList = getSSTDiffList(srcSnapshotVersion, destSnapshotVersion, prefixInfo,
-        tablesToLookup, srcVersion == 0);
+    // if the source snapshot is the first version (srcVersion == 0), uses the compaction DAG path;
+    // otherwise, uses the fallback set-comparison path.
+    boolean useCompactionDag = srcVersion == 0;
+    Optional<List<SstFileInfo>> sstDiffList = getSSTDiffList(srcSnapshotVersion, destSnapshotVersion, prefixInfo,
+        tablesToLookup, useCompactionDag);

Conversation

swamirishi commented Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 13, 2025

Choose a reason for hiding this comment

Uh oh!

jojochuang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swamirishi commented Nov 14, 2025

Uh oh!

jojochuang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

swamirishi commented Nov 8, 2025 •

edited

Loading

jojochuang Nov 17, 2025 •

edited

Loading