sumitagrawl commented on code in PR #5183:
URL: https://github.com/apache/ozone/pull/5183#discussion_r1308705290
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCommitRequestWithFSO.java:
##########
@@ -180,15 +181,25 @@ public OMClientResponse
validateAndUpdateCache(OzoneManager ozoneManager,
// creation and key commit, old versions will be just overwritten and
// not kept. Bucket versioning will be effective from the first key
// creation after the knob turned on.
+ boolean isPreviousCommitHsync = false;
Map<String, RepeatedOmKeyInfo> oldKeyVersionsToDeleteMap = null;
OmKeyInfo keyToDelete =
omMetadataManager.getKeyTable(getBucketLayout()).get(dbFileKey);
+ if (null != keyToDelete) {
+ final String clientIdString
+ = String.valueOf(commitKeyRequest.getClientID());
+ isPreviousCommitHsync = java.util.Optional.ofNullable(keyToDelete)
+ .map(WithMetadata::getMetadata)
+ .map(meta -> meta.get(OzoneConsts.HSYNC_CLIENT_ID))
+ .filter(id -> id.equals(clientIdString))
+ .isPresent();
+ }
long correctedSpace = omKeyInfo.getReplicatedSize();
// if keyToDelete isn't null, usedNamespace shouldn't check and
// increase.
- if (keyToDelete != null && isHSync) {
+ if (keyToDelete != null && (isHSync || isPreviousCommitHsync)) {
Review Comment:
The issue here was, when final Hsync commit has come, in commit request,
hsync flag is false. so it was treated as overwrite -- i.e. previous commit
already present as keyToDelete != null, and isHsync as false (going to
overwrite flow), and this is causing deletion of blocks.
deletedTable can still have blocks for deletion for below scenario:
- commit have certain blocks not present in commit request, i.e. uncommited
blocks
Above is fixed alternatively in overwrite
[HDDS-9146](https://issues.apache.org/jira/browse/HDDS-9146), and test case
updated same and covered.
> How to test this? What if we test by hsync an empy file twice and check
deletedTable is empty?
This will always have deletedTable as empty, can not differentiate overwrite
and test same
Existing testcase covered scenario as differentiation using UT, with
oldKeyVersionsToDeleteMap as null in this case.
--
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]
