This is an automated email from the ASF dual-hosted git repository.
agupta 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 0fac57ab89 HDDS-9065. [FSO]ListKeys: Incorrect result when keyPrefix
matching multiple exist keys. (#6072)
0fac57ab89 is described below
commit 0fac57ab89e2338cb53a6b79b8d67b4e7a3c5af6
Author: Sadanand Shenoy <[email protected]>
AuthorDate: Fri Feb 2 13:59:39 2024 +0530
HDDS-9065. [FSO]ListKeys: Incorrect result when keyPrefix matching multiple
exist keys. (#6072)
---
.../hadoop/ozone/om/TestListKeysWithFSO.java | 6 ++
.../hadoop/ozone/om/OzoneListStatusHelper.java | 113 ++++++++++++++-------
2 files changed, 83 insertions(+), 36 deletions(-)
diff --git
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java
index f5d6ed7529..99fb69048c 100644
---
a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java
+++
b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestListKeysWithFSO.java
@@ -417,6 +417,11 @@ public class TestListKeysWithFSO {
expectedKeys = getExpectedKeyList("a", "a1", legacyOzoneBucket2);
checkKeyList("a", "a1", expectedKeys, fsoOzoneBucket2);
+
+ // test when the keyPrefix = existing key
+ expectedKeys =
+ getExpectedKeyList("x/y/z/z1.tx", "", legacyOzoneBucket2);
+ checkKeyList("x/y/z/z1.tx", "", expectedKeys, fsoOzoneBucket2);
}
@Test
@@ -549,6 +554,7 @@ public class TestListKeysWithFSO {
keys.add("/a3/b1/c1/c1.tx");
keys.add("/x/y/z/z1.tx");
+ keys.add("/x/y/z/z1.txdir/z2.tx");
keys.add("/dir1/dir2/dir3/d11.tx");
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 9735ea209d..d93e1db736 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
@@ -85,21 +85,17 @@ public class OzoneListStatusHelper {
String startKey, long numEntries, String clientAddress,
boolean allowPartialPrefixes) throws IOException {
Preconditions.checkNotNull(args, "Key args can not be null");
-
final String volumeName = args.getVolumeName();
final String bucketName = args.getBucketName();
String keyName = args.getKeyName();
String prefixKey = keyName;
-
final String volumeKey = metadataManager.getVolumeKey(volumeName);
final String bucketKey = metadataManager.getBucketKey(volumeName,
bucketName);
-
final OmVolumeArgs volumeInfo = metadataManager.getVolumeTable()
.get(volumeKey);
final OmBucketInfo omBucketInfo = metadataManager.getBucketTable()
.get(bucketKey);
-
if (volumeInfo == null || omBucketInfo == null) {
if (LOG.isDebugEnabled()) {
LOG.debug(String.format("%s does not exist.", (volumeInfo == null) ?
@@ -109,16 +105,9 @@ public class OzoneListStatusHelper {
return new ArrayList<>();
}
- // Determine if the prefixKey is determined from the startKey
- // if the keyName is null
if (StringUtils.isNotBlank(startKey)) {
if (StringUtils.isNotBlank(keyName)) {
- if (!OzoneFSUtils.isSibling(keyName, startKey) &&
- !OzoneFSUtils.isImmediateChild(keyName, startKey)) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("StartKey {} is not an immediate child or not a sibling"
- + " of keyName {}. Returns empty list", startKey, keyName);
- }
+ if (!validateStartKey(startKey, keyName)) {
return new ArrayList<>();
}
} else {
@@ -131,10 +120,8 @@ public class OzoneListStatusHelper {
.build();
}
}
-
OzoneFileStatus fileStatus =
getStatusHelper.apply(args, clientAddress, allowPartialPrefixes);
-
String dbPrefixKey;
if (fileStatus == null) {
// if the file status is null, prefix is a not a valid filesystem path
@@ -155,19 +142,65 @@ public class OzoneListStatusHelper {
throw ome;
}
} else {
- // If the keyname is a file just return one entry
+ // If the keyName is a file just return one entry if partial prefixes are
+ // not allowed.
+ // If partial prefixes are allowed, the found file should also be
+ // considered as a prefix.
if (fileStatus.isFile()) {
- return Collections.singletonList(fileStatus);
+ if (!allowPartialPrefixes) {
+ return Collections.singletonList(fileStatus);
+ } else {
+ try {
+ dbPrefixKey = getDbKey(keyName, args, volumeInfo, omBucketInfo);
+ prefixKey = OzoneFSUtils.getParentDir(keyName);
+ } catch (OMException ome) {
+ if (ome.getResult() == FILE_NOT_FOUND) {
+ // the parent dir cannot be found return null list
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Parent directory of keyName:{} does not exist." +
+ "Returns empty list", keyName);
+ }
+ return new ArrayList<>();
+ }
+ throw ome;
+ }
+ }
+ } else {
+ // fetch the db key based on parent prefix id.
+ long id = getId(fileStatus, omBucketInfo);
+ final long volumeId = volumeInfo.getObjectID();
+ final long bucketId = omBucketInfo.getObjectID();
+ dbPrefixKey =
+ metadataManager.getOzonePathKey(volumeId, bucketId, id, "");
}
+ }
+ String startKeyPrefix = getStartKeyPrefixIfPresent(args, startKey,
volumeInfo, omBucketInfo);
+ TreeMap<String, OzoneFileStatus> map =
+ getSortedEntries(numEntries, prefixKey, dbPrefixKey, startKeyPrefix,
omBucketInfo);
+
+ return map.values().stream().filter(e -> e != null).collect(
+ Collectors.toList());
+ }
- // fetch the db key based on parent prefix id.
- long id = getId(fileStatus, omBucketInfo);
- final long volumeId = volumeInfo.getObjectID();
- final long bucketId = omBucketInfo.getObjectID();
- dbPrefixKey = metadataManager.getOzonePathKey(volumeId, bucketId,
- id, "");
+ /**
+ * Determine if the prefixKey is determined from the startKey
+ * if the keyName is null.
+ */
+ private static boolean validateStartKey(
+ String startKey, String keyName) {
+ if (!OzoneFSUtils.isSibling(keyName, startKey) &&
+ !OzoneFSUtils.isImmediateChild(keyName, startKey)) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("StartKey {} is not an immediate child or not a sibling"
+ + " of keyName {}. Returns empty list", startKey, keyName);
+ }
+ return false;
}
+ return true;
+ }
+ private String getStartKeyPrefixIfPresent(OmKeyArgs args, String startKey,
+ OmVolumeArgs volumeInfo, OmBucketInfo omBucketInfo) throws IOException {
// Determine startKeyPrefix for DB iteration
String startKeyPrefix = "";
try {
@@ -179,41 +212,49 @@ public class OzoneListStatusHelper {
throw ome;
}
}
+ return startKeyPrefix;
+ }
- TreeMap<String, OzoneFileStatus> map = new TreeMap<>();
-
- BucketLayout bucketLayout = omBucketInfo.getBucketLayout();
+ /**
+ * fetch the sorted output using a min heap iterator where
+ * every remove from the heap will give the smallest entry and return
+ * a treemap.
+ */
+ private TreeMap<String, OzoneFileStatus> getSortedEntries(long numEntries,
+ String prefixKey, String dbPrefixKey, String startKeyPrefix,
+ OmBucketInfo bucketInfo) throws IOException {
+ String volumeName = bucketInfo.getVolumeName();
+ String bucketName = bucketInfo.getBucketName();
+ BucketLayout bucketLayout = bucketInfo.getBucketLayout();
ReplicationConfig replication =
- Optional.ofNullable(omBucketInfo.getDefaultReplicationConfig())
+ Optional.ofNullable(bucketInfo.getDefaultReplicationConfig())
.map(DefaultReplicationConfig::getReplicationConfig)
.orElse(omDefaultReplication);
- // fetch the sorted output using a min heap iterator where
- // every remove from the heap will give the smallest entry.
- try (ListIterator.MinHeapIterator heapIterator =
- new ListIterator.MinHeapIterator(metadataManager, dbPrefixKey,
- bucketLayout, startKeyPrefix, volumeName, bucketName)) {
+ TreeMap<String, OzoneFileStatus> map = new TreeMap<>();
+ try (
+ ListIterator.MinHeapIterator heapIterator = new
ListIterator.MinHeapIterator(
+ metadataManager, dbPrefixKey, bucketLayout, startKeyPrefix,
+ volumeName, bucketName)) {
try {
while (map.size() < numEntries && heapIterator.hasNext()) {
ListIterator.HeapEntry entry = heapIterator.next();
- OzoneFileStatus status = getStatus(prefixKey,
- scmBlockSize, volumeName, bucketName, replication, entry);
+ OzoneFileStatus status = getStatus(prefixKey, scmBlockSize,
volumeName, bucketName,
+ replication, entry);
// 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.getKey())) {
map.put(entry.getKey(), status);
}
}
+ return map;
} catch (NoSuchElementException e) {
throw new IOException(e);
} catch (UncheckedIOException e) {
throw e.getCause();
}
}
-
- return map.values().stream().filter(e -> e != null).collect(
- Collectors.toList());
}
private OzoneFileStatus getStatus(String prefixPath, long scmBlockSz,
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
