JyotinderSingh commented on a change in pull request #3227:
URL: https://github.com/apache/ozone/pull/3227#discussion_r834369769
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmMetadataManagerImpl.java
##########
@@ -707,46 +707,104 @@ public boolean isVolumeEmpty(String volume) throws
IOException {
@Override
public boolean isBucketEmpty(String volume, String bucket)
throws IOException {
- String keyPrefix = getBucketKey(volume, bucket);
+ String bucketKey = getBucketKey(volume, bucket);
+ OmBucketInfo omBucketInfo = getBucketTable().get(bucketKey);
+ String bucketId = String.valueOf(omBucketInfo.getObjectID());
+ BucketLayout bucketLayout = omBucketInfo.getBucketLayout();
+
+ // keyPrefix is different in case of fileTable and keyTable.
+ // 1. For OBS and LEGACY buckets:
+ // the entries are present in the keyTable and are of the
+ // format <bucketKey>/<key-name>
+ // 2. For FSO buckets:
+ // - TOP-LEVEL DIRECTORY would be of the format <bucket ID>/dirName
+ // inside the dirTable.
+ // - TOP-LEVEL FILE (a file directly placed under the bucket without
+ // any sub paths) would be of the format <bucket ID>/fileName inside
+ // the fileTable.
+ String keyPrefix =
+ bucketLayout.isFileSystemOptimized() ? bucketId : bucketKey;
+
+ // Check key/file Table
+ Table<String, OmKeyInfo> table = getKeyTable(bucketLayout);
+
+ // First check in table cache.
+ if (isKeyPresentInTableCache(keyPrefix, table)) {
+ return false;
+ }
+ // check in table
+ if (isKeyPresentInTable(keyPrefix, table)) {
+ return false; // we found at least one key with this vol/bucket
+ }
- // First check in key table cache.
+ // Check dirTable as well in case of FSO bucket.
+ if (bucketLayout.isFileSystemOptimized()) {
+ // First check in dirTable cache
+ if (isKeyPresentInTableCache(keyPrefix, dirTable)) {
+ return false;
+ }
+ // Check in the table
+ return !isKeyPresentInTable(keyPrefix, dirTable);
+ }
+ return true;
+ }
+
+
+ /**
+ * Checks if a key starting with a given keyPrefix exists in the table cache.
+ *
+ * @param keyPrefix - key prefix to be searched.
+ * @param table - table to be searched.
+ * @return true if the key is present in the cache.
+ */
+ private boolean isKeyPresentInTableCache(String keyPrefix,
+ Table table) {
Iterator<Map.Entry<CacheKey<String>, CacheValue<OmKeyInfo>>> iterator =
- ((TypedTable< String, OmKeyInfo>) keyTable).cacheIterator();
+ table.cacheIterator();
while (iterator.hasNext()) {
- Map.Entry< CacheKey<String>, CacheValue<OmKeyInfo>> entry =
+ Map.Entry<CacheKey<String>, CacheValue<OmKeyInfo>> entry =
iterator.next();
String key = entry.getKey().getCacheKey();
OmKeyInfo omKeyInfo = entry.getValue().getCacheValue();
// Making sure that entry is not for delete key request.
if (key.startsWith(keyPrefix) && omKeyInfo != null) {
- return false;
+ return true;
}
}
- try (TableIterator<String, ? extends KeyValue<String, OmKeyInfo>> keyIter =
- keyTable.iterator()) {
+ return false;
+ }
+
+ /**
+ * Checks if a key starts with the given prefix is present in the table.
+ *
+ * @param keyPrefix - Prefix to check for
+ * @param table - Table to check in
+ * @return true if the key is present in the table
+ * @throws IOException
+ */
+ private boolean isKeyPresentInTable(String keyPrefix,
+ Table<String, OmKeyInfo> table)
+ throws IOException {
+ try (TableIterator<String, ? extends KeyValue<String, OmKeyInfo>>
+ keyIter = table.iterator()) {
KeyValue<String, OmKeyInfo> kv = keyIter.seek(keyPrefix);
if (kv != null) {
// Check the entry in db is not marked for delete. This can happen
// while entry is marked for delete, but it is not flushed to DB.
CacheValue<OmKeyInfo> cacheValue =
- keyTable.getCacheValue(new CacheKey(kv.getKey()));
+ table.getCacheValue(new CacheKey(kv.getKey()));
if (cacheValue != null) {
- if (kv.getKey().startsWith(keyPrefix)
- && cacheValue.getCacheValue() != null) {
- return false; // we found at least one key with this vol/bucket
- // prefix.
- }
+ // we found at least one key with this prefix.
+ return kv.getKey().startsWith(keyPrefix)
+ && cacheValue.getCacheValue() != null;
Review comment:
That's a good point, this case skipped my mind. I have updated the code
to loop over the said prefix range to make sure we look at all such keys.
Please let me know if it looks good to you.
--
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]
