[
https://issues.apache.org/jira/browse/HDDS-1301?focusedWorklogId=227671&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-227671
]
ASF GitHub Bot logged work on HDDS-1301:
----------------------------------------
Author: ASF GitHub Bot
Created on: 15/Apr/19 13:11
Start Date: 15/Apr/19 13:11
Worklog Time Spent: 10m
Work Description: mukul1987 commented on pull request #718: HDDS-1301.
Optimize recursive ozone filesystem apis
URL: https://github.com/apache/hadoop/pull/718#discussion_r275343085
##########
File path:
hadoop-ozone/ozonefs/src/main/java/org/apache/hadoop/fs/ozone/BasicOzoneFileSystem.java
##########
@@ -319,337 +260,45 @@ boolean processKey(String key) throws IOException {
public boolean rename(Path src, Path dst) throws IOException {
incrementCounter(Statistic.INVOCATION_RENAME);
statistics.incrementWriteOps(1);
- if (src.equals(dst)) {
- return true;
- }
-
LOG.trace("rename() from:{} to:{}", src, dst);
- if (src.isRoot()) {
- // Cannot rename root of file system
- LOG.trace("Cannot rename the root of a filesystem");
- return false;
- }
-
- // Cannot rename a directory to its own subdirectory
- Path dstParent = dst.getParent();
- while (dstParent != null && !src.equals(dstParent)) {
- dstParent = dstParent.getParent();
- }
- Preconditions.checkArgument(dstParent == null,
- "Cannot rename a directory to its own subdirectory");
- // Check if the source exists
- FileStatus srcStatus;
- try {
- srcStatus = getFileStatus(src);
- } catch (FileNotFoundException fnfe) {
- // source doesn't exist, return
- return false;
- }
-
- // Check if the destination exists
- FileStatus dstStatus;
- try {
- dstStatus = getFileStatus(dst);
- } catch (FileNotFoundException fnde) {
- dstStatus = null;
- }
-
- if (dstStatus == null) {
- // If dst doesn't exist, check whether dst parent dir exists or not
- // if the parent exists, the source can still be renamed to dst path
- dstStatus = getFileStatus(dst.getParent());
- if (!dstStatus.isDirectory()) {
- throw new IOException(String.format(
- "Failed to rename %s to %s, %s is a file", src, dst,
- dst.getParent()));
- }
- } else {
- // if dst exists and source and destination are same,
- // check both the src and dst are of same type
- if (srcStatus.getPath().equals(dstStatus.getPath())) {
- return !srcStatus.isDirectory();
- } else if (dstStatus.isDirectory()) {
- // If dst is a directory, rename source as subpath of it.
- // for example rename /source to /dst will lead to /dst/source
- dst = new Path(dst, src.getName());
- FileStatus[] statuses;
- try {
- statuses = listStatus(dst);
- } catch (FileNotFoundException fnde) {
- statuses = null;
- }
-
- if (statuses != null && statuses.length > 0) {
- // If dst exists and not a directory not empty
- throw new FileAlreadyExistsException(String.format(
- "Failed to rename %s to %s, file already exists or not empty!",
- src, dst));
- }
- } else {
- // If dst is not a directory
- throw new FileAlreadyExistsException(String.format(
- "Failed to rename %s to %s, file already exists!", src, dst));
- }
- }
-
- if (srcStatus.isDirectory()) {
- if (dst.toString().startsWith(src.toString() + OZONE_URI_DELIMITER)) {
- LOG.trace("Cannot rename a directory to a subdirectory of self");
- return false;
- }
- }
- RenameIterator iterator = new RenameIterator(src, dst);
- return iterator.iterate();
- }
-
- private class DeleteIterator extends OzoneListingIterator {
- private boolean recursive;
-
- DeleteIterator(Path f, boolean recursive)
- throws IOException {
- super(f);
- this.recursive = recursive;
- if (getStatus().isDirectory()
- && !this.recursive
- && listStatus(f).length != 0) {
- throw new PathIsNotEmptyDirectoryException(f.toString());
- }
- }
-
- @Override
- boolean processKey(String key) throws IOException {
- if (key.equals("")) {
- LOG.trace("Skipping deleting root directory");
- return true;
- } else {
- LOG.trace("deleting key:" + key);
- boolean succeed = adapter.deleteObject(key);
- // if recursive delete is requested ignore the return value of
- // deleteObject and issue deletes for other keys.
- return recursive || succeed;
- }
- }
- }
-
- /**
- * 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);
- return iterator.iterate();
- } catch (FileNotFoundException e) {
- LOG.debug("Couldn't delete {} - does not exist", f);
- return false;
- }
+ return adapter.rename(pathToKey(src), pathToKey(dst));
}
@Override
public boolean delete(Path f, boolean recursive) throws IOException {
incrementCounter(Statistic.INVOCATION_DELETE);
statistics.incrementWriteOps(1);
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 = adapter.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);
- adapter.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;
- }
+ return adapter.delete(pathToKey(f), recursive);
}
- private class ListStatusIterator extends OzoneListingIterator {
- // _fileStatuses_ maintains a list of file(s) which is either the input
- // path itself or a child of the input directory path.
- private List<FileStatus> fileStatuses = new ArrayList<>(LISTING_PAGE_SIZE);
- // _subDirStatuses_ maintains a list of sub-dirs of the input directory
- // path.
- private Map<Path, FileStatus> subDirStatuses =
- new HashMap<>(LISTING_PAGE_SIZE);
- private Path f; // the input path
-
- ListStatusIterator(Path f) throws IOException {
- super(f);
- this.f = f;
- }
+ @Override
+ public FileStatus[] listStatus(Path f) throws IOException {
+ incrementCounter(Statistic.INVOCATION_LIST_STATUS);
+ statistics.incrementReadOps(1);
+ LOG.trace("listStatus() path:{}", f);
+ int numEntries = LISTING_PAGE_SIZE;
+ LinkedList<OzoneFileStatus> statuses = new LinkedList<>();
+ List<OzoneFileStatus> tmpStatus;
+ String startKey = "";
- /**
- * 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
- */
- @Override
- 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 {
- addFileStatus(keyPath);
- return true;
- }
- }
- // 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))) {
- // This key is an immediate child. Can be file or directory
- if (key.endsWith(OZONE_URI_DELIMITER)) {
- // Key is a directory
- addSubDirStatus(keyPath);
+ do {
+ tmpStatus = adapter.listStatus(pathToKey(f), false, startKey,
numEntries);
+ if (!tmpStatus.isEmpty()) {
+ if (startKey.isEmpty()) {
+ statuses.addAll(tmpStatus);
} 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());
- if (immediateChildPath != null) {
- 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 IOException {
- // Check if subdir path is already included in statuses.
- if (!subDirStatuses.containsKey(dirPath)) {
- subDirStatuses.put(dirPath, getFileStatus(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.
- */
- Path getImmediateChildPath(Path keyPath) {
- Path path = keyPath;
- Path parent = path.getParent();
- while (parent != null) {
- if (pathToKey(parent).equals(pathToKey(f))) {
- return path;
+ statuses.addAll(tmpStatus.subList(1, tmpStatus.size()));
}
- path = parent;
- parent = path.getParent();
+ startKey = pathToKey(statuses.getLast().getPath());
}
- return null;
- }
+ } while (tmpStatus.size() == numEntries);
Review comment:
Should this be statuses in place of tmpStatus ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 227671)
Time Spent: 1.5h (was: 1h 20m)
> Optimize recursive ozone filesystem apis
> ----------------------------------------
>
> Key: HDDS-1301
> URL: https://issues.apache.org/jira/browse/HDDS-1301
> Project: Hadoop Distributed Data Store
> Issue Type: Sub-task
> Reporter: Lokesh Jain
> Assignee: Lokesh Jain
> Priority: Major
> Labels: pull-request-available
> Attachments: HDDS-1301.001.patch
>
> Time Spent: 1.5h
> Remaining Estimate: 0h
>
> This Jira aims to optimise recursive apis in ozone file system. These are the
> apis which have a recursive flag which requires an operation to be performed
> on all the children of the directory. The Jira would add support for
> recursive apis in Ozone manager in order to reduce the number of rpc calls to
> Ozone Manager. Also currently these operations are not atomic. This Jira
> would make all the operations in ozone filesystem atomic.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
