hadoop-hdfs-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Andrew Wang (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HDFS-5224) Refactor PathBasedCache* methods to use a Path rather than a String
Date Thu, 10 Oct 2013 23:08:42 GMT

    [ 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)

Mime
View raw message