sadanand48 commented on code in PR #5244:
URL: https://github.com/apache/ozone/pull/5244#discussion_r1317101959
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -1540,8 +1542,14 @@ public List<OzoneFileStatus> listStatus(OmKeyArgs args,
boolean recursive,
countEntries = 0;
// Convert results in cacheKeyMap to List
for (OzoneFileStatus fileStatus : cacheKeyMap.values()) {
- // No need to check if a key is deleted or not here, this is handled
- // when adding entries to cacheKeyMap from DB.
+ // Here need to check if a key is deleted as cacheKeyMap will contain
+ // deleted entries as well. Adding deleted entries in cacheKeyMap is done
+ // as there is a possible race condition where table cache iterator is
+ // flushed already and isKeyDeleted check may not work as expected
Review Comment:
Thanks @devmadhuu for explaining, missed the part where you added the null
value. makes sense to avoid the isKeyDeleted check. We should check other
usages of `isKeyDeleted` if it causes similar issues elsewhere.
Maybe the correct way to check isKeyDeleted would be to check the existence
of the key in table if no value is present in cache.
--
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]
