adoroszlai commented on code in PR #9293:
URL: https://github.com/apache/ozone/pull/9293#discussion_r2529877272
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/file/TestOMRecoverLeaseRequest.java:
##########
@@ -591,8 +592,9 @@ bucketName, keyName, replicationConfig, new
OmKeyLocationInfoGroup(version, new
.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)));
Review Comment:
Metadata can be added before `build()` to avoid extra copy.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -228,7 +228,8 @@ private RecoverLeaseResponse doWork(OzoneManager
ozoneManager,
throw new OMException("Open Key " + keyName + " updated recently and
is inside soft limit period",
KEY_UNDER_LEASE_SOFT_LIMIT_PERIOD);
}
- openKeyInfo.getMetadata().put(OzoneConsts.LEASE_RECOVERY, "true");
+ openKeyInfo = openKeyInfo.withMetadataMutations(
+ metadata -> metadata.put(OzoneConsts.LEASE_RECOVERY, "true"));
openKeyInfo.setUpdateID(transactionLogIndex);
openKeyInfo.setModificationTime(Time.now());
Review Comment:
Let's change to
```java
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).
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequestWithFSO.java:
##########
@@ -202,7 +202,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
Long.parseLong(keyToDelete.getMetadata().get(OzoneConsts.HSYNC_CLIENT_ID)));
openKeyToDelete = OMFileRequest.getOmKeyInfoFromFileTable(true,
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);
Review Comment:
Let's use `toBuilder()` directly here as well to prepare for future changes.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -981,9 +981,11 @@ protected OmKeyInfo prepareFileInfo(
dbKeyInfo.setReplicationConfig(replicationConfig);
// Construct a new metadata map from KeyArgs.
- dbKeyInfo.getMetadata().clear();
- dbKeyInfo.getMetadata().putAll(KeyValueUtil.getFromProtobuf(
- keyArgs.getMetadataList()));
+ dbKeyInfo = dbKeyInfo.withMetadataMutations(metadata -> {
+ metadata.clear();
+ metadata.putAll(KeyValueUtil.getFromProtobuf(
+ keyArgs.getMetadataList()));
+ });
Review Comment:
Let's use `toBuilder()` directly here as well to prepare for future changes.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMKeyCreateRequest.java:
##########
@@ -500,8 +500,7 @@ public void testOverwritingExistingMetadata(
// We have to add the key to the key table, as validateAndUpdateCache only
// updates the cache and not the DB.
OmKeyInfo keyInfo = createOmKeyInfo(volumeName, bucketName, keyName,
- replicationConfig).build();
- keyInfo.setMetadata(initialMetadata);
+ replicationConfig).build().withMetadata(initialMetadata);
Review Comment:
Metadata can be added before `build()` to avoid extra copy.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -274,7 +274,8 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
dbOpenKeyToDeleteKey = omMetadataManager.getOpenKey(volumeName,
bucketName,
keyName,
Long.parseLong(keyToDelete.getMetadata().get(OzoneConsts.HSYNC_CLIENT_ID)));
openKeyToDelete =
omMetadataManager.getOpenKeyTable(getBucketLayout()).get(dbOpenKeyToDeleteKey);
- 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);
Review Comment:
Let's use `toBuilder()` directly here as well to prepare for future changes.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestSnapshotUtils.java:
##########
@@ -68,7 +68,8 @@ private OmKeyInfo createOmKeyInfo(boolean hsync,
List<OmKeyLocationInfoGroup> gr
.setObjectID(objectId)
.build();
if (hsync) {
- keyInfo.getMetadata().put(OzoneConsts.HSYNC_CLIENT_ID, "clientid");
+ keyInfo = keyInfo.withMetadataMutations(metadata ->
+ metadata.put(OzoneConsts.HSYNC_CLIENT_ID, "clientid"));
}
Review Comment:
Metadata can be added before `build()` to avoid extra copy.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java:
##########
@@ -520,10 +520,11 @@ protected OmKeyInfo getOmKeyInfo(long trxnLogIndex,
omKeyInfo.setDataSize(dataSize);
omKeyInfo.setReplicationConfig(dbOpenKeyInfo.getReplicationConfig());
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));
Review Comment:
Let's combine the two updates.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -379,9 +385,11 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager, Execut
dbOpenKey, trxnLogIndex);
// 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));
}
Review Comment:
Let's combine the two updates.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java:
##########
@@ -161,8 +161,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
openKey + "entry is not found in the openKey table",
KEY_NOT_FOUND);
}
- omKeyInfo.getMetadata().putAll(KeyValueUtil.getFromProtobuf(
- keyArgs.getMetadataList()));
+ omKeyInfo = omKeyInfo.withMetadataMutations(metadata ->
+ metadata.putAll(KeyValueUtil.getFromProtobuf(
+ keyArgs.getMetadataList())));
Review Comment:
Let's use `toBuilder()` directly here as well to prepare for future changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
