This is an automated email from the ASF dual-hosted git repository.
adoroszlai pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new f1552bc05d HDDS-9233. listStatus() API is not correctly iterating
cache. (#5244)
f1552bc05d is described below
commit f1552bc05d656537c76e8363417d306a00991d68
Author: Devesh Kumar Singh <[email protected]>
AuthorDate: Sat Sep 9 22:03:19 2023 +0530
HDDS-9233. listStatus() API is not correctly iterating cache. (#5244)
---
.../org/apache/hadoop/ozone/om/KeyManagerImpl.java | 27 +++++++++++++++-------
.../hadoop/ozone/om/OzoneListStatusHelper.java | 6 ++++-
2 files changed, 24 insertions(+), 9 deletions(-)
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
index 8bce4eb70f..10d60033fa 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
@@ -1426,7 +1426,13 @@ public class KeyManagerImpl implements KeyManager {
}
OzoneFileStatus fileStatus = new OzoneFileStatus(
cacheOmKeyInfo, scmBlockSize, !OzoneFSUtils.isFile(cacheKey));
- cacheKeyMap.put(cacheKey, fileStatus);
+ cacheKeyMap.putIfAbsent(cacheKey, fileStatus);
+ // This else block has been added to capture deleted entries in cache.
+ // Adding deleted entries in cacheKeyMap as there is a possible race
+ // condition where table cache iterator is flushed already when
+ // using in the caller of this method.
+ } else if (cacheOmKeyInfo == null && !cacheKeyMap.containsKey(cacheKey))
{
+ cacheKeyMap.put(cacheKey, null);
}
}
}
@@ -1540,8 +1546,14 @@ public class KeyManagerImpl implements KeyManager {
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
+ // before putting entries in cacheKeyMap in findKeyInDbWithIterator call.
+ if (fileStatus == null) {
+ continue;
+ }
fileStatusList.add(fileStatus);
countEntries++;
if (countEntries >= numEntries) {
@@ -1611,9 +1623,8 @@ public class KeyManagerImpl implements KeyManager {
String entryKeyName = omKeyInfo.getKeyName();
if (recursive) {
// for recursive list all the entries
-
- if (!isKeyDeleted(entryInDb, keyTable)) {
- cacheKeyMap.putIfAbsent(entryInDb, new OzoneFileStatus(omKeyInfo,
+ if (!cacheKeyMap.containsKey(entryInDb)) {
+ cacheKeyMap.put(entryInDb, new OzoneFileStatus(omKeyInfo,
scmBlockSize, !OzoneFSUtils.isFile(entryKeyName)));
countEntries++;
}
@@ -1626,14 +1637,14 @@ public class KeyManagerImpl implements KeyManager {
.getImmediateChild(entryKeyName, keyName);
boolean isFile = OzoneFSUtils.isFile(immediateChild);
if (isFile) {
- if (!isKeyDeleted(entryInDb, keyTable)) {
+ if (!cacheKeyMap.containsKey(entryInDb)) {
cacheKeyMap.put(entryInDb,
new OzoneFileStatus(omKeyInfo, scmBlockSize, !isFile));
countEntries++;
}
} else {
// if entry is a directory
- if (!isKeyDeleted(entryInDb, keyTable)) {
+ if (!cacheKeyMap.containsKey(entryInDb)) {
if (!entryKeyName.equals(immediateChild)) {
OmKeyInfo fakeDirEntry = createDirectoryKey(
omKeyInfo, immediateChild);
diff --git
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java
index 758206f200..4bfe888d43 100644
---
a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java
+++
b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneListStatusHelper.java
@@ -215,7 +215,11 @@ public class OzoneListStatusHelper {
HeapEntry entry = heapIterator.next();
OzoneFileStatus status = entry.getStatus(prefixKey,
scmBlockSize, volumeName, bucketName, replication);
- map.putIfAbsent(entry.key, status);
+ // Caution: DO NOT use putIfAbsent. putIfAbsent undesirably overwrites
+ // the value with `status` when the existing value in the map is null.
+ if (!map.containsKey(entry.key)) {
+ map.put(entry.key, status);
+ }
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
