hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Todd Lipcon (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HADOOP-5700) INode.getPathComponents throws NPE when given a non-absolute path
Date Fri, 29 May 2009 06:24:45 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-5700?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12714324#action_12714324
] 

Todd Lipcon commented on HADOOP-5700:
-------------------------------------

bq. The patch introduces a redundant check, which degrades the namenode performance. All production
systems will suffer this. The performance impact is still unknown since no performance measurement
is even done.

I strongly disagree on the performance issue. I'd go into detail about why this cannot possibly
cause a performance problem, but clearly this is an idealogical debate rather than a practical
one for this case. In short: the data accessed is about to be split, meaning that it can't
be an extra cache miss, and the splitting and allocation will be several of orders of magnitude
slower than a constant-time single-byte comparison.

bq.  The patch uses "/" but not the Path.SEPARATOR constant. It is incorrect, strictly speaking.

Good point. That should be fixed.

bq. The patch does not fix any bug: it changes NPE to IAE. Both of them are uncaught runtime
exceptions

I think of IAEs as stricly more informative than NPEs for the developer who has committed
the error. Again, no sense continuing that discussion.

bq. As mentioned by Todd's earlier comment, parameter validation should be happening at the
outer layers of the API but not internal methods. However, the patch does parameter validation
in INode.getPathComponents(..) which is an internal method.

Also should be fixed.

I'll prepare a patch to address the above things.

On a side note, what is the policy for dealing with updated patches after a JIRA has already
been committed? As far as I knew, discussion of a better/new patch is supposed to go on in
a new JIRA.

> INode.getPathComponents throws NPE when given a non-absolute path
> -----------------------------------------------------------------
>
>                 Key: HADOOP-5700
>                 URL: https://issues.apache.org/jira/browse/HADOOP-5700
>             Project: Hadoop Core
>          Issue Type: Bug
>          Components: dfs
>            Reporter: Todd Lipcon
>            Assignee: Todd Lipcon
>            Priority: Minor
>             Fix For: 0.21.0
>
>         Attachments: HADOOP-5700.txt
>
>
> If you pass a path that doesn't start with '/' to INode.getPathComponents, it throws
a NullPointerException. Instead it should throw IllegalArgumentException to make it clear
that absolute paths are required in this code.
> The attached patch fixes this, clarifies, the javadoc, and adds a test case.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message