This is an automated email from the ASF dual-hosted git repository.
sumitagrawal 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 693d0f483e HDDS-12589. Fix Incorrect FSO Key Listing for
Container-to-Key Mapping. (#8109)
693d0f483e is described below
commit 693d0f483e07a06c75115d0155a71955bf977b88
Author: Arafat2198 <[email protected]>
AuthorDate: Thu Mar 20 10:14:08 2025 +0530
HDDS-12589. Fix Incorrect FSO Key Listing for Container-to-Key Mapping.
(#8109)
---
.../hadoop/ozone/recon/api/ContainerEndpoint.java | 23 ++--
.../recon/spi/ReconContainerMetadataManager.java | 2 +-
.../impl/ReconContainerMetadataManagerImpl.java | 8 +-
.../ozone/recon/api/TestContainerEndpoint.java | 118 +++++++++++++++++++++
.../TestReconContainerMetadataManagerImpl.java | 10 +-
.../recon/tasks/TestContainerKeyMapperTask.java | 116 ++++++++++++++++++++
6 files changed, 254 insertions(+), 23 deletions(-)
diff --git
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java
index af69fb88f9..17382f0634 100644
---
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java
+++
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java
@@ -235,10 +235,14 @@ public Response getKeysForContainer(
// Last key prefix to be used for pagination. It will be exposed in the
response.
String lastKey = "";
+ // If -1 is passed, set limit to the maximum integer value to retrieve all
records
+ if (limit == -1) {
+ limit = Integer.MAX_VALUE;
+ }
+
try {
Map<ContainerKeyPrefix, Integer> containerKeyPrefixMap =
- reconContainerMetadataManager.getKeyPrefixesForContainer(containerID,
- prevKeyPrefix);
+
reconContainerMetadataManager.getKeyPrefixesForContainer(containerID,
prevKeyPrefix, limit);
// Get set of Container-Key mappings for given containerId.
for (ContainerKeyPrefix containerKeyPrefix : containerKeyPrefixMap
.keySet()) {
@@ -267,10 +271,7 @@ public Response getKeysForContainer(
List<ContainerBlockMetadata> blockIds =
getBlocks(matchedKeys, containerID);
- String ozoneKey = omMetadataManager.getOzoneKey(
- omKeyInfo.getVolumeName(),
- omKeyInfo.getBucketName(),
- omKeyInfo.getKeyName());
+ String ozoneKey = containerKeyPrefix.getKeyPrefix();
lastKey = ozoneKey;
if (keyMetadataMap.containsKey(ozoneKey)) {
keyMetadataMap.get(ozoneKey).getVersions()
@@ -279,10 +280,6 @@ public Response getKeysForContainer(
keyMetadataMap.get(ozoneKey).getBlockIds()
.put(containerKeyPrefix.getKeyVersion(), blockIds);
} else {
- // break the for loop if limit has been reached
- if (keyMetadataMap.size() == limit) {
- break;
- }
KeyMetadata keyMetadata = new KeyMetadata();
keyMetadata.setBucket(omKeyInfo.getBucketName());
keyMetadata.setVolume(omKeyInfo.getVolumeName());
@@ -305,11 +302,9 @@ public Response getKeysForContainer(
totalCount =
reconContainerMetadataManager.getKeyCountForContainer(containerID);
} catch (IOException ioEx) {
- throw new WebApplicationException(ioEx,
- Response.Status.INTERNAL_SERVER_ERROR);
+ throw new WebApplicationException(ioEx,
Response.Status.INTERNAL_SERVER_ERROR);
}
- KeysResponse keysResponse =
- new KeysResponse(totalCount, keyMetadataMap.values(), lastKey);
+ KeysResponse keysResponse = new KeysResponse(totalCount,
keyMetadataMap.values(), lastKey);
return Response.ok(keysResponse).build();
}
diff --git
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ReconContainerMetadataManager.java
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ReconContainerMetadataManager.java
index 1400279d12..4b0b7df42f 100644
---
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ReconContainerMetadataManager.java
+++
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/ReconContainerMetadataManager.java
@@ -172,7 +172,7 @@ Map<ContainerKeyPrefix, Integer> getKeyPrefixesForContainer(
* @return Map of Key prefix -> count.
*/
Map<ContainerKeyPrefix, Integer> getKeyPrefixesForContainer(
- long containerId, String prevKeyPrefix) throws IOException;
+ long containerId, String prevKeyPrefix, int limit) throws IOException;
/**
* Get a Map of containerID, containerMetadata of Containers only for the
diff --git
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
index 17cb793985..771e4372ad 100644
---
a/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
+++
b/hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/ReconContainerMetadataManagerImpl.java
@@ -48,6 +48,7 @@
import org.apache.hadoop.hdds.utils.db.TableIterator;
import org.apache.hadoop.ozone.om.helpers.BucketLayout;
import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
+import org.apache.hadoop.ozone.recon.ReconConstants;
import org.apache.hadoop.ozone.recon.ReconUtils;
import org.apache.hadoop.ozone.recon.api.types.ContainerKeyPrefix;
import org.apache.hadoop.ozone.recon.api.types.ContainerMetadata;
@@ -346,7 +347,8 @@ public Integer getCountForContainerKeyPrefix(
public Map<ContainerKeyPrefix, Integer> getKeyPrefixesForContainer(
long containerId) throws IOException {
// set the default startKeyPrefix to empty string
- return getKeyPrefixesForContainer(containerId, StringUtils.EMPTY);
+ return getKeyPrefixesForContainer(containerId, StringUtils.EMPTY,
+ Integer.parseInt(ReconConstants.DEFAULT_FETCH_COUNT));
}
/**
@@ -360,7 +362,7 @@ public Map<ContainerKeyPrefix, Integer>
getKeyPrefixesForContainer(
*/
@Override
public Map<ContainerKeyPrefix, Integer> getKeyPrefixesForContainer(
- long containerId, String prevKeyPrefix) throws IOException {
+ long containerId, String prevKeyPrefix, int limit) throws IOException {
Map<ContainerKeyPrefix, Integer> prefixes = new LinkedHashMap<>();
try (TableIterator<ContainerKeyPrefix,
@@ -387,7 +389,7 @@ public Map<ContainerKeyPrefix, Integer>
getKeyPrefixesForContainer(
return prefixes;
}
- while (containerIterator.hasNext()) {
+ while (containerIterator.hasNext() && prefixes.size() < limit) {
KeyValue<ContainerKeyPrefix, Integer> keyValue =
containerIterator.next();
ContainerKeyPrefix containerKeyPrefix = keyValue.getKey();
diff --git
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerEndpoint.java
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerEndpoint.java
index dd85f6c753..4db2a276c8 100644
---
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerEndpoint.java
+++
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerEndpoint.java
@@ -1701,4 +1701,122 @@ public void
testGetOmContainersDeletedInSCMPrevContainerParam()
assertEquals(1, containerDiscrepancyInfoList.size());
assertEquals(2, containerDiscrepancyInfoList.get(0).getContainerID());
}
+
+ /**
+ * Helper method that creates duplicate FSO file keys – two keys having the
same file
+ * name but under different directories. It creates the necessary volume,
bucket, and
+ * directory entries, and then writes two keys using writeKeyToOm.
+ */
+ private void setUpDuplicateFSOFileKeys() throws IOException {
+ // Ensure the volume exists.
+ String volumeKey = reconOMMetadataManager.getVolumeKey(VOLUME_NAME);
+ OmVolumeArgs volArgs = OmVolumeArgs.newBuilder()
+ .setVolume(VOLUME_NAME)
+ .setAdminName("TestUser")
+ .setOwnerName("TestUser")
+ .setObjectID(VOL_OBJECT_ID)
+ .build();
+ reconOMMetadataManager.getVolumeTable().put(volumeKey, volArgs);
+
+ // Ensure the bucket exists.
+ OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
+ .setVolumeName(VOLUME_NAME)
+ .setBucketName(BUCKET_NAME)
+ .setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED)
+ .setObjectID(BUCKET_OBJECT_ID)
+ .build();
+ String bucketKey = reconOMMetadataManager.getBucketKey(VOLUME_NAME,
BUCKET_NAME);
+ reconOMMetadataManager.getBucketTable().put(bucketKey, bucketInfo);
+
+ // Create two directories: "dirA" and "dirB" with unique object IDs.
+ // For a top-level directory in a bucket, the parent's object id is the
bucket's id.
+ OmDirectoryInfo dirA = OmDirectoryInfo.newBuilder()
+ .setName("dirA")
+ .setParentObjectID(BUCKET_OBJECT_ID)
+ .setUpdateID(1L)
+ .setObjectID(5L) // Unique object id for dirA.
+ .build();
+ OmDirectoryInfo dirB = OmDirectoryInfo.newBuilder()
+ .setName("dirB")
+ .setParentObjectID(BUCKET_OBJECT_ID)
+ .setUpdateID(1L)
+ .setObjectID(6L) // Unique object id for dirB.
+ .build();
+ // Build DB directory keys. (The third parameter is used to form a unique
key.)
+ String dirKeyA = reconOMMetadataManager.getOzonePathKey(VOL_OBJECT_ID,
BUCKET_OBJECT_ID, 5L, "dirA");
+ String dirKeyB = reconOMMetadataManager.getOzonePathKey(VOL_OBJECT_ID,
BUCKET_OBJECT_ID, 6L, "dirB");
+ reconOMMetadataManager.getDirectoryTable().put(dirKeyA, dirA);
+ reconOMMetadataManager.getDirectoryTable().put(dirKeyB, dirB);
+
+ // Use a common OmKeyLocationInfoGroup.
+ OmKeyLocationInfoGroup locationInfoGroup = getLocationInfoGroup1();
+
+ // Write two FSO keys with the same file name ("dupFile") but in different
directories.
+ // The file name stored in OM is the full path (e.g., "dirA/dupFile" vs.
"dirB/dupFile").
+ writeKeyToOm(reconOMMetadataManager,
+ "dupFileKey1", // internal key name for the first key
+ BUCKET_NAME,
+ VOLUME_NAME,
+ "dupFileKey1", // full file path for the first key
+ 100L, // object id (example)
+ 5L, // parent's object id for dirA (same as
dirA's object id)
+ BUCKET_OBJECT_ID,
+ VOL_OBJECT_ID,
+ Collections.singletonList(locationInfoGroup),
+ BucketLayout.FILE_SYSTEM_OPTIMIZED,
+ KEY_ONE_SIZE);
+
+ writeKeyToOm(reconOMMetadataManager,
+ "dupFileKey1", // internal key name for the second key
+ BUCKET_NAME,
+ VOLUME_NAME,
+ "dupFileKey1", // full file path for the second key
+ 100L,
+ 6L, // parent's object id for dirB
+ BUCKET_OBJECT_ID,
+ VOL_OBJECT_ID,
+ Collections.singletonList(locationInfoGroup),
+ BucketLayout.FILE_SYSTEM_OPTIMIZED,
+ KEY_ONE_SIZE);
+ }
+
+ /**
+ * Test method that sets up two duplicate FSO file keys (same file name but
in different directories)
+ * and then verifies that the ContainerEndpoint returns two distinct key
records.
+ */
+ @Test
+ public void testDuplicateFSOKeysForContainerEndpoint() throws IOException {
+ // Set up duplicate FSO file keys.
+ setUpDuplicateFSOFileKeys();
+ NSSummaryTaskWithFSO nSSummaryTaskWithFso =
+ new NSSummaryTaskWithFSO(reconNamespaceSummaryManager,
+ reconOMMetadataManager, 10);
+ nSSummaryTaskWithFso.reprocessWithFSO(reconOMMetadataManager);
+ // Reprocess the container key mappings so that the new keys are loaded.
+ reprocessContainerKeyMapper();
+
+ // Assume that FSO keys are mapped to container ID 20L (as in previous
tests for FSO).
+ Response response = containerEndpoint.getKeysForContainer(20L, -1, "");
+ KeysResponse keysResponse = (KeysResponse) response.getEntity();
+ Collection<KeyMetadata> keyMetadataList = keysResponse.getKeys();
+
+ // We expect two distinct keys.
+ assertEquals(2, keysResponse.getTotalCount());
+ assertEquals(2, keyMetadataList.size());
+
+ for (KeyMetadata km : keyMetadataList) {
+ String completePath = km.getCompletePath();
+ assertNotNull(completePath);
+ // Verify that the complete path reflects either directory "dirA" or
"dirB"
+ if (completePath.contains("dirA")) {
+ assertEquals(VOLUME_NAME + "/" + BUCKET_NAME + "/dirA/dupFileKey1",
completePath);
+ } else if (completePath.contains("dirB")) {
+ assertEquals(VOLUME_NAME + "/" + BUCKET_NAME + "/dirB/dupFileKey1",
completePath);
+ } else {
+ throw new AssertionError("Unexpected complete path: " + completePath);
+ }
+ }
+ }
+
+
}
diff --git
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestReconContainerMetadataManagerImpl.java
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestReconContainerMetadataManagerImpl.java
index a3b5cef123..693bb50049 100644
---
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestReconContainerMetadataManagerImpl.java
+++
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestReconContainerMetadataManagerImpl.java
@@ -309,25 +309,25 @@ public void testGetKeyPrefixesForContainerWithKeyPrefix()
throws Exception {
Map<ContainerKeyPrefix, Integer> keyPrefixMap =
reconContainerMetadataManager.getKeyPrefixesForContainer(containerId,
- keyPrefix1);
+ keyPrefix1, 1000);
assertEquals(1, keyPrefixMap.size());
assertEquals(2, keyPrefixMap.get(containerKeyPrefix2).longValue());
keyPrefixMap = reconContainerMetadataManager.getKeyPrefixesForContainer(
- nextContainerId, keyPrefix3);
+ nextContainerId, keyPrefix3, 1000);
assertEquals(0, keyPrefixMap.size());
// test for negative cases
keyPrefixMap = reconContainerMetadataManager.getKeyPrefixesForContainer(
- containerId, "V3/B1/invalid");
+ containerId, "V3/B1/invalid", 1000);
assertEquals(0, keyPrefixMap.size());
keyPrefixMap = reconContainerMetadataManager.getKeyPrefixesForContainer(
- containerId, keyPrefix3);
+ containerId, keyPrefix3, 1000);
assertEquals(0, keyPrefixMap.size());
keyPrefixMap = reconContainerMetadataManager.getKeyPrefixesForContainer(
- 10L, "");
+ 10L, "", 1000);
assertEquals(0, keyPrefixMap.size());
}
diff --git
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestContainerKeyMapperTask.java
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestContainerKeyMapperTask.java
index fb31537ec7..c30fc451b0 100644
---
a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestContainerKeyMapperTask.java
+++
b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestContainerKeyMapperTask.java
@@ -472,6 +472,122 @@ public void testFileTableProcess() throws Exception {
firstKeyPrefix.getKeyPrefix());
}
+ @Test
+ public void testDuplicateFSOKeysInDifferentDirectories() throws Exception {
+ // Ensure container 1 is initially empty.
+ Map<ContainerKeyPrefix, Integer> keyPrefixesForContainer =
+ reconContainerMetadataManager.getKeyPrefixesForContainer(1L);
+ assertThat(keyPrefixesForContainer).isEmpty();
+
+ Pipeline pipeline = getRandomPipeline();
+ // Create a common OmKeyLocationInfoGroup for all keys.
+ List<OmKeyLocationInfo> omKeyLocationInfoList = new ArrayList<>();
+ BlockID blockID = new BlockID(1L, 1L);
+ OmKeyLocationInfo omKeyLocationInfo = getOmKeyLocationInfo(blockID,
pipeline);
+ omKeyLocationInfoList.add(omKeyLocationInfo);
+ OmKeyLocationInfoGroup omKeyLocationInfoGroup =
+ new OmKeyLocationInfoGroup(0L, omKeyLocationInfoList);
+
+ // Define file names.
+ String file1Key = "file1";
+ String file2Key = "file2";
+
+ // Define directory (parent) object IDs with shorter values.
+ long dir1Id = -101L;
+ long dir2Id = -102L;
+ long dir3Id = -103L;
+
+ // Write three FSO keys for "file1" with different parent object IDs.
+ writeKeyToOm(reconOMMetadataManager,
+ file1Key, // keyName
+ BUCKET_NAME, // bucketName
+ VOLUME_NAME, // volName
+ file1Key, // fileName
+ KEY_ONE_OBJECT_ID, // objectId
+ dir1Id, // ObjectId for first directory
+ BUCKET_ONE_OBJECT_ID, // bucketObjectId
+ VOL_OBJECT_ID, // volumeObjectId
+ Collections.singletonList(omKeyLocationInfoGroup),
+ BucketLayout.FILE_SYSTEM_OPTIMIZED,
+ KEY_ONE_SIZE);
+
+ writeKeyToOm(reconOMMetadataManager,
+ file1Key,
+ BUCKET_NAME,
+ VOLUME_NAME,
+ file1Key,
+ KEY_ONE_OBJECT_ID,
+ dir2Id, // ObjectId for second directory
+ BUCKET_ONE_OBJECT_ID,
+ VOL_OBJECT_ID,
+ Collections.singletonList(omKeyLocationInfoGroup),
+ BucketLayout.FILE_SYSTEM_OPTIMIZED,
+ KEY_ONE_SIZE);
+
+ writeKeyToOm(reconOMMetadataManager,
+ file1Key,
+ BUCKET_NAME,
+ VOLUME_NAME,
+ file1Key,
+ KEY_ONE_OBJECT_ID,
+ dir3Id, // ObjectId for third directory
+ BUCKET_ONE_OBJECT_ID,
+ VOL_OBJECT_ID,
+ Collections.singletonList(omKeyLocationInfoGroup),
+ BucketLayout.FILE_SYSTEM_OPTIMIZED,
+ KEY_ONE_SIZE);
+
+ // Write three FSO keys for "file2" with different parent object IDs.
+ writeKeyToOm(reconOMMetadataManager,
+ "fso-file2",
+ BUCKET_NAME,
+ VOLUME_NAME,
+ file2Key,
+ KEY_ONE_OBJECT_ID,
+ dir1Id,
+ BUCKET_ONE_OBJECT_ID,
+ VOL_OBJECT_ID,
+ Collections.singletonList(omKeyLocationInfoGroup),
+ BucketLayout.FILE_SYSTEM_OPTIMIZED,
+ KEY_ONE_SIZE);
+
+ writeKeyToOm(reconOMMetadataManager,
+ "fso-file2",
+ BUCKET_NAME,
+ VOLUME_NAME,
+ file2Key,
+ KEY_ONE_OBJECT_ID,
+ dir2Id,
+ BUCKET_ONE_OBJECT_ID,
+ VOL_OBJECT_ID,
+ Collections.singletonList(omKeyLocationInfoGroup),
+ BucketLayout.FILE_SYSTEM_OPTIMIZED,
+ KEY_ONE_SIZE);
+
+ writeKeyToOm(reconOMMetadataManager,
+ "fso-file2",
+ BUCKET_NAME,
+ VOLUME_NAME,
+ file2Key,
+ KEY_ONE_OBJECT_ID,
+ dir3Id,
+ BUCKET_ONE_OBJECT_ID,
+ VOL_OBJECT_ID,
+ Collections.singletonList(omKeyLocationInfoGroup),
+ BucketLayout.FILE_SYSTEM_OPTIMIZED,
+ KEY_ONE_SIZE);
+
+ // Reprocess container key mappings.
+ ContainerKeyMapperTaskFSO containerKeyMapperTask =
+ new ContainerKeyMapperTaskFSO(reconContainerMetadataManager,
omConfiguration);
+ containerKeyMapperTask.reprocess(reconOMMetadataManager);
+
+ // With our changes using the raw key prefix as the unique identifier,
+ // we expect six distinct entries in container 1.
+ keyPrefixesForContainer =
reconContainerMetadataManager.getKeyPrefixesForContainer(1L);
+ assertEquals(6, keyPrefixesForContainer.size());
+ }
+
private OmKeyInfo buildOmKeyInfo(String volume,
String bucket,
String key,
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
