HDDS-13612. Track SST file ranges in sst file metadata by swamirishi · Pull Request #8981 · apache/ozone

swamirishi · 2025-08-29T16:41:49Z

What changes were proposed in this pull request?

Snapshot metadata should track sst file key ranges. These ranges would be used for snapshot diff. The snapshot meta should also track the previous snapshotId meta to know the version built from which snapshot and the corresponding version.

What is the link to the Apache JIRA

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

How was this patch tested?

Changed existing unit tests to account for the additional fields

swamirishi · 2025-08-29T16:57:25Z

This commit makes significant changes to the way Ozone's snapshot local data tracks and serializes SST (Sorted String Table) file information. Here’s a summary of all changes:

1. Refactoring SST File Tracking Structure

Old Approach: SST files (both uncompacted and compacted) were tracked using maps:
- Map<String, Set<String>> for uncompacted SST file lists
- Map<Integer, Map<String, Set<String>>> for compacted SST file lists
New Approach:
- Introduces a new class SstFileInfo to encapsulate SST file metadata: file name, start key, end key, column family.
- Compacted/uncompacted SST files are now tracked as lists of SstFileInfo objects grouped by snapshot version in a new VersionMeta class.
- VersionMeta stores:
  - previousSnapshotVersion
  - List<SstFileInfo> sstFiles
- The maps are replaced by Map<Integer, VersionMeta> versionSstFileInfos.

2. Class and Code Structure Changes

New Class:
- SstFileInfo is added, encapsulating SST file details and supporting deep copy, equality, and string representation.
- VersionMeta is added as a static inner class of OmSnapshotLocalData, representing a snapshot version's SST file metadata.
Existing Classes Modified:
- CompactionFileInfo now extends SstFileInfo instead of keeping its own fields for SST file metadata.
- Constructors and methods throughout OmSnapshotLocalData, OmSnapshotLocalDataYaml, and related test classes are updated to use the new data structures.

3. YAML Serialization/Deserialization Improvements

Custom YAML tags and representers for OmSnapshotLocalDataYaml, VersionMeta, and SstFileInfo are introduced for more precise serialization.
The code now omits null properties in YAML output.
Reading/writing YAML files for snapshot data now uses the new structures and custom serialization logic.

4. API and Method Changes

Methods for getting/setting uncompacted and compacted SST files are replaced with versions that use VersionMeta and SstFileInfo.
Methods like addUncompactedSSTFileList and addCompactedSSTFileList are replaced with addVersionSSTFileInfos.

5. Test Updates

All relevant tests (TestOmSnapshotLocalDataYaml, TestOmSnapshotManager) are updated to use the new data structures.
Mocks for LiveFileMetaData now include start/end keys and column family.
Assertions now verify the new model (VersionMeta, SstFileInfo) rather than simple sets/maps.

6. Constants and Field Additions

New string constants added to OzoneConsts for the new YAML field names.
New fields added for previous snapshot IDs and more detailed SST file tracking.

7. Code Clean-Up

Removes unused imports and code related to the old data structure.
Refactors deep copy and equality/hashCode logic to match the new design.

Impact and Motivation

More Structured Metadata: SST file information is now stored as rich objects instead of simple strings, enabling more precise tracking and manipulation.
Future-Proofing: The new model is more extensible for future metadata additions.
Serialization Reliability: Custom YAML serialization ensures the new data structures are reliably persisted and restored.
Test Coverage: The changes are well covered by updates to existing unit tests.

In summary:
This commit replaces set/map-based SST file tracking in snapshot local data with a model based on rich metadata objects (SstFileInfo, VersionMeta), improves YAML serialization/deserialization, and updates all related code and tests to support this more robust and extensible structure.

Change-Id: I822eb168408b65364bc9e009a5818d7f5dfd907f

Change-Id: I2bc385065589e2ea2ffde0c09ace3ab7d4efb5ce

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

Change-Id: Icf245ff856f13ffab73beefc84b743f0de69ef58

Change-Id: I220b088e09d90c7dd86af0f27a0bdfe8243a5ac0

Copilot

Pull Request Overview

This PR enhances the snapshot metadata system by tracking SST file key ranges for snapshot diff functionality. The changes replace the previous approach of tracking simple SST file lists with a more detailed structure that includes key ranges and version metadata.

Introduces SstFileInfo class to track file names, start/end keys, and column families
Refactors snapshot metadata to use version-based tracking with VersionMeta class
Implements object pooling for YAML operations to improve performance

Reviewed Changes

Copilot reviewed 12 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pom.xml	Added commons-pool2 dependency for object pooling
hadoop-ozone/ozone-manager/pom.xml	Added commons-pool2 dependency to ozone-manager module
SstFileInfo.java	New class for tracking SST file metadata with key ranges
CompactionFileInfo.java	Refactored to extend SstFileInfo base class
OmSnapshotLocalData.java	Replaced SST file tracking with VersionMeta structure
OmSnapshotLocalDataYaml.java	Added YAML pooling and updated serialization for new metadata structure
OmSnapshotManager.java	Added YAML object pooling and updated snapshot creation
OzoneConsts.java	Added constants for new YAML field names
Test files	Updated tests to accommodate new metadata structure with mocks

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

Copilot · 2025-09-03T16:23:31Z

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


    // Get the YAML representation
-    final Yaml yaml = getYamlForSnapshotLocalData();
+    try (UncheckedAutoCloseableSupplier<Yaml> yaml = snapshotManager.getSnapshotLocalYaml()) {


The verifyChecksum method should handle the IOException that can be thrown by getSnapshotLocalYaml() instead of letting it propagate as an unchecked exception. Consider wrapping the IOException in a RuntimeException or adding it to the method signature.

Copilot · 2025-09-03T16:23:32Z

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

+    private class RepresentVersionMeta implements Represent {
+      @Override
+      public Node representData(Object data) {
+        VersionMeta meta = (VersionMeta) data;
+        Map<String, Object> map = new java.util.LinkedHashMap<>();
+        map.put(OzoneConsts.OM_SLD_VERSION_META_PREV_SNAP_VERSION, meta.getPreviousSnapshotVersion());
+        map.put(OzoneConsts.OM_SLD_VERSION_META_SST_FILES, meta.getSstFiles());

-    if (!isValid) {
-      LOG.warn("Checksum verification failed for snapshot local data. " +
-          "Stored: {}, Computed: {}", storedChecksum, computedChecksum);
+        return representMapping(SNAPSHOT_VERSION_META_TAG, map, DumperOptions.FlowStyle.BLOCK);
+      }
    }


The cast to VersionMeta on line 159 is unsafe. Consider adding a type check before casting to prevent ClassCastException if an incorrect object type is passed.

Copilot · 2025-09-03T16:23:32Z

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

+    private class RepresentSstFileInfo implements Represent {
+      @Override
+      public Node representData(Object data) {
+        SstFileInfo info = (SstFileInfo) data;
+        Map<String, Object> map = new java.util.LinkedHashMap<>();
+        map.put(OzoneConsts.OM_SST_FILE_INFO_FILE_NAME, info.getFileName());
+        map.put(OzoneConsts.OM_SST_FILE_INFO_START_KEY, info.getStartKey());
+        map.put(OzoneConsts.OM_SST_FILE_INFO_END_KEY, info.getEndKey());
+        map.put(OzoneConsts.OM_SST_FILE_INFO_COL_FAMILY, info.getColumnFamily());
+
+        // Explicitly create a mapping node with the desired tag
+        return representMapping(SST_FILE_INFO_TAG, map, DumperOptions.FlowStyle.BLOCK);
+      }
+    }


The cast to SstFileInfo on line 144 is unsafe. Consider adding a type check before casting to prevent ClassCastException if an incorrect object type is passed.

Copilot · 2025-09-03T16:23:32Z

...ksdb-checkpoint-differ/src/main/java/org/apache/ozone/compaction/log/CompactionFileInfo.java

+    if (!(o instanceof CompactionFileInfo)) {
+      return false;
+    }
+    return super.equals(o) && pruned == ((CompactionFileInfo)o).pruned;


[nitpick] The cast to CompactionFileInfo is unsafe since the type check was performed for CompactionFileInfo. However, consider storing the cast result in a variable for better readability: CompactionFileInfo other = (CompactionFileInfo) o; return super.equals(o) && pruned == other.pruned;

Suggested change

return super.equals(o) && pruned == ((CompactionFileInfo)o).pruned;

CompactionFileInfo other = (CompactionFileInfo) o;

return super.equals(o) && pruned == other.pruned;

SaketaChalamchala · 2025-09-04T23:15:05Z

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

   * Creates a OmSnapshotLocalData object with default values.
   */
-  public OmSnapshotLocalData(Map<String, Set<String>> uncompactedSSTFileList) {
+  public OmSnapshotLocalData(List<LiveFileMetaData> uncompactedSSTFileList, UUID previousSnapshotId) {


What is the reason to store previousSnapshotId in the snapshot YAML file when it is already available in the SnapshotInfoTable?
Do we plan to update the previous snapshot ID in the YAML whenever there is a change to the snapshot chain as well i.e., snapshot delete?

yeah the previousSnapshotId in the snapshot chain could be different. Since we are not updating the previous snapshot version value within the same ratis txn it is impossible to achieve atomicity when done via ratis when we have an independent yaml file.

swamirishi · 2025-09-06T11:52:49Z

@smengcl @SaketaChalamchala @jojochuang Can you take a look at this patch?

jojochuang

I don't think i am doing a quality review, but this looks good to me.

jojochuang · 2025-09-09T01:04:26Z

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

-      }
-    });
-    return sstFileList;
+    return store.getDb().getLiveFilesMetaData().stream()


I remembered seeing a case where getLiveFilesMetaData() took too long (too many sst files?) and the everything came to a halt. This is used inside OMSnapshotCreateResponse() so if that happens it's going to slow down OM for sure.

But I guess there's no way around it. Perhaps we can add a metric to measure the time spent in snapshot create response handler (do we have that already? not sure) so at least we know what happened.

Eventually we can move this out from the createSnapshot flow and move this to a background service after HDDS-13628

smengcl · 2025-09-09T18:04:05Z

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

-  // Map of version to compacted SST file list
-  // Map<version, Map<Table, sstFileList>>
-  private Map<Integer, Map<String, Set<String>>> compactedSSTFileList;
+  private UUID previousSnapshotId;


nit: add javadoc

smengcl · 2025-09-09T18:26:35Z

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

-  // Map of version to compacted SST file list
-  // Map<version, Map<Table, sstFileList>>
-  private Map<Integer, Map<String, Set<String>>> compactedSSTFileList;
+  private UUID previousSnapshotId;


The snapshot meta should also track the previous snapshotId meta to know the version built from which snapshot and the corresponding version.

It occurred to be that we should have a defrag log somewhere to keep track of thing happening on each OM.

I guess we can think about adding some kind of defrag log to a separate file to debug operations similar to the container log file in the datanode

smengcl

Thanks @swamirishi . Structurally lgtm. Comments inline.

Change-Id: Id4b3d6fb8b3138eef663cc35ba8690ae6c6cd5dc

swamirishi · 2025-09-10T14:58:14Z

thank you for reviewing the patch @jojochuang @smengcl @SaketaChalamchala

swamirishi marked this pull request as draft August 29, 2025 16:42

swamirishi requested a review from jojochuang August 29, 2025 17:14

swamirishi changed the title ~~HDDS-13612. Track Sst file ranges in sst file metadata~~ HDDS-13612. Track SST file ranges in sst file metadata Aug 29, 2025

swamirishi requested a review from smengcl August 29, 2025 17:15

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

4d7e2e9

Change-Id: I822eb168408b65364bc9e009a5818d7f5dfd907f

swamirishi force-pushed the HDDS-13612 branch from faa0f19 to 4d7e2e9 Compare August 29, 2025 18:47

swamirishi requested a review from sadanand48 August 29, 2025 18:48

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

swamirishi added 4 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

HDDS-13612. Fix Test case

0bced59

Change-Id: I220b088e09d90c7dd86af0f27a0bdfe8243a5ac0

swamirishi marked this pull request as ready for review September 3, 2025 12:18

jojochuang requested a review from Copilot September 3, 2025 16:21

Copilot AI reviewed Sep 3, 2025

View reviewed changes

jojochuang mentioned this pull request Sep 4, 2025

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

Merged

SaketaChalamchala reviewed Sep 4, 2025

View reviewed changes

jojochuang reviewed Sep 9, 2025

View reviewed changes

smengcl reviewed Sep 9, 2025

View reviewed changes

smengcl approved these changes Sep 9, 2025

View reviewed changes

HDDS-13612. Add javadoc

f643193

Change-Id: Id4b3d6fb8b3138eef663cc35ba8690ae6c6cd5dc

swamirishi merged commit 4f3795b into apache:master Sep 10, 2025
42 checks passed

	return super.equals(o) && pruned == ((CompactionFileInfo)o).pruned;
	CompactionFileInfo other = (CompactionFileInfo) o;
	return super.equals(o) && pruned == other.pruned;

Conversation

swamirishi commented Aug 29, 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!

swamirishi commented Aug 29, 2025

1. Refactoring SST File Tracking Structure

2. Class and Code Structure Changes

3. YAML Serialization/Deserialization Improvements

4. API and Method Changes

5. Test Updates

6. Constants and Field Additions

7. Code Clean-Up

Impact and Motivation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swamirishi Sep 5, 2025

Choose a reason for hiding this comment

Uh oh!

swamirishi commented Sep 6, 2025

Uh oh!

jojochuang left a comment

Choose a reason for hiding this comment

Uh oh!

jojochuang Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

swamirishi Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

smengcl Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

swamirishi Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

smengcl Sep 9, 2025

Choose a reason for hiding this comment

Uh oh!

swamirishi Sep 10, 2025

Choose a reason for hiding this comment

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

swamirishi commented Sep 10, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

swamirishi commented Aug 29, 2025 •

edited

Loading

SaketaChalamchala Sep 4, 2025 •

edited

Loading