Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

HDDS-7253. Fix exception when '/' in key name #4038

Merged
merged 10 commits into from Dec 4, 2022
Merged

HDDS-7253. Fix exception when '/' in key name #4038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
d348fa9
HDDS-7253. Fix exception when '/' in key name
xichen01 Sep 22, 2022
ed80f87
checkstyle
xichen01 Sep 22, 2022
075f79b
Fix unit test; Improve code
xichen01 Sep 25, 2022
3f68af1
checkstyle
xichen01 Sep 25, 2022
17de24c
Adaptation createDirectoryKey modified
xichen01 Oct 10, 2022
cafa3a9
Remove catch Exection; Change createFile to openKey
xichen01 Oct 12, 2022
8c38928
fix compile issue
xichen01 Oct 13, 2022
3cf696e
fix integration test testLookupKeyWithLocation
xichen01 Oct 13, 2022
dc8fdd0
Extract "create fake dir" logic as a function
xichen01 Oct 14, 2022
8939baf
fix can not seek the correct key in some case
xichen01 Dec 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
81 changes: 77 additions & 4 deletions ...-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java
View file
Open in desktop
Expand Up @@ -153,6 +153,7 @@ public TestOzoneFileSystem(boolean setDefaultFs, boolean enableOMRatis) {
private static OzoneManagerProtocol writeClient;
private static FileSystem fs;
private static OzoneFileSystem o3fs;
private static OzoneBucket ozoneBucket;
private static String volumeName;
private static String bucketName;
private static Trash trash;
Expand All @@ -179,10 +180,9 @@ private void init() throws Exception {
writeClient = cluster.getRpcClient().getObjectStore()
.getClientProxy().getOzoneManagerClient();
// create a volume and a bucket to be used by OzoneFileSystem
OzoneBucket bucket =
TestDataUtil.createVolumeAndBucket(cluster, bucketLayout);
volumeName = bucket.getVolumeName();
bucketName = bucket.getName();
ozoneBucket = TestDataUtil.createVolumeAndBucket(cluster, bucketLayout);
volumeName = ozoneBucket.getVolumeName();
bucketName = ozoneBucket.getName();

String rootPath = String.format("%s://%s.%s/",
OzoneConsts.OZONE_URI_SCHEME, bucketName, volumeName);
Expand Down Expand Up @@ -334,6 +334,30 @@ public void testMakeDirsWithAnExistingDirectoryPath() throws Exception {
assertTrue("Shouldn't send error if dir exists", status);
}

@Test
public void testMakeDirsWithAnFakeDirectory() throws Exception {
/*
* Op 1. commit a key -> "dir1/dir2/key1"
* Op 2. create dir -> "dir1/testDir", the dir1 is a fake dir,
* "dir1/testDir" can be created normal
*/

String fakeGrandpaKey = "dir1";
String fakeParentKey = fakeGrandpaKey + "/dir2";
String fullKeyName = fakeParentKey + "/key1";
TestDataUtil.createKey(ozoneBucket, fullKeyName, "");

// /dir1/dir2 should not exist
assertFalse(fs.exists(new Path(fakeParentKey)));

// /dir1/dir2/key2 should be created because has a fake parent directory
Path subdir = new Path(fakeParentKey, "key2");
assertTrue(fs.mkdirs(subdir));
// the intermediate directories /dir1 and /dir1/dir2 will be created too
assertTrue(fs.exists(new Path(fakeGrandpaKey)));
assertTrue(fs.exists(new Path(fakeParentKey)));
}

@Test
public void testCreateWithInvalidPaths() throws Exception {
// Test for path with ..
Expand Down Expand Up @@ -727,6 +751,37 @@ public void testListStatusOnLargeDirectory() throws Exception {
}
}

@Test
public void testListStatusOnKeyNameContainDelimiter() throws Exception {
/*
* op1: create a key -> "dir1/dir2/key1"
* op2: `ls /` child dir "/dir1/" will be return
* op2: `ls /dir1` child dir "/dir1/dir2/" will be return
* op3: `ls /dir1/dir2` file "/dir1/dir2/key" will be return
*
* the "/dir1", "/dir1/dir2/" are fake directory
* */
String keyName = "dir1/dir2/key1";
TestDataUtil.createKey(ozoneBucket, keyName, "");
FileStatus[] fileStatuses;

fileStatuses = fs.listStatus(new Path("/"));
assertEquals(1, fileStatuses.length);
assertEquals("/dir1", fileStatuses[0].getPath().toUri().getPath());
assertTrue(fileStatuses[0].isDirectory());

fileStatuses = fs.listStatus(new Path("/dir1"));
assertEquals(1, fileStatuses.length);
assertEquals("/dir1/dir2", fileStatuses[0].getPath().toUri().getPath());
assertTrue(fileStatuses[0].isDirectory());

fileStatuses = fs.listStatus(new Path("/dir1/dir2"));
assertEquals(1, fileStatuses.length);
assertEquals("/dir1/dir2/key1",
fileStatuses[0].getPath().toUri().getPath());
assertTrue(fileStatuses[0].isFile());
}

/**
* Cleanup files and directories.
*
Expand Down Expand Up @@ -1273,6 +1328,24 @@ public void testRenameFileToDir() throws Exception {
"file1")));
}

@Test
public void testRenameContainDelimiterFile() throws Exception {
String fakeGrandpaKey = "dir1";
String fakeParentKey = fakeGrandpaKey + "/dir2";
String sourceKeyName = fakeParentKey + "/key1";
String targetKeyName = fakeParentKey + "/key2";
TestDataUtil.createKey(ozoneBucket, sourceKeyName, "");

Path sourcePath = new Path(fs.getUri().toString() + "/" + sourceKeyName);
Path targetPath = new Path(fs.getUri().toString() + "/" + targetKeyName);
assertTrue(fs.rename(sourcePath, targetPath));
assertFalse(fs.exists(sourcePath));
assertTrue(fs.exists(targetPath));
// intermediate directories will not be created
assertFalse(fs.exists(new Path(fakeGrandpaKey)));
assertFalse(fs.exists(new Path(fakeParentKey)));
}


/**
* Fails if the (a) parent of dst does not exist or (b) parent is a file.
Expand Down
80 changes: 76 additions & 4 deletions ...p-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java
View file
Open in desktop
Expand Up @@ -800,19 +800,20 @@ public void testLookupKeyWithLocation() throws IOException {
List<OmKeyLocationInfo> locationList =
keySession.getKeyInfo().getLatestVersionLocations().getLocationList();
Assert.assertEquals(1, locationList.size());
long containerID = locationList.get(0).getContainerID();
locationInfoList.add(
new OmKeyLocationInfo.Builder().setPipeline(pipeline)
.setBlockID(new BlockID(locationList.get(0).getContainerID(),
.setBlockID(new BlockID(containerID,
locationList.get(0).getLocalID())).build());
keyArgs.setLocationInfoList(locationInfoList);

writeClient.commitKey(keyArgs, keySession.getId());
ContainerInfo containerInfo = new ContainerInfo.Builder().setContainerID(1L)
.setPipelineID(pipeline.getId()).build();
ContainerInfo containerInfo = new ContainerInfo.Builder()
.setContainerID(containerID).setPipelineID(pipeline.getId()).build();
List<ContainerWithPipeline> containerWithPipelines = Arrays.asList(
new ContainerWithPipeline(containerInfo, pipeline));
when(mockScmContainerClient.getContainerWithPipelineBatch(
Arrays.asList(1L))).thenReturn(containerWithPipelines);
Arrays.asList(containerID))).thenReturn(containerWithPipelines);

OmKeyInfo key = keyManager.lookupKey(keyArgs, null);
Assert.assertEquals(key.getKeyName(), keyName);
Expand Down Expand Up @@ -1273,6 +1274,77 @@ public void testListStatus() throws IOException {
}
}

@Test
public void testGetFileStatus() throws IOException {
// create a key
String keyName = RandomStringUtils.randomAlphabetic(5);
OmKeyArgs keyArgs = createBuilder()
.setKeyName(keyName)
.setLatestVersionLocation(true)
.build();
writeClient.createFile(keyArgs, false, false);
OpenKeySession keySession = writeClient.createFile(keyArgs, true, true);
keyArgs.setLocationInfoList(
keySession.getKeyInfo().getLatestVersionLocations().getLocationList());
writeClient.commitKey(keyArgs, keySession.getId());
OzoneFileStatus ozoneFileStatus = keyManager.getFileStatus(keyArgs);
Assert.assertEquals(keyName, ozoneFileStatus.getKeyInfo().getFileName());
}

@Test
public void testGetFileStatusWithFakeDir() throws IOException {
String parentDir = "dir1";
String fileName = "file1";
String keyName1 = parentDir + OZONE_URI_DELIMITER + fileName;
// "dir1.file1" used to confirm that it will not affect
// the creation of fake directory "dir1"
String keyName2 = parentDir + "." + fileName;
OzoneFileStatus ozoneFileStatus;

// create a key "dir1/key1"
OmKeyArgs keyArgs = createBuilder().setKeyName(keyName1).build();
OpenKeySession keySession = writeClient.openKey(keyArgs);
keyArgs.setLocationInfoList(
keySession.getKeyInfo().getLatestVersionLocations().getLocationList());
writeClient.commitKey(keyArgs, keySession.getId());

// create a key "dir1.key"
keyArgs = createBuilder().setKeyName(keyName2).build();
keySession = writeClient.createFile(keyArgs, true, true);
keyArgs.setLocationInfoList(
keySession.getKeyInfo().getLatestVersionLocations().getLocationList());
writeClient.commitKey(keyArgs, keySession.getId());

// verify key "dir1/key1" and "dir1.key1" can be found in the bucket, and
// "dir1" can not be found in the bucket
Assert.assertNull(metadataManager.getKeyTable(getDefaultBucketLayout())
.get(metadataManager.getOzoneKey(VOLUME_NAME, BUCKET_NAME, parentDir)));
Assert.assertNotNull(metadataManager.getKeyTable(getDefaultBucketLayout())
.get(metadataManager.getOzoneKey(VOLUME_NAME, BUCKET_NAME, keyName1)));
Assert.assertNotNull(metadataManager.getKeyTable(getDefaultBucketLayout())
.get(metadataManager.getOzoneKey(VOLUME_NAME, BUCKET_NAME, keyName2)));

// get a non-existing "dir1", since the key is prefixed "dir1/key1",
// a fake "/dir1" will be returned
keyArgs = createBuilder().setKeyName(parentDir).build();
ozoneFileStatus = keyManager.getFileStatus(keyArgs);
Assert.assertEquals(parentDir, ozoneFileStatus.getKeyInfo().getFileName());
Assert.assertTrue(ozoneFileStatus.isDirectory());

// get a non-existing "dir", since the key is not prefixed "dir1/key1",
// a `OMException` will be thrown
keyArgs = createBuilder().setKeyName("dir").build();
OmKeyArgs finalKeyArgs = keyArgs;
Assert.assertThrows(OMException.class, () -> keyManager.getFileStatus(
finalKeyArgs));

// get a file "dir1/key1"
keyArgs = createBuilder().setKeyName(keyName1).build();
ozoneFileStatus = keyManager.getFileStatus(keyArgs);
Assert.assertEquals(fileName, ozoneFileStatus.getKeyInfo().getFileName());
Assert.assertTrue(ozoneFileStatus.isFile());
}

@Test
public void testRefreshPipeline() throws Exception {

Expand Down
69 changes: 57 additions & 12 deletions hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java
View file
Open in desktop
Expand Up @@ -1209,6 +1209,8 @@ private OzoneFileStatus getOzoneFileStatus(OmKeyArgs args,
final String keyName = args.getKeyName();

OmKeyInfo fileKeyInfo = null;
OmKeyInfo dirKeyInfo = null;
OmKeyInfo fakeDirKeyInfo = null;
metadataManager.getLock().acquireReadLock(BUCKET_LOCK, volumeName,
bucketName);
try {
Expand All @@ -1221,28 +1223,27 @@ private OzoneFileStatus getOzoneFileStatus(OmKeyArgs args,
// Check if the key is a file.
String fileKeyBytes = metadataManager.getOzoneKey(
volumeName, bucketName, keyName);
fileKeyInfo = metadataManager
.getKeyTable(getBucketLayout(metadataManager, volumeName, bucketName))
.get(fileKeyBytes);
BucketLayout layout =
getBucketLayout(metadataManager, volumeName, bucketName);
fileKeyInfo = metadataManager.getKeyTable(layout).get(fileKeyBytes);
String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);

// Check if the key is a directory.
if (fileKeyInfo == null) {
String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
String dirKeyBytes = metadataManager.getOzoneKey(
volumeName, bucketName, dirKey);
OmKeyInfo dirKeyInfo = metadataManager.getKeyTable(
getBucketLayout(metadataManager, volumeName, bucketName))
.get(dirKeyBytes);
if (dirKeyInfo != null) {
return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true);
dirKeyInfo = metadataManager.getKeyTable(layout).get(dirKeyBytes);
if (dirKeyInfo == null) {
fakeDirKeyInfo =
createFakeDirIfShould(volumeName, bucketName, keyName, layout);
}
}
} finally {
metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
bucketName);

// if the key is a file then do refresh pipeline info in OM by asking SCM
if (fileKeyInfo != null) {
// if the key is a file
// then do refresh pipeline info in OM by asking SCM
if (args.getLatestVersionLocation()) {
slimLocationVersion(fileKeyInfo);
}
Expand All @@ -1257,10 +1258,21 @@ private OzoneFileStatus getOzoneFileStatus(OmKeyArgs args,
sortDatanodes(clientAddress, fileKeyInfo);
}
}
return new OzoneFileStatus(fileKeyInfo, scmBlockSize, false);
}
}

if (fileKeyInfo != null) {
return new OzoneFileStatus(fileKeyInfo, scmBlockSize, false);
}

if (dirKeyInfo != null) {
return new OzoneFileStatus(dirKeyInfo, scmBlockSize, true);
}

if (fakeDirKeyInfo != null) {
return new OzoneFileStatus(fakeDirKeyInfo, scmBlockSize, true);
}

// Key is not found, throws exception
if (LOG.isDebugEnabled()) {
LOG.debug("Unable to get file status for the key: volume: {}, bucket:" +
Expand All @@ -1272,6 +1284,38 @@ private OzoneFileStatus getOzoneFileStatus(OmKeyArgs args,
FILE_NOT_FOUND);
}

/**
* Create a fake directory if the key is a path prefix,
* otherwise returns null.
* Some keys may contain '/' Ozone will treat '/' as directory separator
* such as : key name is 'a/b/c', 'a' and 'b' may not really exist,
* but Ozone treats 'a' and 'b' as a directory.
* we need create a fake directory 'a' or 'a/b'
*
* @return OmKeyInfo if the key is a path prefix, otherwise returns null.
*/
private OmKeyInfo createFakeDirIfShould(String volume, String bucket,
String keyName, BucketLayout layout) throws IOException {
OmKeyInfo fakeDirKeyInfo = null;
String dirKey = OzoneFSUtils.addTrailingSlashIfNeeded(keyName);
String fileKeyBytes = metadataManager.getOzoneKey(volume, bucket, keyName);
Table.KeyValue<String, OmKeyInfo> keyValue =
metadataManager.getKeyTable(layout).iterator()
.seek(OzoneFSUtils.addTrailingSlashIfNeeded(fileKeyBytes));

if (keyValue != null) {
Path fullPath = Paths.get(keyValue.getValue().getKeyName());
Path subPath = Paths.get(dirKey);
OmKeyInfo omKeyInfo = keyValue.getValue();
if (fullPath.startsWith(subPath)) {
// create fake directory
fakeDirKeyInfo = createDirectoryKey(omKeyInfo, dirKey);
}
}

return fakeDirKeyInfo;
}


private OzoneFileStatus getOzoneFileStatusFSO(OmKeyArgs args,
String clientAddress, boolean skipFileNotFoundError) throws IOException {
Expand Down Expand Up @@ -1349,6 +1393,7 @@ private OmKeyInfo createDirectoryKey(OmKeyInfo keyInfo, String keyName)
.setVolumeName(keyInfo.getVolumeName())
.setBucketName(keyInfo.getBucketName())
.setKeyName(dir)
.setFileName(OzoneFSUtils.getFileName(keyName))
.setOmKeyLocationInfos(Collections.singletonList(
new OmKeyLocationInfoGroup(0, new ArrayList<>())))
.setCreationTime(Time.now())
Expand Down