HDDS-14037. Create DBDefinition and corresponding MetadataManager for SnapshotDiff DB by swamirishi · Pull Request #9398 · apache/ozone

swamirishi · 2025-11-30T07:14:20Z

What changes were proposed in this pull request?

Create DBDefinition and corresponding MetadataManager for SnapshotDiff DB

What is the link to the Apache JIRA

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

How was this patch tested?

Just refactoring no additional unit tests required

… SnapshotDiff DB Change-Id: I30eb4fe01cd73fcf8113b2896e6eaff4b77eeb26

adoroszlai · 2025-12-02T05:20:41Z

hadoop-hdds/interface-server/src/main/proto/OmServerProtocol.proto

This file should be in hadoop-ozone/interface-storage (or -client, if it's needed for client/server communication), not in hadoop-hdds.

We don't need this for client server communication

Then move to hadoop-ozone/interface-storage.

Copilot

Pull request overview

This PR refactors the snapshot diff database management by creating dedicated DBDefinition and MetadataManager classes for the SnapshotDiff DB. The refactoring extracts table definitions and constants previously scattered across OmSnapshotManager and consolidates them into a new SnapshotDiffDBDefinition class, along with a corresponding SnapshotDiffMetadataManager interface and implementation for better separation of concerns.

Introduces SnapshotDiffDBDefinition with all table definitions and column family definitions for snapshot diff operations
Creates SnapshotDiffMetadataManager interface and SnapshotDiffMetadataManagerImpl for managing snapshot diff metadata with version control
Adds SnapshotDiffObjectInfo helper class with protobuf serialization support
Updates existing classes to reference the new centralized definitions

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`hadoop-hdds/interface-server/src/main/proto/OmServerProtocol.proto`	New protobuf definition for SnapDiffObjectInfo message used in snapshot diff serialization
`hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/db/package-info.java`	Package documentation for snapshot diff DB management classes
`hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/db/SnapshotDiffDBDefinition.java`	Defines schema and table definitions for snapshot diff database with 6 tables (jobs, reports, purged jobs, from/to snapshot objects, unique IDs)
`hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/db/SnapshotDiffMetadataManager.java`	Interface providing access methods for all snapshot diff metadata tables
`hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/db/SnapshotDiffMetadataManagerImpl.java`	Implementation of metadata manager with DB initialization, version management, and table access
`hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/diff/helper/package-info.java`	Package documentation for snapshot diff helper classes
`hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/diff/helper/SnapshotDiffObjectInfo.java`	Helper class representing object information in snapshot diffs with protobuf codec support
`hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java`	Updated to use centralized table name constants from SnapshotDiffDBDefinition instead of local definitions
`hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotDiffManager.java`	Updated to reference table name constants from SnapshotDiffDBDefinition for column family creation
`hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotDiffManager.java`	Updated test imports to use new SnapshotDiffDBDefinition constants

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

Copilot · 2025-12-05T22:22:37Z

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java

+        Arrays.asList(DEFAULT_COLUMN_FAMILY_NAME, SnapshotDiffDBDefinition.SNAP_DIFF_JOB_TABLE_NAME,
+            SnapshotDiffDBDefinition.SNAP_DIFF_REPORT_TABLE_NAME, SNAP_DIFF_PURGED_JOB_TABLE_NAME));


[nitpick] Inconsistent constant usage: SNAP_DIFF_PURGED_JOB_TABLE_NAME is statically imported (line 56), while SNAP_DIFF_JOB_TABLE_NAME and SNAP_DIFF_REPORT_TABLE_NAME are referenced with the class prefix SnapshotDiffDBDefinition. For consistency, either statically import all three table name constants or use the class prefix for all of them.

Copilot · 2025-12-05T22:22:37Z

hadoop-hdds/interface-server/src/main/proto/OmServerProtocol.proto

+option java_outer_classname = "OmServerProtocolProtos";
+option java_generic_services = true;
+option java_generate_equals_and_hash = true;
+package hadoop.hdds.block;


The proto package hadoop.hdds.block seems inconsistent with the file name OmServerProtocol.proto and the comment "// OM Server protocol". Since this is related to OM (Ozone Manager) server protocol, consider using a more appropriate package name like hadoop.hdds.om or hadoop.hdds.ozone.om instead of hadoop.hdds.block.

Suggested change

package hadoop.hdds.block;

package hadoop.hdds.om;

Copilot · 2025-12-05T22:22:38Z

...e-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/db/SnapshotDiffDBDefinition.java

+      SnapshotDiffObjectInfo.getCodec());
+
+  /**
+   * Contains all the snap diff job intermediate object output for to snapshot.


The documentation comment incorrectly states "Contains all the snap diff job intermediate object output for to snapshot." This should describe what the unique IDs table actually stores (e.g., "Contains unique object IDs encountered during snapshot diff job processing.").

Suggested change

* Contains all the snap diff job intermediate object output for to snapshot.

* Contains unique object IDs encountered during snapshot diff job processing.

* Each entry maps a jobId-objectId pair to a boolean indicating whether the

* object ID is unique for the given job.

Copilot · 2025-12-05T22:22:38Z

...er/src/main/java/org/apache/hadoop/ozone/om/snapshot/diff/helper/SnapshotDiffObjectInfo.java

+    this.objectId = objectId;
+    this.key = key;
+  }
+


The SnapshotDiffObjectInfo class is missing getter methods for its fields (objectId and key). While the class currently only needs to support serialization/deserialization, adding getters would make it more useful when the stored data needs to be read and used by other code.

Suggested change

/**

* Returns the object ID.

*/

public long getObjectId() {

return objectId;

}

/**

* Returns the key name.

*/

public String getKey() {

return key;

}

Copilot · 2025-12-05T22:22:38Z

...er/src/main/java/org/apache/hadoop/ozone/om/snapshot/db/SnapshotDiffMetadataManagerImpl.java

+    if (!versionContentMatches) {
+      writeVersionFileContent(versionFilePath);
+    }


The version file is written after the database is created. If writeVersionFileContent() fails (line 82), the database will exist without a proper version file. On the next startup, this will cause the database to be deleted (line 102). Consider writing the version file before creating the database or immediately after in a try-catch block with rollback logic to ensure consistency.

jojochuang

snapDiffJobTable looks backward compatible.
snapDiffReportTable however, the serialization mapping was updated from byte[], byte[] to string, DiffReportEntry.

swamirishi · 2025-12-06T00:41:25Z

snapDiffJobTable looks backward compatible. snapDiffReportTable however, the serialization mapping was updated from byte[], byte[] to string, DiffReportEntry.

We don't need the db to be ever backward compatible. This is an ephemeral db we would delete it if the version in the version file and code doesn't match and recreate it

jojochuang

Ok this looks more like a refactoring, not really adding new changes.
SnapshotDiffMetadataManagerImpl: I assume you'll be adding test code around it later?

Apart from Attila's suggest it looks good to me.

swamirishi · 2025-12-12T00:03:32Z

SnapshotDiffMetadataManagerImpl: I assume you'll be adding test code around it later?

I don't think we need test as it is just wrapper interface on DBStore but I can add something later

Change-Id: I385d44a6390218b8ff67b9cbdc5908222118ad78

Change-Id: I0db942ce8c1fad1ad621221ecf1ad8b2cf2224a5

swamirishi · 2025-12-12T15:59:02Z

thanks for the review @jojochuang and @adoroszlai

swamirishi requested review from hemantk-12 and jojochuang November 30, 2025 07:14

HDDS-14037. Create DBDefinition and corresponding MetadataManager for…

a7fea98

… SnapshotDiff DB Change-Id: I30eb4fe01cd73fcf8113b2896e6eaff4b77eeb26

swamirishi force-pushed the HDDS-14037 branch from 00f0e79 to a7fea98 Compare November 30, 2025 07:58

swamirishi added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Nov 30, 2025

jojochuang requested review from sadanand48 and smengcl December 1, 2025 17:27

adoroszlai reviewed Dec 2, 2025

View reviewed changes

jojochuang requested a review from Copilot December 5, 2025 22:14

Copilot started reviewing on behalf of jojochuang December 5, 2025 22:14 View session

Copilot AI reviewed Dec 5, 2025

View reviewed changes

jojochuang reviewed Dec 5, 2025

View reviewed changes

jojochuang reviewed Dec 9, 2025

View reviewed changes

swamirishi added 2 commits December 12, 2025 02:54

HDDS-14037. Move SnapshotDiffObjectInfo proto to interface storage

5e3b2b6

Change-Id: I385d44a6390218b8ff67b9cbdc5908222118ad78

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

d1f8851

Change-Id: I0db942ce8c1fad1ad621221ecf1ad8b2cf2224a5

swamirishi marked this pull request as ready for review December 12, 2025 07:59

swamirishi merged commit 89df7ed into apache:master Dec 12, 2025
55 checks passed

		Arrays.asList(DEFAULT_COLUMN_FAMILY_NAME, SnapshotDiffDBDefinition.SNAP_DIFF_JOB_TABLE_NAME,
		SnapshotDiffDBDefinition.SNAP_DIFF_REPORT_TABLE_NAME, SNAP_DIFF_PURGED_JOB_TABLE_NAME));

-   * Contains all the snap diff job intermediate object output for to snapshot.
+   * Contains unique object IDs encountered during snapshot diff job processing.
+   * Each entry maps a jobId-objectId pair to a boolean indicating whether the
+   * object ID is unique for the given job.

+  /**
+   * Returns the object ID.
+   */
+  public long getObjectId() {
+    return objectId;
+  }
+  /**
+   * Returns the key name.
+   */
+  public String getKey() {
+    return key;
+  }

Conversation

swamirishi commented Nov 30, 2025

What changes were proposed in this pull request?

What is the link to the Apache JIRA

How was this patch tested?

Uh oh!

adoroszlai Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

swamirishi Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

adoroszlai Dec 7, 2025

Choose a reason for hiding this comment

Uh oh!

swamirishi Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

Uh oh!

jojochuang left a comment

Choose a reason for hiding this comment

Uh oh!

swamirishi commented Dec 6, 2025

Uh oh!

jojochuang left a comment

Choose a reason for hiding this comment

Uh oh!

swamirishi commented Dec 12, 2025

Uh oh!

Uh oh!

swamirishi commented Dec 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants