From bac8807c8b7abb4864aed921585f6e6fc5e9cd5c Mon Sep 17 00:00:00 2001 From: Arpit Agarwal Date: Wed, 14 Nov 2018 16:12:06 -0800 Subject: [PATCH] HDDS-819. Match OzoneFileSystem behavior with S3AFileSystem. Contributed by Hanisha Koneru. --- .../hadoop/fs/ozone/OzoneFileSystem.java | 283 +++++++++++++++--- .../hadoop/fs/ozone/TestOzoneFileSystem.java | 174 ++++++++++- 2 files changed, 414 insertions(+), 43 deletions(-) diff --git a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java index 13363827bc..78b6e5d210 100644 --- a/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java +++ b/hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/OzoneFileSystem.java @@ -24,13 +24,17 @@ import java.net.URISyntaxException; import java.util.ArrayList; import java.util.EnumSet; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Iterator; import java.util.regex.Matcher; import java.util.regex.Pattern; import com.google.common.base.Preconditions; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -199,17 +203,7 @@ public FSDataOutputStream create(Path f, FsPermission permission, deleteObject(key); } } catch (FileNotFoundException ignored) { - // check if the parent directory needs to be created - Path parent = f.getParent(); - try { - // create all the directories for the parent - FileStatus parentStatus = getFileStatus(parent); - LOG.trace("parent key:{} status:{}", key, parentStatus); - } catch (FileNotFoundException e) { - mkdirs(parent); - } - // This exception needs to ignored as this means that the file currently - // does not exists and a new file can thus be created. + // this means the file is not found } OzoneOutputStream ozoneOutputStream = @@ -390,8 +384,14 @@ boolean processKey(String key) throws IOException { } } - @Override - public boolean delete(Path f, boolean recursive) throws IOException { + /** + * Deletes the children of the input dir path by iterating though the + * DeleteIterator. + * @param f directory path to be deleted + * @return true if successfully deletes all required keys, false otherwise + * @throws IOException + */ + private boolean innerDelete(Path f, boolean recursive) throws IOException { LOG.trace("delete() path:{} recursive:{}", f, recursive); try { DeleteIterator iterator = new DeleteIterator(f, recursive); @@ -402,35 +402,185 @@ public boolean delete(Path f, boolean recursive) throws IOException { } } + @Override + public boolean delete(Path f, boolean recursive) throws IOException { + LOG.debug("Delete path {} - recursive {}", f, recursive); + FileStatus status; + try { + status = getFileStatus(f); + } catch (FileNotFoundException ex) { + LOG.warn("delete: Path does not exist: {}", f); + return false; + } + + String key = pathToKey(f); + boolean result; + + if (status.isDirectory()) { + LOG.debug("delete: Path is a directory: {}", f); + key = addTrailingSlashIfNeeded(key); + + if (key.equals("/")) { + LOG.warn("Cannot delete root directory."); + return false; + } + + result = innerDelete(f, recursive); + } else { + LOG.debug("delete: Path is a file: {}", f); + result = deleteObject(key); + } + + if (result) { + // If this delete operation removes all files/directories from the + // parent direcotry, then an empty parent directory must be created. + Path parent = f.getParent(); + if (parent != null && !parent.isRoot()) { + createFakeDirectoryIfNecessary(parent); + } + } + + return result; + } + + /** + * Create a fake parent directory key if it does not already exist and no + * other child of this parent directory exists. + * @param f path to the fake parent directory + * @throws IOException + */ + private void createFakeDirectoryIfNecessary(Path f) throws IOException { + String key = pathToKey(f); + if (!key.isEmpty() && !o3Exists(f)) { + LOG.debug("Creating new fake directory at {}", f); + String dirKey = addTrailingSlashIfNeeded(key); + createDirectory(dirKey); + } + } + + /** + * Check if a file or directory exists corresponding to given path. + * @param f path to file/directory. + * @return true if it exists, false otherwise. + * @throws IOException + */ + private boolean o3Exists(final Path f) throws IOException { + Path path = makeQualified(f); + try { + getFileStatus(path); + return true; + } catch (FileNotFoundException ex) { + return false; + } + } + private class ListStatusIterator extends OzoneListingIterator { - private List statuses = new ArrayList<>(LISTING_PAGE_SIZE); - private Path f; + // _fileStatuses_ maintains a list of file(s) which is either the input + // path itself or a child of the input directory path. + private List fileStatuses = new ArrayList<>(LISTING_PAGE_SIZE); + // _subDirStatuses_ maintains a list of sub-dirs of the input directory + // path. + private Map subDirStatuses = + new HashMap<>(LISTING_PAGE_SIZE); + private Path f; // the input path ListStatusIterator(Path f) throws IOException { super(f); this.f = f; } + /** + * Add the key to the listStatus result if the key corresponds to the + * input path or is an immediate child of the input path. + * @param key key to be processed + * @return always returns true + * @throws IOException + */ boolean processKey(String key) throws IOException { Path keyPath = new Path(OZONE_URI_DELIMITER + key); if (key.equals(getPathKey())) { if (pathIsDirectory()) { + // if input path is a directory, we add the sub-directories and + // files under this directory. return true; } else { - statuses.add(getFileStatus(keyPath)); + addFileStatus(keyPath); return true; } } - // left with only subkeys now + // Left with only subkeys now + // We add only the immediate child files and sub-dirs i.e. we go only + // upto one level down the directory tree structure. if (pathToKey(keyPath.getParent()).equals(pathToKey(f))) { - // skip keys which are for subdirectories of the directory - statuses.add(getFileStatus(keyPath)); + // This key is an immediate child. Can be file or directory + if (key.endsWith(OZONE_URI_DELIMITER)) { + // Key is a directory + addSubDirStatus(keyPath); + } else { + addFileStatus(keyPath); + } + } else { + // This key is not the immediate child of the input directory. So we + // traverse the parent tree structure of this key until we get the + // immediate child of the input directory. + Path immediateChildPath = getImmediateChildPath(keyPath.getParent()); + addSubDirStatus(immediateChildPath); } return true; } + /** + * Adds the FileStatus of keyPath to final result of listStatus. + * @param filePath path to the file + * @throws FileNotFoundException + */ + void addFileStatus(Path filePath) throws IOException { + fileStatuses.add(getFileStatus(filePath)); + } + + /** + * Adds the FileStatus of the subdir to final result of listStatus, if not + * already included. + * @param dirPath path to the dir + * @throws FileNotFoundException + */ + void addSubDirStatus(Path dirPath) throws FileNotFoundException { + // Check if subdir path is already included in statuses. + if (!subDirStatuses.containsKey(dirPath)) { + subDirStatuses.put(dirPath, innerGetFileStatusForDir(dirPath)); + } + } + + /** + * Traverse the parent directory structure of keyPath to determine the + * which parent/ grand-parent/.. is the immediate child of the input path f. + * @param keyPath path whose parent directory structure should be traversed. + * @return immediate child path of the input path f. + * @return immediate child path of the input path f. + */ + Path getImmediateChildPath(Path keyPath) { + Path path = keyPath; + Path parent = path.getParent(); + while (parent != null && !parent.isRoot()) { + if (pathToKey(parent).equals(pathToKey(f))) { + return path; + } + path = parent; + parent = path.getParent(); + } + return null; + } + + /** + * Return the result of listStatus operation. If the input path is a + * file, return the status for only that file. If the input path is a + * directory, return the statuses for all the child files and sub-dirs. + */ FileStatus[] getStatuses() { - return statuses.toArray(new FileStatus[statuses.size()]); + List result = Stream.concat( + fileStatuses.stream(), subDirStatuses.values().stream()) + .collect(Collectors.toList()); + return result.toArray(new FileStatus[result.size()]); } } @@ -529,28 +679,58 @@ public FileStatus getFileStatus(Path f) throws IOException { bucket.getCreationTime(), qualifiedPath); } - // consider this a file and get key status - OzoneKey meta = getKeyInfo(key); - if (meta == null) { - key = addTrailingSlashIfNeeded(key); - meta = getKeyInfo(key); - } - - if (meta == null) { - LOG.trace("File:{} not found", f); - throw new FileNotFoundException(f + ": No such file or directory!"); - } else if (isDirectory(meta)) { - return new FileStatus(0, true, 1, 0, - meta.getModificationTime(), 0, - FsPermission.getDirDefault(), getUsername(), getUsername(), - qualifiedPath); - } else { - //TODO: Fetch replication count from ratis config - return new FileStatus(meta.getDataSize(), false, 1, - getDefaultBlockSize(f), meta.getModificationTime(), 0, + // Check if the key exists + OzoneKey ozoneKey = getKeyInfo(key); + if (ozoneKey != null) { + LOG.debug("Found exact file for path {}: normal file", f); + return new FileStatus(ozoneKey.getDataSize(), false, 1, + getDefaultBlockSize(f), ozoneKey.getModificationTime(), 0, FsPermission.getFileDefault(), getUsername(), getUsername(), qualifiedPath); } + + return innerGetFileStatusForDir(f); + } + + /** + * Get the FileStatus for input directory path. + * They key corresponding to input path is appended with a trailing slash + * to return only the corresponding directory key in the bucket. + * @param f directory path + * @return FileStatus for the input directory path + * @throws FileNotFoundException + */ + public FileStatus innerGetFileStatusForDir(Path f) + throws FileNotFoundException { + Path qualifiedPath = f.makeQualified(uri, workingDir); + String key = pathToKey(qualifiedPath); + key = addTrailingSlashIfNeeded(key); + + OzoneKey ozoneKey = getKeyInfo(key); + if(ozoneKey != null) { + if (isDirectory(ozoneKey)) { + // Key is a directory + LOG.debug("Found file (with /) for path {}: fake directory", f); + } else { + // Key is a file with trailing slash + LOG.warn("Found file (with /) for path {}: real file? should not " + + "happen", f, key); + } + return new FileStatus(0, true, 1, 0, + ozoneKey.getModificationTime(), 0, + FsPermission.getDirDefault(), getUsername(), getUsername(), + qualifiedPath); + } + + // File or directory corresponding to input path does not exist. + // Check if there exists a key prefixed with this key. + boolean hasChildren = bucket.listKeys(key).hasNext(); + if (hasChildren) { + return new FileStatus(0, true, 1, 0, 0, 0, FsPermission.getDirDefault(), + getUsername(), getUsername(), qualifiedPath); + } + + throw new FileNotFoundException(f + ": No such file or directory!"); } /** @@ -562,7 +742,7 @@ private OzoneKey getKeyInfo(String key) { try { return bucket.getKey(key); } catch (IOException e) { - LOG.trace("Key:{} does not exists", key); + LOG.trace("Key:{} does not exist", key); return null; } } @@ -651,6 +831,13 @@ public String toString() { + "}"; } + /** + * This class provides an interface to iterate through all the keys in the + * bucket prefixed with the input path key and process them. + * + * Each implementing class should define how the keys should be processed + * through the processKey() function. + */ private abstract class OzoneListingIterator { private final Path path; private final FileStatus status; @@ -668,9 +855,23 @@ private abstract class OzoneListingIterator { keyIterator = bucket.listKeys(pathKey); } + /** + * The output of processKey determines if further iteration through the + * keys should be done or not. + * @return true if we should continue iteration of keys, false otherwise. + * @throws IOException + */ abstract boolean processKey(String key) throws IOException; - // iterates all the keys in the particular path + /** + * Iterates thorugh all the keys prefixed with the input path's key and + * processes the key though processKey(). + * If for any key, the processKey() returns false, then the iteration is + * stopped and returned with false indicating that all the keys could not + * be processed successfully. + * @return true if all keys are processed successfully, false otherwise. + * @throws IOException + */ boolean iterate() throws IOException { LOG.trace("Iterating path {}", path); if (status.isDirectory()) { diff --git a/hadoop-ozone/ozonefs/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java b/hadoop-ozone/ozonefs/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java index ba5253f921..075a105cc2 100644 --- a/hadoop-ozone/ozonefs/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java +++ b/hadoop-ozone/ozonefs/src/test/java/org/apache/hadoop/fs/ozone/TestOzoneFileSystem.java @@ -18,22 +18,192 @@ package org.apache.hadoop.fs.ozone; +import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.RandomStringUtils; +import org.apache.hadoop.fs.CommonConfigurationKeysPublic; +import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.contract.ContractTestUtils; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdfs.server.datanode.ObjectStoreHandler; +import org.apache.hadoop.ozone.MiniOzoneCluster; import org.apache.hadoop.ozone.OzoneConsts; -import org.junit.Assert; +import org.apache.hadoop.ozone.client.rest.OzoneException; +import org.apache.hadoop.ozone.web.handlers.BucketArgs; +import org.apache.hadoop.ozone.web.handlers.KeyArgs; +import org.apache.hadoop.ozone.web.handlers.UserArgs; +import org.apache.hadoop.ozone.web.handlers.VolumeArgs; +import org.apache.hadoop.ozone.web.interfaces.StorageHandler; +import org.apache.hadoop.ozone.web.response.KeyInfo; +import org.apache.hadoop.ozone.web.utils.OzoneUtils; +import org.apache.hadoop.test.GenericTestUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import java.io.IOException; +import org.junit.rules.Timeout; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; /** * Ozone file system tests that are not covered by contract tests. */ public class TestOzoneFileSystem { + @Rule + public Timeout globalTimeout = new Timeout(300_000); + + private static MiniOzoneCluster cluster = null; + + private static FileSystem fs; + private static OzoneFileSystem o3fs; + + private static StorageHandler storageHandler; + private static UserArgs userArgs; + private String volumeName; + private String bucketName; + private String userName; + private String rootPath; + + @Before + public void init() throws Exception { + OzoneConfiguration conf = new OzoneConfiguration(); + cluster = MiniOzoneCluster.newBuilder(conf) + .setNumDatanodes(3) + .build(); + cluster.waitForClusterToBeReady(); + storageHandler = + new ObjectStoreHandler(conf).getStorageHandler(); + + // create a volume and a bucket to be used by OzoneFileSystem + userName = "user" + RandomStringUtils.randomNumeric(5); + String adminName = "admin" + RandomStringUtils.randomNumeric(5); + volumeName = "volume" + RandomStringUtils.randomNumeric(5); + bucketName = "bucket" + RandomStringUtils.randomNumeric(5); + userArgs = new UserArgs(null, OzoneUtils.getRequestID(), + null, null, null, null); + VolumeArgs volumeArgs = new VolumeArgs(volumeName, userArgs); + volumeArgs.setUserName(userName); + volumeArgs.setAdminName(adminName); + storageHandler.createVolume(volumeArgs); + BucketArgs bucketArgs = new BucketArgs(volumeName, bucketName, userArgs); + storageHandler.createBucket(bucketArgs); + + rootPath = String.format("%s://%s.%s/", + OzoneConsts.OZONE_URI_SCHEME, bucketName, volumeName); + + // Set the fs.defaultFS and start the filesystem + conf.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, rootPath); + fs = FileSystem.get(conf); + o3fs = (OzoneFileSystem) fs; + } + + @After + public void teardown() throws IOException { + if (cluster != null) { + cluster.shutdown(); + } + IOUtils.closeQuietly(fs); + IOUtils.closeQuietly(storageHandler); + } + @Test public void testOzoneFsServiceLoader() throws IOException { - Assert.assertEquals( + assertEquals( FileSystem.getFileSystemClass(OzoneConsts.OZONE_URI_SCHEME, null), OzoneFileSystem.class); } + + @Test + public void testCreateDoesNotAddParentDirKeys() throws Exception { + Path grandparent = new Path("/testCreateDoesNotAddParentDirKeys"); + Path parent = new Path(grandparent, "parent"); + Path child = new Path(parent, "child"); + ContractTestUtils.touch(fs, child); + + KeyInfo key = getKey(child, false); + assertEquals(key.getKeyName(), o3fs.pathToKey(child)); + + // Creating a child should not add parent keys to the bucket + try { + getKey(parent, true); + } catch (IOException ex) { + assertKeyNotFoundException(ex); + } + + // List status on the parent should show the child file + assertEquals("List status of parent should include the 1 child file", 1L, + (long)fs.listStatus(parent).length); + assertTrue("Parent directory does not appear to be a directory", + fs.getFileStatus(parent).isDirectory()); + } + + @Test + public void testDeleteCreatesFakeParentDir() throws Exception { + Path grandparent = new Path("/testDeleteCreatesFakeParentDir"); + Path parent = new Path(grandparent, "parent"); + Path child = new Path(parent, "child"); + ContractTestUtils.touch(fs, child); + + // Verify that parent dir key does not exist + // Creating a child should not add parent keys to the bucket + try { + getKey(parent, true); + } catch (IOException ex) { + assertKeyNotFoundException(ex); + } + + // Delete the child key + fs.delete(child, false); + + // Deleting the only child should create the parent dir key if it does + // not exist + String parentKey = o3fs.pathToKey(parent) + "/"; + KeyInfo parentKeyInfo = getKey(parent, true); + assertEquals(parentKey, parentKeyInfo.getKeyName()); + } + + @Test + public void testListStatus() throws Exception { + Path parent = new Path("/testListStatus"); + Path file1 = new Path(parent, "key1"); + Path file2 = new Path(parent, "key1/key2"); + ContractTestUtils.touch(fs, file1); + ContractTestUtils.touch(fs, file2); + + + // ListStatus on a directory should return all subdirs along with + // files, even if there exists a file and sub-dir with the same name. + FileStatus[] fileStatuses = o3fs.listStatus(parent); + assertEquals("FileStatus did not return all children of the directory", + 2, fileStatuses.length); + + // ListStatus should return only the immediate children of a directory. + Path file3 = new Path(parent, "dir1/key3"); + Path file4 = new Path(parent, "dir1/key4"); + ContractTestUtils.touch(fs, file3); + ContractTestUtils.touch(fs, file4); + fileStatuses = o3fs.listStatus(parent); + assertEquals("FileStatus did not return all children of the directory", + 3, fileStatuses.length); + } + + private KeyInfo getKey(Path keyPath, boolean isDirectory) + throws IOException, OzoneException { + String key = o3fs.pathToKey(keyPath); + if (isDirectory) { + key = key + "/"; + } + KeyArgs parentKeyArgs = new KeyArgs(volumeName, bucketName, key, + userArgs); + return storageHandler.getKeyInfo(parentKeyArgs); + } + + private void assertKeyNotFoundException(IOException ex) { + GenericTestUtils.assertExceptionContains("KEY_NOT_FOUND", ex); + } }