Skip to content

Commit 68e3842

Browse files
ArafatKhan2198ArafatKhan2198
authored
HDDS-12589. Fix Incorrect FSO Key Listing for Container-to-Key Mapping. (apache#8078)
1 parent c8e77f8 commit 68e3842

File tree

3 files changed

+240
-8
lines changed

3 files changed

+240
-8
lines changed

‎hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/api/ContainerEndpoint.java

+6-8
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,11 @@ public Response getKeysForContainer(
241241
for (ContainerKeyPrefix containerKeyPrefix : containerKeyPrefixMap
242242
.keySet()) {
243243

244+
// break the for loop if limit has been reached
245+
if (keyMetadataMap.size() == limit) {
246+
break;
247+
}
248+
244249
// Directly calling getSkipCache() on the Key/FileTable table
245250
// instead of iterating since only full keys are supported now. We will
246251
// try to get the OmKeyInfo object by searching the KEY_TABLE table with
@@ -265,10 +270,7 @@ public Response getKeysForContainer(
265270
List<ContainerBlockMetadata> blockIds =
266271
getBlocks(matchedKeys, containerID);
267272

268-
String ozoneKey = omMetadataManager.getOzoneKey(
269-
omKeyInfo.getVolumeName(),
270-
omKeyInfo.getBucketName(),
271-
omKeyInfo.getKeyName());
273+
String ozoneKey = containerKeyPrefix.getKeyPrefix();
272274
lastKey = ozoneKey;
273275
if (keyMetadataMap.containsKey(ozoneKey)) {
274276
keyMetadataMap.get(ozoneKey).getVersions()
@@ -277,10 +279,6 @@ public Response getKeysForContainer(
277279
keyMetadataMap.get(ozoneKey).getBlockIds()
278280
.put(containerKeyPrefix.getKeyVersion(), blockIds);
279281
} else {
280-
// break the for loop if limit has been reached
281-
if (keyMetadataMap.size() == limit) {
282-
break;
283-
}
284282
KeyMetadata keyMetadata = new KeyMetadata();
285283
keyMetadata.setBucket(omKeyInfo.getBucketName());
286284
keyMetadata.setVolume(omKeyInfo.getVolumeName());

‎hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/api/TestContainerEndpoint.java

+118
Original file line numberDiff line numberDiff line change
@@ -1665,4 +1665,122 @@ public void testGetOmContainersDeletedInSCMPrevContainerParam()
16651665
assertEquals(1, containerDiscrepancyInfoList.size());
16661666
assertEquals(2, containerDiscrepancyInfoList.get(0).getContainerID());
16671667
}
1668+
1669+
/**
1670+
* Helper method that creates duplicate FSO file keys – two keys having the same file
1671+
* name but under different directories. It creates the necessary volume, bucket, and
1672+
* directory entries, and then writes two keys using writeKeyToOm.
1673+
*/
1674+
private void setUpDuplicateFSOFileKeys() throws IOException {
1675+
// Ensure the volume exists.
1676+
String volumeKey = reconOMMetadataManager.getVolumeKey(VOLUME_NAME);
1677+
OmVolumeArgs volArgs = OmVolumeArgs.newBuilder()
1678+
.setVolume(VOLUME_NAME)
1679+
.setAdminName("TestUser")
1680+
.setOwnerName("TestUser")
1681+
.setObjectID(VOL_OBJECT_ID)
1682+
.build();
1683+
reconOMMetadataManager.getVolumeTable().put(volumeKey, volArgs);
1684+
1685+
// Ensure the bucket exists.
1686+
OmBucketInfo bucketInfo = OmBucketInfo.newBuilder()
1687+
.setVolumeName(VOLUME_NAME)
1688+
.setBucketName(BUCKET_NAME)
1689+
.setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED)
1690+
.setObjectID(BUCKET_OBJECT_ID)
1691+
.build();
1692+
String bucketKey = reconOMMetadataManager.getBucketKey(VOLUME_NAME, BUCKET_NAME);
1693+
reconOMMetadataManager.getBucketTable().put(bucketKey, bucketInfo);
1694+
1695+
// Create two directories: "dirA" and "dirB" with unique object IDs.
1696+
// For a top-level directory in a bucket, the parent's object id is the bucket's id.
1697+
OmDirectoryInfo dirA = OmDirectoryInfo.newBuilder()
1698+
.setName("dirA")
1699+
.setParentObjectID(BUCKET_OBJECT_ID)
1700+
.setUpdateID(1L)
1701+
.setObjectID(5L) // Unique object id for dirA.
1702+
.build();
1703+
OmDirectoryInfo dirB = OmDirectoryInfo.newBuilder()
1704+
.setName("dirB")
1705+
.setParentObjectID(BUCKET_OBJECT_ID)
1706+
.setUpdateID(1L)
1707+
.setObjectID(6L) // Unique object id for dirB.
1708+
.build();
1709+
// Build DB directory keys. (The third parameter is used to form a unique key.)
1710+
String dirKeyA = reconOMMetadataManager.getOzonePathKey(VOL_OBJECT_ID, BUCKET_OBJECT_ID, 5L, "dirA");
1711+
String dirKeyB = reconOMMetadataManager.getOzonePathKey(VOL_OBJECT_ID, BUCKET_OBJECT_ID, 6L, "dirB");
1712+
reconOMMetadataManager.getDirectoryTable().put(dirKeyA, dirA);
1713+
reconOMMetadataManager.getDirectoryTable().put(dirKeyB, dirB);
1714+
1715+
// Use a common OmKeyLocationInfoGroup.
1716+
OmKeyLocationInfoGroup locationInfoGroup = getLocationInfoGroup1();
1717+
1718+
// Write two FSO keys with the same file name ("dupFile") but in different directories.
1719+
// The file name stored in OM is the full path (e.g., "dirA/dupFile" vs. "dirB/dupFile").
1720+
writeKeyToOm(reconOMMetadataManager,
1721+
"dupFileKey1", // internal key name for the first key
1722+
BUCKET_NAME,
1723+
VOLUME_NAME,
1724+
"dupFileKey1", // full file path for the first key
1725+
100L, // object id (example)
1726+
5L, // parent's object id for dirA (same as dirA's object id)
1727+
BUCKET_OBJECT_ID,
1728+
VOL_OBJECT_ID,
1729+
Collections.singletonList(locationInfoGroup),
1730+
BucketLayout.FILE_SYSTEM_OPTIMIZED,
1731+
KEY_ONE_SIZE);
1732+
1733+
writeKeyToOm(reconOMMetadataManager,
1734+
"dupFileKey1", // internal key name for the second key
1735+
BUCKET_NAME,
1736+
VOLUME_NAME,
1737+
"dupFileKey1", // full file path for the second key
1738+
100L,
1739+
6L, // parent's object id for dirB
1740+
BUCKET_OBJECT_ID,
1741+
VOL_OBJECT_ID,
1742+
Collections.singletonList(locationInfoGroup),
1743+
BucketLayout.FILE_SYSTEM_OPTIMIZED,
1744+
KEY_ONE_SIZE);
1745+
}
1746+
1747+
/**
1748+
* Test method that sets up two duplicate FSO file keys (same file name but in different directories)
1749+
* and then verifies that the ContainerEndpoint returns two distinct key records.
1750+
*/
1751+
@Test
1752+
public void testDuplicateFSOKeysForContainerEndpoint() throws IOException {
1753+
// Set up duplicate FSO file keys.
1754+
setUpDuplicateFSOFileKeys();
1755+
NSSummaryTaskWithFSO nSSummaryTaskWithFso =
1756+
new NSSummaryTaskWithFSO(reconNamespaceSummaryManager,
1757+
reconOMMetadataManager, 10);
1758+
nSSummaryTaskWithFso.reprocessWithFSO(reconOMMetadataManager);
1759+
// Reprocess the container key mappings so that the new keys are loaded.
1760+
reprocessContainerKeyMapper();
1761+
1762+
// Assume that FSO keys are mapped to container ID 20L (as in previous tests for FSO).
1763+
Response response = containerEndpoint.getKeysForContainer(20L, -1, "");
1764+
KeysResponse keysResponse = (KeysResponse) response.getEntity();
1765+
Collection<KeyMetadata> keyMetadataList = keysResponse.getKeys();
1766+
1767+
// We expect two distinct keys.
1768+
assertEquals(2, keysResponse.getTotalCount());
1769+
assertEquals(2, keyMetadataList.size());
1770+
1771+
for (KeyMetadata km : keyMetadataList) {
1772+
String completePath = km.getCompletePath();
1773+
assertNotNull(completePath);
1774+
// Verify that the complete path reflects either directory "dirA" or "dirB"
1775+
if (completePath.contains("dirA")) {
1776+
assertEquals(VOLUME_NAME + "/" + BUCKET_NAME + "/dirA/dupFileKey1", completePath);
1777+
} else if (completePath.contains("dirB")) {
1778+
assertEquals(VOLUME_NAME + "/" + BUCKET_NAME + "/dirB/dupFileKey1", completePath);
1779+
} else {
1780+
throw new AssertionError("Unexpected complete path: " + completePath);
1781+
}
1782+
}
1783+
}
1784+
1785+
16681786
}

‎hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/tasks/TestContainerKeyMapperTask.java

+116
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,122 @@ public void testFileTableProcess() throws Exception {
472472
firstKeyPrefix.getKeyPrefix());
473473
}
474474

475+
@Test
476+
public void testDuplicateFSOKeysInDifferentDirectories() throws Exception {
477+
// Ensure container 1 is initially empty.
478+
Map<ContainerKeyPrefix, Integer> keyPrefixesForContainer =
479+
reconContainerMetadataManager.getKeyPrefixesForContainer(1L);
480+
assertThat(keyPrefixesForContainer).isEmpty();
481+
482+
Pipeline pipeline = getRandomPipeline();
483+
// Create a common OmKeyLocationInfoGroup for all keys.
484+
List<OmKeyLocationInfo> omKeyLocationInfoList = new ArrayList<>();
485+
BlockID blockID = new BlockID(1L, 1L);
486+
OmKeyLocationInfo omKeyLocationInfo = getOmKeyLocationInfo(blockID, pipeline);
487+
omKeyLocationInfoList.add(omKeyLocationInfo);
488+
OmKeyLocationInfoGroup omKeyLocationInfoGroup =
489+
new OmKeyLocationInfoGroup(0L, omKeyLocationInfoList);
490+
491+
// Define file names.
492+
String file1Key = "file1";
493+
String file2Key = "file2";
494+
495+
// Define directory (parent) object IDs with shorter values.
496+
long dir1Id = -101L;
497+
long dir2Id = -102L;
498+
long dir3Id = -103L;
499+
500+
// Write three FSO keys for "file1" with different parent object IDs.
501+
writeKeyToOm(reconOMMetadataManager,
502+
file1Key, // keyName
503+
BUCKET_NAME, // bucketName
504+
VOLUME_NAME, // volName
505+
file1Key, // fileName
506+
KEY_ONE_OBJECT_ID, // objectId
507+
dir1Id, // ObjectId for first directory
508+
BUCKET_ONE_OBJECT_ID, // bucketObjectId
509+
VOL_OBJECT_ID, // volumeObjectId
510+
Collections.singletonList(omKeyLocationInfoGroup),
511+
BucketLayout.FILE_SYSTEM_OPTIMIZED,
512+
KEY_ONE_SIZE);
513+
514+
writeKeyToOm(reconOMMetadataManager,
515+
file1Key,
516+
BUCKET_NAME,
517+
VOLUME_NAME,
518+
file1Key,
519+
KEY_ONE_OBJECT_ID,
520+
dir2Id, // ObjectId for second directory
521+
BUCKET_ONE_OBJECT_ID,
522+
VOL_OBJECT_ID,
523+
Collections.singletonList(omKeyLocationInfoGroup),
524+
BucketLayout.FILE_SYSTEM_OPTIMIZED,
525+
KEY_ONE_SIZE);
526+
527+
writeKeyToOm(reconOMMetadataManager,
528+
file1Key,
529+
BUCKET_NAME,
530+
VOLUME_NAME,
531+
file1Key,
532+
KEY_ONE_OBJECT_ID,
533+
dir3Id, // ObjectId for third directory
534+
BUCKET_ONE_OBJECT_ID,
535+
VOL_OBJECT_ID,
536+
Collections.singletonList(omKeyLocationInfoGroup),
537+
BucketLayout.FILE_SYSTEM_OPTIMIZED,
538+
KEY_ONE_SIZE);
539+
540+
// Write three FSO keys for "file2" with different parent object IDs.
541+
writeKeyToOm(reconOMMetadataManager,
542+
"fso-file2",
543+
BUCKET_NAME,
544+
VOLUME_NAME,
545+
file2Key,
546+
KEY_ONE_OBJECT_ID,
547+
dir1Id,
548+
BUCKET_ONE_OBJECT_ID,
549+
VOL_OBJECT_ID,
550+
Collections.singletonList(omKeyLocationInfoGroup),
551+
BucketLayout.FILE_SYSTEM_OPTIMIZED,
552+
KEY_ONE_SIZE);
553+
554+
writeKeyToOm(reconOMMetadataManager,
555+
"fso-file2",
556+
BUCKET_NAME,
557+
VOLUME_NAME,
558+
file2Key,
559+
KEY_ONE_OBJECT_ID,
560+
dir2Id,
561+
BUCKET_ONE_OBJECT_ID,
562+
VOL_OBJECT_ID,
563+
Collections.singletonList(omKeyLocationInfoGroup),
564+
BucketLayout.FILE_SYSTEM_OPTIMIZED,
565+
KEY_ONE_SIZE);
566+
567+
writeKeyToOm(reconOMMetadataManager,
568+
"fso-file2",
569+
BUCKET_NAME,
570+
VOLUME_NAME,
571+
file2Key,
572+
KEY_ONE_OBJECT_ID,
573+
dir3Id,
574+
BUCKET_ONE_OBJECT_ID,
575+
VOL_OBJECT_ID,
576+
Collections.singletonList(omKeyLocationInfoGroup),
577+
BucketLayout.FILE_SYSTEM_OPTIMIZED,
578+
KEY_ONE_SIZE);
579+
580+
// Reprocess container key mappings.
581+
ContainerKeyMapperTask containerKeyMapperTask =
582+
new ContainerKeyMapperTask(reconContainerMetadataManager, omConfiguration);
583+
containerKeyMapperTask.reprocess(reconOMMetadataManager);
584+
585+
// With our changes using the raw key prefix as the unique identifier,
586+
// we expect six distinct entries in container 1.
587+
keyPrefixesForContainer = reconContainerMetadataManager.getKeyPrefixesForContainer(1L);
588+
assertEquals(6, keyPrefixesForContainer.size());
589+
}
590+
475591
private OmKeyInfo buildOmKeyInfo(String volume,
476592
String bucket,
477593
String key,

0 commit comments

Comments
 (0)