neils-dev commented on code in PR #4021:
URL: https://github.com/apache/ozone/pull/4021#discussion_r1047931585
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/key/TestOMDirectoriesPurgeRequestAndResponse.java:
##########
@@ -52,6 +53,13 @@ public class TestOMDirectoriesPurgeRequestAndResponse
extends TestOMKeyRequest {
* Creates volume, bucket and key entries and adds to OM DB and then
* deletes these keys to move them to deletedKeys table.
*/
+ @Before
+ public void cleanup() throws Exception {
+ String bucketKey = omMetadataManager.getBucketKey(volumeName,
+ bucketName);
+ omMetadataManager.getBucketTable().delete(bucketKey);
+ }
+
Review Comment:
Appears to be unnecessary. Remove? Tests pass without. Also `bucketKey`
appears to never be in `bucketTable` when called.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java:
##########
@@ -75,12 +75,13 @@ public OMClientResponse validateAndUpdateCache(OzoneManager
ozoneManager,
volumeName, bucketName);
lockSet.add(volBucketPair);
}
- OmBucketInfo omBucketInfo = getBucketInfo(omMetadataManager,
- volumeName, bucketName);
+ updateBucketInfo(volBucketInfoMap, omMetadataManager, path,
+ volumeName, bucketName, volBucketPair);
+ OmBucketInfo omBucketInfo = volBucketInfoMap.get(volBucketPair);
// bucketInfo can be null in case of delete volume or bucket
+ // or key does not belong to bucket as bucket is recreated
if (null != omBucketInfo) {
omBucketInfo.incrUsedNamespace(-1L);
- volBucketInfoMap.putIfAbsent(volBucketPair, omBucketInfo);
}
}
Review Comment:
The problem appears to stem from using the `volume,bucket` _pair_ to
identify the `bucketInfo `when it _**should be**_ the `objectID` that uniquely
selects the `bucketInfo`. Instead of creating the `updateBucketInfo()` to get
the `omBucketInfo` from the first volume,bucket key entry in the
`volBucketInfoMap` of lines 78-80,
https://github.com/apache/ozone/blob/e4aa45b9e33c99c5c278f461b7afe1758c9938ce/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java#L78-L80
test for `objectID` of volume,bucket pair in
`getBucketInfo(omMetadataManager...)` is same as `bucketID` of
`PurgePathRequest`. This results in checking if `objectID `of bucket _is same_
as `bucketID` of request. If they are the same, update the `bucketInfo` and
`volBucketInfoMap` in lines,
https://github.com/apache/ozone/blob/d6f63bf2e53ac8a4220ee829d8bbb2427dd7d3fa/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java#L82-L84
so,
```
if (omBucketInfo.getObjectID() == path.getBucketId()) {
omBucketInfo.incrUsedNamespace(-1L);
volBucketInfoMap.putIfAbsent(volBucketPair, omBucketInfo);
}
```
With this change the correct objectID is chosen to update.
Is there a reason why the `updateBucketInfo()` is necessary other than to
ensure the correct bucketInfo is updated? Otherwise this appears to cleanup
the code considerably, resolves the bucket re-create issue and passes all test
cases.
--
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]
