HDDS-13070. OM Follower changes to create and place sst files from hardlink file. #8761

sadanand48 · 2025-07-08T08:06:25Z

What changes were proposed in this pull request?

OM Follower changes to create and place sst files from hardlink file. This sets the default servlet provider as OMDBCheckpointServletInodeBasedXfer

What is the link to the Apache JIRA

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

How was this patch tested?

Existing unit tests i.e TestOMRatisSnapshots/TestOMRatisSnapshotProvider

...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java

smengcl

Looks like the most critical thing this patch does is switching to OMDBCheckpointServletInodeBasedXfer. Apart from that there is a minor factoring for source files clean up.

+1. Pls take care of @jojochuang 's comment as well before we merge this.

jojochuang · 2025-07-14T19:52:05Z

Does not compile!

…rdlink file

...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java

sadanand48 · 2025-07-16T06:35:56Z

The test failure looks related to the change, I'm checking this.

jojochuang · 2025-07-18T21:41:33Z

There are fewer test errors but still a few left.

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

Copilot

Pull Request Overview

This PR implements changes to enable OM Follower to create and place SST files from hardlink files, setting the default servlet provider to OMDBCheckpointServletInodeBasedXfer. The implementation adds functionality to delete source files after creating hardlinks and updates the file handling logic to work with general files rather than just SST files.

Key changes:

Modified createHardLinks method to optionally delete source files after creating hardlinks
Updated file scanning logic from SST-specific to general file handling
Changed default servlet to use inode-based transfer mechanism

Reviewed Changes

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

Show a summary per file

File	Description
OmSnapshotUtils.java	Enhanced `createHardLinks` method with source file deletion capability and improved error handling
HAUtils.java	Renamed and generalized `getExistingSstFiles` to `getExistingFiles` for broader file type support
OzoneManagerHttpServer.java	Changed default servlet from `OMDBCheckpointServlet` to `OMDBCheckpointServletInodeBasedXfer`
Multiple test files	Updated method calls and test logic to use the new file handling approach
OMDBCheckpointServletInodeBasedXfer.java	Simplified logging parameter from `excludedSstList` to `receivedSstFiles`

Comments suppressed due to low confidence (1)

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java:334

The variable 'sstList' is misleading since the method now returns all files, not just SST files. It should be renamed to 'fileList' or similar to reflect its actual content.

      sstList = files.filter(p -> p.toFile().isFile())

...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java

...op-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java

hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestRDBSnapshotProvider.java

jojochuang

LGTM. A few minior suggestions that can be address later.

jojochuang · 2025-07-21T21:42:17Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java

+   * @return the list of file names. If db not exist, will return empty list
   */
-  public static List<String> getExistingSstFiles(File db) throws IOException {
+  public static List<String> getExistingFiles(File db) throws IOException {


To be honest getExistingFiles() shouldn't be in the HAUtils. Instead it belongs to RocksDBUtils.

jojochuang · 2025-07-21T21:46:27Z

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java

-      sstList =
-          files.filter(path -> path.toString().endsWith(ROCKSDB_SST_SUFFIX)).
-              map(p -> p.toString().substring(truncateLength)).
+      sstList = files.filter(p -> p.toFile().isFile())


A potential problem is flagged by Gemini: the files object could contain relative path rather than the file name only. It's okay though for rocksdb as the directory is flat with no subdirectories.

jojochuang#598 (comment)

jojochuang · 2025-07-21T22:08:14Z

Thanks @SaketaChalamchala and @sadanand48 !

adoroszlai · 2025-07-22T03:49:29Z

When merging PRs, please remove co-author information if it's the same person with different email address.

* master: (730 commits) HDDS-13083. Handle cases where block deletion generates tree file before scanner (apache#8565) HDDS-12982. Reduce log level for snapshot validation failure (apache#8851) HDDS-13396. Documentation: Improve the top-level overview page for new users. (apache#8753) HDDS-13176. containerIds table value format change to proto from string (apache#8589) HDDS-13449. Incorrect Interrupt Handling for DirectoryDeletingService and KeyDeletingService (apache#8817) HDDS-2453. Add Freon tests for S3 MPU Keys (apache#8803) HDDS-13237. Container data checksum should contain block IDs. (apache#8773) HDDS-13489. Fix SCMBlockdeleting unnecessary iteration in corner case. (apache#8847) HDDS-13464. Make ozone.snapshot.filtering.service.interval reconfigurable (apache#8825) HDDS-13473. Amend validation for OZONE_OM_SNAPSHOT_DB_MAX_OPEN_FILES (apache#8829) HDDS-13435. Add an OzoneManagerAuthorizer interface (apache#8840) HDDS-8565. Recon memory leak in NSSummary (apache#8823). HDDS-12852. Implement a sliding window counter utility (apache#8498) HDDS-12000. Add unit test for RatisContainerSafeModeRule and ECContainerSafeModeRule (apache#8801) HDDS-13092. Container scanner should trigger volume scan when marking a container unhealthy (apache#8603) HDDS-13070. OM Follower changes to create and place sst files from hardlink file. (apache#8761) HDDS-13482. Mark testWriteStateMachineDataIdempotencyWithClosedContainer as flaky HDDS-13481. Fix success latency metric in SCM panels of deletion grafana dashboard (apache#8835) HDDS-13468. Update default value of ozone.scm.ha.dbtransactionbuffer.flush.interval. (apache#8834) HDDS-13410. Control block deletion for each DN from SCM. (apache#8767) ... hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaInfo.java hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestReconcileSubcommand.java

…rdlink file. (apache#8761) Co-authored-by: Sadanand Shenoy <[email protected]>

… from hardlink file. (apache#8761)" This reverts commit c98c10b. Conflicts: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java Change-Id: Ie562cb0b08b60c6451bac23051807b303d77bb9d

… from hardlink file. (#8761)" This reverts commit c98c10b. Conflicts: hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMDbCheckpointServletInodeBasedXfer.java hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java Change-Id: Ie562cb0b08b60c6451bac23051807b303d77bb9d

sadanand48 added the snapshot https://issues.apache.org/jira/browse/HDDS-6517 label Jul 8, 2025

jojochuang requested review from jojochuang, smengcl and swamirishi July 8, 2025 13:59

jojochuang reviewed Jul 8, 2025

View reviewed changes

...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java Outdated Show resolved Hide resolved

jojochuang reviewed Jul 11, 2025

View reviewed changes

...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java Outdated Show resolved Hide resolved

smengcl approved these changes Jul 12, 2025

View reviewed changes

smengcl marked this pull request as ready for review July 12, 2025 01:14

Sadanand Shenoy added 4 commits July 15, 2025 12:53

HDDS-13070. OM Follower changes to create and place sst files from ha…

dbba48f

…rdlink file

address comment

9f0dbf9

findbugs fix

6559738

fix compile

7f2e037

sadanand48 force-pushed the HDDS-13070 branch from 78a71b4 to 7f2e037 Compare July 15, 2025 07:47

address comment

7b8cbee

SaketaChalamchala reviewed Jul 16, 2025

View reviewed changes

...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/OmSnapshotUtils.java Show resolved Hide resolved

ivandika3 marked this pull request as draft July 16, 2025 01:35

Sadanand Shenoy added 3 commits July 18, 2025 14:32

fix 1

4f6bb79

checkstyle

16f55dc

fix 2

60f7af5

fix 3

ea5b898

sadanand48 commented Jul 20, 2025

View reviewed changes

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java Show resolved Hide resolved

sadanand48 marked this pull request as ready for review July 20, 2025 09:03

jojochuang requested review from SaketaChalamchala, Copilot, jojochuang and smengcl and removed request for SaketaChalamchala July 20, 2025 19:46

Copilot AI reviewed Jul 20, 2025

View reviewed changes

jojochuang reviewed Jul 21, 2025

View reviewed changes

hadoop-hdds/framework/src/test/java/org/apache/hadoop/hdds/utils/TestRDBSnapshotProvider.java Outdated Show resolved Hide resolved

address comments

b21cda3

jojochuang approved these changes Jul 21, 2025

View reviewed changes

jojochuang merged commit c98c10b into apache:master Jul 21, 2025
80 of 82 checks passed

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Jul 31, 2025

HDDS-13070. OM Follower changes to create and place sst files from ha…

5af26cf

…rdlink file. (apache#8761) Co-authored-by: Sadanand Shenoy <[email protected]>

jojochuang mentioned this pull request Nov 20, 2025

HDDS-13908. Disable OMDBCheckpointServletInodeBasedXfer in Ozone 2.1 #9333

Closed

HDDS-13070. OM Follower changes to create and place sst files from hardlink file. #8761

HDDS-13070. OM Follower changes to create and place sst files from hardlink file. #8761

Uh oh!

Conversation

sadanand48 commented Jul 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!

Uh oh!

Uh oh!

smengcl left a comment

Choose a reason for hiding this comment

Uh oh!

jojochuang commented Jul 14, 2025

Uh oh!

Uh oh!

sadanand48 commented Jul 16, 2025

Uh oh!

jojochuang commented Jul 18, 2025

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jojochuang left a comment

Choose a reason for hiding this comment

Uh oh!

jojochuang Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

jojochuang Jul 21, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jojochuang commented Jul 21, 2025

Uh oh!

adoroszlai commented Jul 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

sadanand48 commented Jul 8, 2025 •

edited

Loading