HDDS-10388. Make WithMetadata immutable #9293

echonesis · 2025-11-14T09:21:46Z

What changes were proposed in this pull request?

This PR makes WithMetadata immutable. See #6193 for similar change and make them consistent.

Essential modifications:

Add @Immutable to WithMetadata
Modify OmKeyInfo w.r.t. usage under RpcClient

What is the link to the Apache JIRA

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

How was this patch tested?

GitHub Action CI: https://github.com/echonesis/ozone/actions/runs/19369580582
Local Check

$ hadoop-ozone/dev-support/checks/findbugs.sh -Dspotbugs.version=4.8.3.0
$ grep -c 'WithMetadata' target/findbugs/summary.txt
0

adoroszlai

Thanks @echonesis for the patch, nicely done.

adoroszlai · 2025-11-15T13:08:17Z

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

        .build();
    if (hsync) {
-      keyInfo.getMetadata().put(OzoneConsts.HSYNC_CLIENT_ID, "clientid");
+      keyInfo = keyInfo.withMetadataMutations(metadata ->
+          metadata.put(OzoneConsts.HSYNC_CLIENT_ID, "clientid"));
    }


Metadata can be added before build() to avoid extra copy.

adoroszlai · 2025-11-15T13:08:32Z

...one-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java

    OmKeyInfo keyInfo = createOmKeyInfo(volumeName, bucketName, keyName,
-        replicationConfig).build();
-    keyInfo.setMetadata(initialMetadata);
+        replicationConfig).build().withMetadata(initialMetadata);


Metadata can be added before build() to avoid extra copy.

adoroszlai · 2025-11-15T13:09:00Z

...manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMRecoverLeaseRequest.java

        .build();
    omKeyInfo.appendNewBlocks(locationList, false);
    if (hsyncFlag) {
-      omKeyInfo.getMetadata().put(OzoneConsts.HSYNC_CLIENT_ID,
-          String.valueOf(clientID));
+      omKeyInfo = omKeyInfo.withMetadataMutations(metadata ->
+          metadata.put(OzoneConsts.HSYNC_CLIENT_ID,
+              String.valueOf(clientID)));


Metadata can be added before build() to avoid extra copy.

adoroszlai · 2025-11-15T13:16:52Z

...one-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java

      }
-      openKeyInfo.getMetadata().put(OzoneConsts.LEASE_RECOVERY, "true");
+      openKeyInfo = openKeyInfo.withMetadataMutations(
+          metadata -> metadata.put(OzoneConsts.LEASE_RECOVERY, "true"));
      openKeyInfo.setUpdateID(transactionLogIndex);
      openKeyInfo.setModificationTime(Time.now());


Let's change to

openKeyInfo = openKeyInfo.toBuilder() .addMetadata(...) .build();

to prepare for upcoming changes making WithObjectID / OmKeyInfo immutable (where we can simply move setUpdateID / setModificationTime into the builder chain).

adoroszlai · 2025-11-15T13:18:00Z

...e/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java

+        openKeyToDelete = openKeyToDelete.withMetadataMutations(
+            metadata -> metadata.put(OzoneConsts.OVERWRITTEN_HSYNC_KEY, "true"));
        openKeyToDelete.setModificationTime(Time.now());
        openKeyToDelete.setUpdateID(trxnLogIndex);


Let's use toBuilder() directly here as well to prepare for future changes.

adoroszlai · 2025-11-15T13:20:54Z

...e/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java

        // Prevent hsync metadata from getting committed to the final key
-        omKeyInfo.getMetadata().remove(OzoneConsts.HSYNC_CLIENT_ID);
+        omKeyInfo = omKeyInfo.withMetadataMutations(
+            metadata -> metadata.remove(OzoneConsts.HSYNC_CLIENT_ID));
        if (isRecovery) {
-          omKeyInfo.getMetadata().remove(OzoneConsts.LEASE_RECOVERY);
+          omKeyInfo = omKeyInfo.withMetadataMutations(
+              metadata -> metadata.remove(OzoneConsts.LEASE_RECOVERY));
        }


Let's combine the two updates.

adoroszlai · 2025-11-15T13:21:46Z

...-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequestWithFSO.java

            omMetadataManager, dbOpenKeyToDeleteKey, keyName);
-        openKeyToDelete.getMetadata().put(OzoneConsts.OVERWRITTEN_HSYNC_KEY, "true");
+        openKeyToDelete = openKeyToDelete.withMetadataMutations(
+            metadata -> metadata.put(OzoneConsts.OVERWRITTEN_HSYNC_KEY, "true"));
        openKeyToDelete.setModificationTime(Time.now());
        openKeyToDelete.setUpdateID(trxnLogIndex);


Let's use toBuilder() directly here as well to prepare for future changes.

adoroszlai · 2025-11-15T13:25:52Z

...p-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java

+      dbKeyInfo = dbKeyInfo.withMetadataMutations(metadata -> {
+        metadata.clear();
+        metadata.putAll(KeyValueUtil.getFromProtobuf(
+            keyArgs.getMetadataList()));
+      });


Let's use toBuilder() directly here as well to prepare for future changes.

adoroszlai · 2025-11-15T13:26:36Z

...java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java

+      omKeyInfo = omKeyInfo.withMetadataMutations(metadata ->
+          metadata.putAll(KeyValueUtil.getFromProtobuf(
+              keyArgs.getMetadataList())));


Let's use toBuilder() directly here as well to prepare for future changes.

adoroszlai · 2025-11-15T13:27:23Z

...n/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java

      if (dbOpenKeyInfo.getMetadata() != null) {
-        omKeyInfo.setMetadata(dbOpenKeyInfo.getMetadata());
+        omKeyInfo = omKeyInfo.withMetadata(dbOpenKeyInfo.getMetadata());
      }
-      omKeyInfo.getMetadata().put(OzoneConsts.ETAG,
-          multipartUploadedKeyHash(partKeyInfoMap));
+      final String multipartHash = multipartUploadedKeyHash(partKeyInfoMap);
+      omKeyInfo = omKeyInfo.withMetadataMutations(
+          metadata -> metadata.put(OzoneConsts.ETAG, multipartHash));


Let's combine the two updates.

adoroszlai · 2025-11-16T07:30:40Z

Thanks @echonesis for the patch.

echonesis · 2025-11-16T13:06:18Z

Thanks @adoroszlai for the review and merging.

HDDS-10388. Make WithMetadata immutable

8225053

echonesis marked this pull request as draft November 14, 2025 09:57

echonesis added 2 commits November 14, 2025 23:28

fix: modify getMetadata Immutable issue

e11d63d

fix: update checkstyle

06cc308

echonesis marked this pull request as ready for review November 15, 2025 04:02

adoroszlai reviewed Nov 15, 2025

View reviewed changes

fix: update w.r.t. code review

5ba971a

adoroszlai approved these changes Nov 15, 2025

View reviewed changes

adoroszlai merged commit f4660a2 into apache:master Nov 16, 2025
43 checks passed

echonesis deleted the HDDS-10388 branch November 16, 2025 13:09

HDDS-10388. Make WithMetadata immutable #9293

HDDS-10388. Make WithMetadata immutable #9293

Uh oh!

Conversation

echonesis commented Nov 14, 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!

adoroszlai 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!

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!

adoroszlai commented Nov 16, 2025

Uh oh!

echonesis commented Nov 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

echonesis commented Nov 14, 2025 •

edited

Loading