HDDS-13628. LocalSnapshotMetadata create should track snapshot from checkpoint instead of active object store by swamirishi · Pull Request #8982 · apache/ozone

swamirishi · 2025-08-29T20:56:27Z

What changes were proposed in this pull request?

Currently Snapshot Create incorrectly tracks sst files from AOS db instead of the checkpoint creation. If there is a background compaction that got committed right after creation of checkpoint this could fetch wrong results.

What is the link to the Apache JIRA

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

How was this patch tested?

Existing unit tests

Change-Id: I822eb168408b65364bc9e009a5818d7f5dfd907f

Change-Id: I0462e1581ac928806ffc291aa6705c470d34fa71

…heckpoint instead of active object store Change-Id: I9d74659323746afb78cba94e8f8c95d8028ecb50

Copilot

Pull Request Overview

This PR modifies the snapshot creation process to track SST files from the checkpoint database instead of the active object store (AOS) database. This prevents incorrect tracking when background compactions occur right after checkpoint creation.

Tracks SST files from snapshot checkpoint instead of active object store
Refactors data structures to use more structured metadata representation
Adds new SstFileInfo class to capture detailed SST file metadata

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
TestOmSnapshotManager.java	Updates test to use new VersionMeta structure and checkpoint-based store
TestOmSnapshotLocalDataYaml.java	Updates tests for new YAML structure with SstFileInfo objects
OmSnapshotManager.java	Changes createNewOmSnapshotLocalDataFile to use snapshot store instead of AOS
OmSnapshotLocalDataYaml.java	Adds support for new data structures and YAML serialization
OmSnapshotLocalData.java	Replaces Map-based SST tracking with VersionMeta structure
SstFileInfo.java	New class for SST file metadata
CompactionFileInfo.java	Refactored to extend SstFileInfo
OzoneConsts.java	Updates YAML field constants for new structure

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-08-29T23:00:47Z

...zone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotLocalDataYaml.java


    // Verify fields
-    assertEquals(42, snapshotData.getVersion());
+    assertEquals(44, snapshotData.getVersion());


The test expects version 44 but the setup code sets version to 42. This causes a test assertion failure - the expected version should be 42 to match the setup.

Suggested change

assertEquals(44, snapshotData.getVersion());

assertEquals(42, snapshotData.getVersion());

this would be 44 since version is going to be auto incremented every time a new version is added

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

Copilot · 2025-08-29T23:00:48Z

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

-      throws IOException {
-    Path snapshotLocalDataPath = Paths.get(getSnapshotLocalPropertyYamlPath(omMetadataManager, snapshotInfo));
+  public static void createNewOmSnapshotLocalDataFile(RDBStore snapshotStore, SnapshotInfo snapshotInfo) throws IOException {
+    Path snapshotLocalDataPath = Paths.get(getSnapshotLocalPropertyYamlPath(snapshotStore.getDbLocation().toPath()));


The method signature changed to accept snapshotStore.getDbLocation().toPath() but getSnapshotLocalPropertyYamlPath likely expects different parameters. This may cause a compilation error or runtime failure.

Suggested change

Path snapshotLocalDataPath = Paths.get(getSnapshotLocalPropertyYamlPath(snapshotStore.getDbLocation().toPath()));

Path snapshotLocalDataPath = getSnapshotLocalPropertyYamlPath(snapshotStore.getDbLocation().toPath());

Change-Id: I2bc385065589e2ea2ffde0c09ace3ab7d4efb5ce

Change-Id: I4c04ad1f9482b4a4cb26364ed1bcaa93061d62b3 # Conflicts: # pom.xml

Change-Id: I125db7402f1df9ff7dadc1251d07c0f1908f38c9 # Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java

Change-Id: Icf245ff856f13ffab73beefc84b743f0de69ef58

Change-Id: Ic61bf54ba6a37952b33ff7613f4ca88d88cad4a0

Change-Id: I220b088e09d90c7dd86af0f27a0bdfe8243a5ac0

Change-Id: If5ce88bfa3e2907b07975c405093da6c758f4e13

Change-Id: If02ecec5fd95896ebe90f418c137954cf084272d

jojochuang · 2025-09-04T06:15:32Z

This PR requires #8981

Change-Id: Id4b3d6fb8b3138eef663cc35ba8690ae6c6cd5dc

Change-Id: Ia1995e6bc6a7ea6d00e15c9253734d90dc835e02

Change-Id: I45495c7330afe19b97aed8fde43b12d16015dbf1 # Conflicts: # hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java # hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java

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

smengcl · 2025-09-15T15:48:54Z

...one/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServlet.java

        if (!filename.startsWith("fabricated") &&
-            !filename.startsWith(OZONE_RATIS_SNAPSHOT_COMPLETE_FLAG_NAME)) {
+            !filename.startsWith(OZONE_RATIS_SNAPSHOT_COMPLETE_FLAG_NAME) &&
+            !(filename.equals("archive") && file.getParent().getFileName().toString().startsWith("om.db"))) {


findbugs warning:

M D NP: Possible null pointer dereference in org.apache.hadoop.ozone.om.TestOMDbCheckpointServlet.getFiles(Path, int, Set) due to return value of called method Dereferenced at TestOMDbCheckpointServlet.java:[line 759]

https://github.com/swamirishi/ozone/actions/runs/17618881306/job/50059192799

smengcl

Thanks @swamirishi for the patch.

lgtm apart from the findbugs and doc change request by @SaketaChalamchala

Change-Id: I26d6de5576d5303ffd29255ba7c51d857d8ad15a

Change-Id: I050a3ea531014e9955e8856e1168836e817e43ea

sadanand48

LGTM

Change-Id: I5957fb5ba0c59e9edf88585f618d6bda12057c31

swamirishi · 2025-09-22T11:51:30Z

@sadanand48 @smengcl I had to fix the way I am opening the rocksdb because of the change in logic of the snapshot cache where we wait for the entire transaction to get flushed instead of just the mere check of snapshot directory exists. Can you check the changes made in the last commit once more

Change-Id: Ia6b01ee8078a4949adb2a9250431d3c87311c1c9

smengcl

lgtm. Thanks @swamirishi for the patch.

swamirishi · 2025-09-23T18:26:04Z

Thank you @sadanand48 @smengcl and @SaketaChalamchala for reviewing the patch

swamirishi added 3 commits August 29, 2025 14:47

HDDS-13612. Track Sst file ranges in sst file metadata

4d7e2e9

Change-Id: I822eb168408b65364bc9e009a5818d7f5dfd907f

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

0492725

Change-Id: I0462e1581ac928806ffc291aa6705c470d34fa71

HDDS-13628. LocalSnapshotMetadata create should track snapshot from c…

9c0899f

…heckpoint instead of active object store Change-Id: I9d74659323746afb78cba94e8f8c95d8028ecb50

jojochuang added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Aug 29, 2025

jojochuang requested review from Copilot, siddhantsangwan and smengcl August 29, 2025 23:00

Copilot AI reviewed Aug 29, 2025

View reviewed changes

swamirishi added 7 commits August 29, 2025 23:20

HDDS-13612. Track Sst file ranges in sst file metadata

ffcd6d4

Change-Id: I2bc385065589e2ea2ffde0c09ace3ab7d4efb5ce

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

f0039d2

Change-Id: I4c04ad1f9482b4a4cb26364ed1bcaa93061d62b3 # Conflicts: # pom.xml

HDDS-13612. Fix dependency

e07d450

Change-Id: Icf245ff856f13ffab73beefc84b743f0de69ef58

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

895d0a1

Change-Id: Ic61bf54ba6a37952b33ff7613f4ca88d88cad4a0

HDDS-13612. Fix Test case

0bced59

Change-Id: I220b088e09d90c7dd86af0f27a0bdfe8243a5ac0

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

001dfc7

Change-Id: If5ce88bfa3e2907b07975c405093da6c758f4e13

swamirishi requested a review from sadanand48 September 3, 2025 10:00

HDDS-13628. Fix test case

fb660d0

Change-Id: If02ecec5fd95896ebe90f418c137954cf084272d

swamirishi added 3 commits September 10, 2025 08:09

HDDS-13612. Add javadoc

f643193

Change-Id: Id4b3d6fb8b3138eef663cc35ba8690ae6c6cd5dc

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

b2485c8

Change-Id: Ia1995e6bc6a7ea6d00e15c9253734d90dc835e02

SaketaChalamchala reviewed Sep 15, 2025

View reviewed changes

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

smengcl reviewed Sep 15, 2025

View reviewed changes

smengcl approved these changes Sep 15, 2025

View reviewed changes

smengcl marked this pull request as ready for review September 18, 2025 08:52

swamirishi added 2 commits September 21, 2025 20:09

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

6187e2e

Change-Id: I26d6de5576d5303ffd29255ba7c51d857d8ad15a

HDDS-13628. Fix tests

2dab381

Change-Id: I050a3ea531014e9955e8856e1168836e817e43ea

sadanand48 approved these changes Sep 22, 2025

View reviewed changes

HDDS-13628. Fix findbugs

5862e63

Change-Id: I5957fb5ba0c59e9edf88585f618d6bda12057c31

swamirishi marked this pull request as draft September 22, 2025 11:53

swamirishi marked this pull request as ready for review September 22, 2025 15:21

HDDS-13628. Fix findbugs

1e4d458

Change-Id: Ia6b01ee8078a4949adb2a9250431d3c87311c1c9

smengcl approved these changes Sep 23, 2025

View reviewed changes

swamirishi merged commit c6c4279 into apache:master Sep 23, 2025
42 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HDDS-13628. LocalSnapshotMetadata create should track snapshot from checkpoint instead of active object store#8982

HDDS-13628. LocalSnapshotMetadata create should track snapshot from checkpoint instead of active object store#8982
swamirishi merged 18 commits intoapache:masterfrom
swamirishi:HDDS-13628

swamirishi commented Aug 29, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Aug 29, 2025

Uh oh!

swamirishi Aug 30, 2025

Uh oh!

Uh oh!

Copilot AI Aug 29, 2025

Uh oh!

jojochuang commented Sep 4, 2025

Uh oh!

Uh oh!

smengcl Sep 15, 2025

Uh oh!

smengcl left a comment

Uh oh!

sadanand48 left a comment

Uh oh!

swamirishi commented Sep 22, 2025 •

edited

Loading

Uh oh!

smengcl left a comment

Uh oh!

Uh oh!

swamirishi commented Sep 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	assertEquals(44, snapshotData.getVersion());
	assertEquals(42, snapshotData.getVersion());

	Path snapshotLocalDataPath = Paths.get(getSnapshotLocalPropertyYamlPath(snapshotStore.getDbLocation().toPath()));
	Path snapshotLocalDataPath = getSnapshotLocalPropertyYamlPath(snapshotStore.getDbLocation().toPath());

Conversation

swamirishi commented Aug 29, 2025

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 Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

swamirishi Aug 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Aug 29, 2025

Choose a reason for hiding this comment

Uh oh!

jojochuang commented Sep 4, 2025

Uh oh!

Uh oh!

smengcl Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

sadanand48 left a comment

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

swamirishi commented Sep 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

swamirishi commented Sep 22, 2025 •

edited

Loading