You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/09/23 08:38:21 UTC
[GitHub] [ozone] sadanand48 commented on a diff in pull request #3774: HDDS-7253. Fix exception when '/' in key name
sadanand48 commented on code in PR #3774:
URL: https://github.com/apache/ozone/pull/3774#discussion_r978381297
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java:
##########
@@ -1272,6 +1272,67 @@ 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 key = "key1";
+ String fullKeyName = parentDir + OZONE_URI_DELIMITER + key;
+ OzoneFileStatus ozoneFileStatus;
+
+ // create a key "dir1/key1"
+ OmKeyArgs keyArgs = createBuilder().setKeyName(fullKeyName).build();
+ OpenKeySession keySession = writeClient.createFile(keyArgs, true, true);
+ keyArgs.setLocationInfoList(
+ keySession.getKeyInfo().getLatestVersionLocations().getLocationList());
+ writeClient.commitKey(keyArgs, keySession.getId());
+
+ // verify
+ String keyArg;
+ keyArg = metadataManager.getOzoneKey(VOLUME_NAME, BUCKET_NAME, parentDir);
+ Assert.assertNull(
+ metadataManager.getKeyTable(getDefaultBucketLayout()).get(keyArg));
Review Comment:
The key table will have an entry for the parentDir here prefixed with "/" since we are using createFile i.e the below code would throw exception as the parentDir is present in keyTable. Using `openKey` should solve this
```
keyArg = metadataManager.getOzoneKey(VOLUME_NAME, BUCKET_NAME, parentDir + "/" );
Assert.assertNull(
metadataManager.getKeyTable(getDefaultBucketLayout()).get(keyArg));
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestKeyManagerImpl.java:
##########
@@ -1272,6 +1272,67 @@ 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 key = "key1";
+ String fullKeyName = parentDir + OZONE_URI_DELIMITER + key;
+ OzoneFileStatus ozoneFileStatus;
+
+ // create a key "dir1/key1"
+ OmKeyArgs keyArgs = createBuilder().setKeyName(fullKeyName).build();
+ OpenKeySession keySession = writeClient.createFile(keyArgs, true, true);
Review Comment:
`writeClient.createFile` would always create intermediate dirs so the dirs returned by getFileStatus are not fake.
Instead we should use `writeClient.openKey` to test this correctly
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java:
##########
@@ -1262,44 +1264,70 @@ 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);
+
+ // Check if the key is a prefix.
+ // 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'
+ if (dirKeyInfo == null) {
+ Table.KeyValue<String, OmKeyInfo> keyValue =
+ metadataManager.getKeyTable(layout).iterator().seek(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.getVolumeName(),
+ omKeyInfo.getBucketName(),
+ dirKey,
+ omKeyInfo.getAcls());
+ }
+ }
}
}
} finally {
metadataManager.getLock().releaseReadLock(BUCKET_LOCK, volumeName,
bucketName);
+ }
+ if (fileKeyInfo != null) {
Review Comment:
It's better to keep these below conditions inside the finally block, as they were earlier.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java:
##########
@@ -1033,6 +1084,18 @@ public void testRenameFileToDir() throws Exception {
"file1")));
}
+ @Test
+ public void testRenameContainDelimiterFile() throws Exception {
+ String sourceKeyName = "dir1/dir2/key1";
+ String targetKeyName = "dir1/dir2/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));
Review Comment:
We could also check if `fs.rename `created intermediate dirs or not.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java:
##########
@@ -335,6 +335,26 @@ 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 fakeParentKey = "dir1/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));
Review Comment:
In here we can check `fs.exists` for dir1 & dir2.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org