[ 
https://issues.apache.org/jira/browse/HDFS-5330?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13795403#comment-13795403
 ] 

Jing Zhao commented on HDFS-5330:
---------------------------------

The patch looks good to me. Some minors:

# Entry3#getFileId, Entry3#getCookie, and DirListPlus3#getEof may not need to 
be declared as public. Please add @VisibleForTesting annotation to 
Entry3#getEntries, EntryPlus3#getEntries, and EntryPlus3#getName.
# The following code appears more than once in RpcProgrammNfs3, thus can be 
wrapped in a separate method:
{code}
+        IOException e2 = e1.unwrapRemoteException();
+        if (!(e2 instanceof DirectoryListingStartAfterNotFoundException)) {
+          return new READDIRPLUS3Response(Nfs3Status.NFS3ERR_IO);
+        }
+        // This happens when startAfter was just deleted
+        LOG.info("Cookie cound't be found: " + new String(startAfter)
+            + ", do listing from beginning");
+        dlisting = dfsClient
+            .listPaths(dirFileIdPath, HdfsFileStatus.EMPTY_NAME);
{code}
Or we can have a separate private listPaths method.
# The new unit test needs some javadoc.
# It may be better to split testReaddirBasic into two separate unit tests: 
testReaddir and testReaddirPlus. 

> fix readdir and readdirplus for large directories
> -------------------------------------------------
>
>                 Key: HDFS-5330
>                 URL: https://issues.apache.org/jira/browse/HDFS-5330
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: nfs
>            Reporter: Brandon Li
>            Assignee: Brandon Li
>         Attachments: HDFS-5330.001.patch, HDFS-5330.002.patch, 
> HDFS-5330.003.patch
>
>
> These two calls need to use cookies to do multiple round trips to namenode to 
> get the complete list of the dirents. Currently implementation passes an 
> inode path as "startAfter" for listPath(), however, namenode doesn't resolve 
> startAfter as an inode path. Better use file name as "startAfter".



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to