hemantk-12 commented on code in PR #4573:
URL: https://github.com/apache/ozone/pull/4573#discussion_r1183121609
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java:
##########
@@ -1108,8 +1111,14 @@ public List<OmBucketInfo> listBuckets(final String
volumeName,
// We should return only the keys, whose keys match with prefix and
// the keys after the startBucket.
if (key.startsWith(seekPrefix) && key.compareTo(startKey) >= 0) {
- result.add(omBucketInfo);
- currentCount++;
+ if (isSnapshot && !listSnapshot(
Review Comment:
`listSnapshot` is an expensive operation to go over for each bucket. It can
be optimized by one the following.
Suggestion1: Use `SnapshotChainManager` to check if buckets contains
snapshot since key in `snapshotChainPath` is `/vol/bucket`. You can add a
helper function which uses
[snapshotChainPath](https://github.com/apache/ozone/blob/8515c13a1eda56d16b531b67246e99b932c52318/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/SnapshotChainManager.java#L48).
One problem, I can think of, is consistency with this approach. When some
`/vol/bucket` gets added to `snapshotChainPath` but [later gets removed because
of some
failure](https://github.com/apache/ozone/blob/8515c13a1eda56d16b531b67246e99b932c52318/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotCreateRequest.java#L198).
I don't know how big it would be a problem tho.
Suggestion2: the other approach is you could traverse the snapshot table
once as mention by @prashantpogde
[here](https://github.com/apache/ozone/pull/4573#issuecomment-1532061676), get
all the snapshotted buckets (Set<String> snapshottedBuckets), keep it memory
for the request and use it to filter.
```
if (snapshottedBuckets.contains(bucketPath)) {
result.add(omBucketInfo); currentCount++;
}
```
The problem with this approach is more GCs.
I prefer first approach because it is simple and should not be a big issue
if there is some inconsistency.
@smengcl please add more if I missed or any other suggestion.
--
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]
