aswinshakil commented on code in PR #4541:
URL: https://github.com/apache/ozone/pull/4541#discussion_r1171694924
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMDirectoriesPurgeResponseWithFSO.java:
##########
@@ -130,11 +130,10 @@ public void processPaths(OMMetadataManager
omMetadataManager,
}
RepeatedOmKeyInfo repeatedOmKeyInfo = OmUtils.prepareKeyForDelete(
- keyInfo, null, keyInfo.getUpdateID(), isRatisEnabled);
+ keyInfo, keyInfo.getUpdateID(), isRatisEnabled);
String deletedKey = omMetadataManager
- .getOzoneKey(keyInfo.getVolumeName(), keyInfo.getBucketName(),
Review Comment:
For FSO, here we are using
`/volumeId/bucketId/parentObjId/fileName/objectId`
but`OMKeyDeleteResponseWithFSO` still uses
`/volumeName/bucketName/keyName/objectId`
https://github.com/apache/ozone/blob/5eead92666b374c4bb912f332488acb23f9a81df/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/OMKeyDeleteResponseWithFSO.java#L92-L102
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -473,15 +471,7 @@ public static RepeatedOmKeyInfo
prepareKeyForDelete(OmKeyInfo keyInfo,
// Set the updateID
keyInfo.setUpdateID(trxnLogIndex, isRatisEnabled);
- if (repeatedOmKeyInfo == null) {
Review Comment:
We might still need this because there is a possibility that
`RepeatedOmKeyInfo` can still have more than one entry. Previously
`wrapUncommittedBlocksAsPseudoKey()` in `OmKeyCommitRequest` uses
`RepeatedOmKeyInfo` to delete the uncommitted blocks. In the event, we create
a key and delete the key immediately. With this logic, it might overwrite the
`RepeatedOmKeyInfo` thus losing the information about the uncommitted blocks.
But with snapshot, we change the objectId to 0 for all these uncommitted
blocks to differentiate between uncommitted blocks and committed blocks. Still,
it will cause the same issue as above where we might have multiple entries with
the same key`/volumeName/bucketName/keyName/objectId`
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/key/AbstractOMKeyDeleteResponse.java:
##########
@@ -115,15 +114,15 @@ protected void addDeletionToBatch(
* file table (which is in prefix format) and adds the fullKey
* into the deletedTable
* @param keyName (format: objectId/key)
- * @param fullKeyName (format: vol/buck/key)
+ * @param deleteKeyName (format: <keyname>/keyObjectId)
* @param omKeyInfo
* @throws IOException
*/
protected void addDeletionToBatch(
OMMetadataManager omMetadataManager,
BatchOperation batchOperation,
Table<String, ?> fromTable,
- String keyName, String fullKeyName,
Review Comment:
The `fullKeyName` passed here is different still
`/volumeName/bucketName/keyName/objectId` please check in other places as well.
--
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]
