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 Wed, 27 May 2009 22:51:45 GMT

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

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

Tsz Wo said:

bq. For internally methods like getPathComponents(..), the parameters are often not validated
for performance reason

While I appreciate the importance of performance, I think in all but the very tightest loops,
constant-time (O(1)) parameter validation is worth it. NN metadata ops are almost never the
bottleneck of a cluster, and I can't imagine a case where an extra memory access or two would
comprise a measurable portion of request latency. Given this, I think it's far more important
to focus on stability, reasonable error messages, and preventing regressions due to API misuse
than it is to worry about extra O(1) method calls.

I do agree that parameter validation should be happening at the outer layers of the API, but
since I somehow managed to get the NPE at this point, I figured this API was somehow publicly
accessible. See below.

Raghu said:

bq. Regd throwing a runtime exception, is it really different from NPE? Wouldn't any runtime
exception in NN indicate a bug in NN?

Yes. For me, the difference in IAE vs NPE is that it's explicitly "intended" and offers some
helpful explanation to the developer about what went wrong in their code. An NPE on the other
hand could be an unknown internal bug, or a misuse of API, or any number of other things.

bq. Can a user commad/usercode actually trigger this NPE? If yes, then it probably should
be regular error (IOException etc) rather than a runtime exception.

You can trigger this NPE if you have a reference to the NameNode (through ClientProtocol).
It looks to me like you could even trigger it by DFSClient.exists or DFSClient.getFileInfo,
though I'd have to write a test case. If people think this error should be caught in either
NameNode, FSNamesystem, or FSDirectory, then I agree - just let me know which place makes
the most sense and I'll move the code there and write an appropriate unit test.

bq. How did you notice this NPE?

I triggered it by calling namenode.getFileInfo() on a relative path from within my thrift
contrib code (HADOOP-4707). This was an error in my code, since I was misusing the API, but
it took me some serious digging to discover that the lack of initial '/' is what caused it.
If the error had been the IAE instead I would have found my error much quicker.

> 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