[
https://issues.apache.org/jira/browse/HDFS-5224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13792086#comment-13792086
]
Andrew Wang commented on HDFS-5224:
-----------------------------------
Hey Chris,
Sorry about the delay on getting back to you. I played with your patch a bit
and eventually arrived at the same conclusion as you. Having {{Path}}s on the
namenode is unfortunate, but it's a lesser of two evils situation.
Some review comments:
* I think we should make it so the PCD path is qualified at creation time, else
you might create a PCD with a relative path, change working directories, and
the PCD now refers to a different path. I think the easiest way of doing this
is a factory method in {{DFS}}.
* CacheAdmin#run: we already check that {{path}} is not null, so I don't think
we need that ternary. The PCDir constructor already does a null check.
* We could do a little cleanup of validation in PCDir too, while we're there. I
think we should move all the checks to {{PCD#validate}}, and call it in
{{Builder#build}}. It'd also be nice to do a little cleanup to have all the
checks in one place, e.g. {{#validate}}.
* I think we still want {{EmptyPathError}} since someone could manually mangle
the protobuf to give us an empty path. It's nicer to throw the named exception
than some Path creation error in the PB translator.
Overall looks really good though, thanks for working on this.
> Refactor PathBasedCache* methods to use a Path rather than a String
> -------------------------------------------------------------------
>
> Key: HDFS-5224
> URL: https://issues.apache.org/jira/browse/HDFS-5224
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: datanode, namenode
> Affects Versions: HDFS-4949
> Reporter: Andrew Wang
> Assignee: Chris Nauroth
> Attachments: HDFS-5224.1.patch, HDFS-5224.2.patch
>
>
> As discussed in HDFS-5213, we should refactor PathBasedCacheDirective and
> related methods in DistributedFileSystem to use a Path to represent paths to
> cache, rather than a String.
--
This message was sent by Atlassian JIRA
(v6.1#6144)
