adoroszlai commented on code in PR #9327:
URL: https://github.com/apache/ozone/pull/9327#discussion_r2542765043
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/WithObjectID.java:
##########
@@ -164,5 +167,10 @@ public long getObjectID() {
public long getUpdateID() {
return updateID;
}
+
+ /** Hook method, customized in subclasses. */
+ public String getObjectInfo() {
Review Comment:
`getObjectInfo` is overridden in value subclasses, but not in builders.
What's more, value classes override `toString()`, but builders do not.
Probably worth a separate sub-task to keep patches from growing huge.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java:
##########
@@ -106,7 +106,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
}
operationResult = omBucketAclOp.test(ozoneAcls, omBucketInfo);
- omBucketInfo.setUpdateID(transactionLogIndex);
+ omBucketInfo = omBucketInfo.toBuilder()
+ .withUpdateID(transactionLogIndex)
+ .build();
Review Comment:
Please combine with the existing builder:
https://github.com/apache/ozone/blob/96f15c74093a00db573bd859e0d7d2c0aafc6653/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/acl/OMBucketAclRequest.java#L126-L127
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequestWithFSO.java:
##########
@@ -206,7 +206,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
.addMetadata(OzoneConsts.OVERWRITTEN_HSYNC_KEY, "true")
.build();
openKeyToDelete.setModificationTime(Time.now());
- openKeyToDelete.setUpdateID(trxnLogIndex);
+ openKeyToDelete = openKeyToDelete.toBuilder()
+ .withUpdateID(trxnLogIndex)
+ .build();
Review Comment:
Please combine with the metadata mutations builder.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCommitPartRequest.java:
##########
@@ -175,7 +175,7 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
// Set Modification time
omKeyInfo.setModificationTime(keyArgs.getModificationTime());
// Set the UpdateID to current transactionLogIndex
- omKeyInfo.setUpdateID(trxnLogIndex);
+ omKeyInfo = omKeyInfo.toBuilder().withUpdateID(trxnLogIndex).build();
Review Comment:
Please combine with the metadata mutations builder.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java:
##########
@@ -531,7 +531,9 @@ protected OmKeyInfo getOmKeyInfo(long trxnLogIndex,
omKeyInfo.setTags(dbOpenKeyInfo.getTags());
}
}
- omKeyInfo.setUpdateID(trxnLogIndex);
+ omKeyInfo = omKeyInfo.toBuilder()
+ .withUpdateID(trxnLogIndex)
+ .build();
Review Comment:
Please combine with the metadata mutations builder.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/file/OMRecoverLeaseRequest.java:
##########
@@ -231,7 +231,9 @@ private RecoverLeaseResponse doWork(OzoneManager
ozoneManager,
openKeyInfo = openKeyInfo.toBuilder()
.addMetadata(OzoneConsts.LEASE_RECOVERY, "true")
.build();
- openKeyInfo.setUpdateID(transactionLogIndex);
+ openKeyInfo = openKeyInfo.toBuilder()
+ .withUpdateID(transactionLogIndex)
+ .build();
Review Comment:
Please combine the builders.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -312,7 +314,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
omKeyInfo.updateLocationInfoList(locationInfoList, false);
// Set the UpdateID to current transactionLogIndex
- omKeyInfo.setUpdateID(trxnLogIndex);
+ omKeyInfo = omKeyInfo.toBuilder()
+ .withUpdateID(trxnLogIndex)
+ .build();
Review Comment:
Please combine with the metadata mutations builder.
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -557,10 +557,9 @@ public static RepeatedOmKeyInfo prepareKeyForDelete(long
bucketId, OmKeyInfo key
}
// Set the updateID
- sanitizedKeyInfo.setUpdateID(trxnLogIndex);
- if (sanitizedKeyInfo != keyInfo) {
- keyInfo.setUpdateID(trxnLogIndex);
- }
+ sanitizedKeyInfo = sanitizedKeyInfo.toBuilder()
+ .withUpdateID(trxnLogIndex)
+ .build();
Review Comment:
Please combine with metadata mutations above to avoid multiple builder
conversions.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1167,7 +1169,9 @@ protected OmKeyInfo wrapUncommittedBlocksAsPseudoKey(
OmKeyInfo pseudoKeyInfo = omKeyInfo.copyObject();
// This is a special marker to indicate that SnapshotDeletingService
// can reclaim this key's blocks unconditionally.
- pseudoKeyInfo.setObjectID(OBJECT_ID_RECLAIM_BLOCKS);
+ pseudoKeyInfo = pseudoKeyInfo.toBuilder()
+ .withObjectID(OBJECT_ID_RECLAIM_BLOCKS)
+ .build();
Review Comment:
We can skip `copyObject`, which just builds without any mutations.
##########
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/om/PrefixParser.java:
##########
@@ -135,7 +135,9 @@ public void parse(String vol, String buck, String db,
info.getVolumeName());
long lastObjectId = info.getObjectID();
WithParentObjectId objectBucketId = new WithParentObjectId();
- objectBucketId.setObjectID(lastObjectId);
+ objectBucketId = objectBucketId.toBuilder()
+ .withObjectID(lastObjectId)
+ .build();
Review Comment:
Let's avoid creating the `WithParentObjectId` instance with default values,
create the builder directly.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequest.java:
##########
@@ -278,7 +278,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager, Execut
.addMetadata(OzoneConsts.OVERWRITTEN_HSYNC_KEY, "true")
.build();
openKeyToDelete.setModificationTime(Time.now());
- openKeyToDelete.setUpdateID(trxnLogIndex);
+ openKeyToDelete = openKeyToDelete.toBuilder()
+ .withUpdateID(trxnLogIndex)
+ .build();
Review Comment:
Please combine the builders.
--
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]
