[jira] Commented: (HDFS-973) DataNode.setDataNode() considered dangerous

["Todd Lipcon (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/HDFS-973?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12833116#action_12833116 ] 

Todd Lipcon commented on HDFS-973:
----------------------------------

Steve: I agree with you in spirit. Have you looked at how much stuff uses this static reference and how difficult it would be to fix? Would be interesting to know.

bq. As part of the service lifecycle patch

I'd rather see a patch for this separate from the lifecycle work. That branch has been around for a year and a half now with little sign of nearing completion.

> DataNode.setDataNode() considered dangerous
> -------------------------------------------
>
>                 Key: HDFS-973
>                 URL: https://issues.apache.org/jira/browse/HDFS-973
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: data-node
>    Affects Versions: 0.22.0
>            Reporter: Steve Loughran
>            Priority: Minor
>
> I don't have any plans to address this, but it seems to me that having the DataNode save a reference to itself in its constructor by way of {{DataNode.setDataNode(this)}} is hazardous. 
> # The reference could be used before the constructor has finished, especially when subclasses are involved
> # Callers may assume the DN is actually live
> # If startup fails, the DN tries to shut down, but the reference hangs around. Dangerous as well as leaking a reference
> # The reference gets retained forever
> # It's a singleton that will get confused if >1 DN gets instantiated in-VM
> The likely way these problems will surface are in race conditions that are more likely the more cores you have on the machine -production rather than development. This is why it is dangerous.
> As part of the service lifecycle patch, I could have this reference only set when the service gets started, set it to null when stopped (and the reference==this). But really the singleton should be removed altogether, somehow. There are methods in DataNode, DataStorage, FSDataset and the namenode that do this, and they should somehow get a reference to any in-VM DN in a cleaner way. For example, servlets can have it set as servlet context.

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