[
https://issues.apache.org/jira/browse/HDFS-5158?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13763704#comment-13763704
]
Colin Patrick McCabe commented on HDFS-5158:
--------------------------------------------
bq. WATERMELON debug prints, extra import
Fixed
bq. Need to handle the case of null Strings being passed to
listPathCacheEntries (pool and path)... I think it's more natural to have null
be "match any path" or "match any pool", saves some overhead too.
OK.
bq. hasMore, isn't it better to keep it optional in the wire format, and set a
default in the PB translators?
Making it mandatory exposed a few bugs where I was forgetting to set it. I
would rather keep this boolean mandatory.
bq. DistributedFileSystem APIs take Paths, not Strings, for paths. Needs to be
resolved against the working directory too (see DFS#fixRelativePart) and then
stringified by getPathName.
I'm not sure that using a Path here adds any value. We know that the path here
is going to be on the DistributedFileSystem; we're not interested in any part
of the URI except the path part. Allowing people to pass full URIs just gives
them a chance to get it wrong-- never a good thing in an API. We'd also have
to create another class besides PathCacheDirective to represent what we were
adding, since PCD takes a string and not a Path. I guess we could convert
relative paths to aboslute using the current working directory. I'm not sure
if we should do that, or just accept absolute paths.
bq. Do we normalize paths for things like a trailing slash or double slashes?
Test case for this would be cool.
The server rejects paths that don't pass validation, so that checks for things
like trailing slashes and double slashes. I can add a test to our existing RPC
tests.
bq. Should document the pool permissions somewhere, the CacheManager class
javadoc is very lacking right now.
I will document them in the javadoc for CachePool#mode.
bq. I think it's also better to stick with execute being the "list pool"
permission, but could be convinced otherwise. I get that one of read or execute
is going to be meaningless.
On directories, the R permission allows you to list them, and the X permission
gives you the ability to access entries inside them. This is true both in
POSIX and in HDFS. We don't make a strong distinction between listing cache
directives and accessing their data ({{listPathCacheDirectives}} does both) so
I think it makes more sense to use R here.
bq. Should add some DistributedFileSystem tests too, no more manual PB mangling
required now with the shiny new APIs!
If I get the CLI tests working, it will test that too.
bq. Need InterfaceStability annotations too on new classes
ok
> add command-line support for manipulating cache directives
> ----------------------------------------------------------
>
> Key: HDFS-5158
> URL: https://issues.apache.org/jira/browse/HDFS-5158
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode, namenode
> Affects Versions: HDFS-4949
> Reporter: Colin Patrick McCabe
> Assignee: Colin Patrick McCabe
> Attachments: HDFS-5158-caching.003.patch
>
>
> We should add command-line support for creating, removing, and listing cache
> directives.
--
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
