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

Andrew Wang commented on HDFS-5213:
-----------------------------------

Hey Colin,

Naming:
* Can we rename {{PathBasedCacheDirectiveWithId}} to something more general, 
like {{PathBasedCacheDescriptor}}? Reminiscent of file descriptor.
* If {{PathBasedCacheDirectiveWithId}} is a client-side class, the javadoc 
shouldn't be saying that it's an "entry in the NameNode's PathBasedCache". That 
sounds like a {{PathBasedCacheEntry}}.
* The {{PathBasedCacheEntry}} javadoc also needs to be improved
* {{getDirective()}} should be named {{getDirectiveWithId()}} (or 
{{getDescriptor()}} if you use my suggestion).
* Exception text shouldn't talk about a {{PathBasedCache entry}} when it's 
supposed to be a private namenode class. There's also no {{PathBasedCache}} 
class, so confusing.

Nits/other:
* For implementing equals, how about comparing {{getClass()}} rather than 
casting? This means it won't take subclasses, which is normally a good thing 
(safe symmetric equals).
* I don't think any of this should be marked {{InterfaceAudience.Stable}} yet.
* extra {{mortbay.log}} import

Unrelated-but-related:
* I still want to see Paths, not Strings, for the DFS methods involving cache 
directives. It's non-standard and weird for the reasons I mentioned before, and 
DFS has error checking for being passed a URI for a different FileSystem.
                
> separate PathBasedCacheEntry and PathBasedCacheDirectiveWithId
> --------------------------------------------------------------
>
>                 Key: HDFS-5213
>                 URL: https://issues.apache.org/jira/browse/HDFS-5213
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>          Components: datanode, namenode
>            Reporter: Colin Patrick McCabe
>            Assignee: Colin Patrick McCabe
>         Attachments: HDFS-5213-caching.001.patch
>
>
> Since PathBasedCacheEntry is intended to be a private (implementation) class,
> return PathBasedCacheDirectiveWithId from all public APIs instead.  Some 
> other miscellaneous cleanups in the caching RPC stuff.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to