[ 
https://issues.apache.org/jira/browse/HADOOP-6678?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12861645#action_12861645
 ] 

Eli Collins commented on HADOOP-6678:
-------------------------------------

Hey Hairong,  no worries - looking at the implementation of FileContext#exists 
I think it can be simplified:

{code}
  public boolean exists(final Path f) throws AccessControlException,  
IOException {
    try {
      return getFileStatus(f) != null;            
    } catch (FileNotFoundException e) {
      return false;
    } catch (UnsupportedFileSystemException e) {
      return false;
    } catch (UnresolvedLinkException e) {
      return false;
    }
  }
{code}

# AFS#getFileStatus should never return null, eg see Hdfs#getFileStatus. Per 
the spec if should throw a FileNotFoundException if f does not exist.
# Seems like the client should have to deal with UnsupportedFileSystemException 
rather than have FC#exists swallow it
# UnresolvedLinkException will never be thrown (see HADOOP-6727)

Therefore I think we can write FC#exists as

{code}
  public boolean exists(final Path f) throws AccessControlException, 
UnsupportedFileSystemException,  IOException {
    try {
      FileStatus fs =  getFileStatus(f);
      assert fs != null;
      return true;
    } catch (FileNotFoundException e) {
      return false;
    }
  }
{code}

Since this is still not a one-liner putting it in FileContext#util seems 
reasonable to me.  I'll send out new patches.



> Remove FileContext#isFile, isDirectory and exists
> -------------------------------------------------
>
>                 Key: HADOOP-6678
>                 URL: https://issues.apache.org/jira/browse/HADOOP-6678
>             Project: Hadoop Common
>          Issue Type: Improvement
>          Components: fs
>            Reporter: Hairong Kuang
>            Assignee: Eli Collins
>             Fix For: 0.21.0, 0.22.0
>
>         Attachments: hadoop-6678-1.patch
>
>
> # Add a method  Iterator<FileStatus> listStatus(Path), which allows HDFS 
> client not to have the whole listing in the memory, benefit more from the 
> iterative listing added in HDFS-985. Move the current FileStatus[] 
> listStatus(Path) to be a utility method.
> # Remove methods isFile(Path), isDirectory(Path), and exists.
> All these methods are implemented by calling getFileStatus(Path).But most 
> users are not aware of this. They would write code as below: 
> {code}
>   FileContext fc = ..;
>   if (fc.exists(path)) {
>     if (fc.isFile(path)) {
>      ...
>     } else {
>     ...
>     }
>   }
> {code}
> The above code adds unnecessary getFileInfo RPC to NameNode. In our 
> production clusters, we often see that the number of getFileStatus calls is 
> multiple times of the open calls. If we remove isFile, isDirectory, and 
> exists from FileContext, users have to explicitly call getFileStatus first, 
> it is more likely that they will write more efficient code as follow:
> {code}
>   FileContext fc = ...;
>   FileStatus fstatus = fc.getFileStatus(path);
>   if (fstatus.isFile() {
>     ...
>   } else {
>     ...
>   }
> {code}

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to