[
https://issues.apache.org/jira/browse/HDFS-13451?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16441106#comment-16441106
]
Daryn Sharp commented on HDFS-13451:
------------------------------------
I disagree with removing the possibility of NPEs by simply ignoring invalid
states. It will only further mask bugs. It will introduce more insanely
difficult to root cause bugs that would have never happened if the code blew up
early due to the invalid state that should be impossible.
The UGI auth method _cannot_ be PROXY w/o having a real user. The UGI is in an
illegal state that cannot be ignored.
When {{BlockInfo#getDatanode(int)}} returns null it's semantically equivalent
to ArrayIndexOutOfBoundsException – which it arguably should be. Treating this
condition as "ok" and skippable only masks bugs. The existing callers are
being bad.
The offline image change appears to simply ignore what is effectively a
corrupted image format.
> Fix Some Potential NPE
> ----------------------
>
> Key: HDFS-13451
> URL: https://issues.apache.org/jira/browse/HDFS-13451
> Project: Hadoop HDFS
> Issue Type: Bug
> Affects Versions: 3.0.0-beta1
> Reporter: lujie
> Priority: Major
> Attachments: HDFS-13451_1.patch
>
>
> We have developed a static analysis tool
> [NPEDetector|https://github.com/lujiefsi/NPEDetector] to find some potential
> NPE. Our analysis shows that some callees may return null in corner case(e.g.
> node crash , IO exception), some of their callers have _!=null_ check but
> some do not have. In this issue we post a patch which can add !=null based
> on existed !=null check. For example:
> callee BlockInfo#getDatanode may return null:
> {code:java}
> public DatanodeDescriptor getDatanode(int index) {
> DatanodeStorageInfo storage = getStorageInfo(index);
> return storage == null ? null : storage.getDatanodeDescriptor();
> }
> {code}
> it has 4 callers, 3 of them have !=null checker, like in
> CacheReplicationMonitor#addNewPendingCached:
> {code:java}
> DatanodeDescriptor datanode = blockInfo.getDatanode(i);
> if (datanode == null) {
> continue;
> }
> {code}
> but in caller NamenodeFsck#blockIdCK have no !null checker, we add checker
> just like CacheReplicationMonitor#addNewPendingCached
> {code:java}
> DatanodeDescriptor dn = blockInfo.getDatanode(idx);
> if (dn == null) {
> continue;
> }
> {code}
> But due to we are not very familiar with HDFS, hope some expert can review
> it.
> Thanks!!!!
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
