[
https://issues.apache.org/jira/browse/HADOOP-5192?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12672399#action_12672399
]
Raghu Angadi commented on HADOOP-5192:
--------------------------------------
> Adding the lock 'unfinalizeBlockIfInvalid() is just being super cautious.
umm.. I would call that 'good practice'. between :
- {{'lock(p); bool cond = condition(); unlock(p); if (cond) {lock (p);
destroy(); unlock(p) ;}'}} and
- {{'lock (p); if (condition()) destroy(); unlock(p);'}}
to me the former looks instinctively error prone (in many cases would really be
a bug).. and it would be hard track in future since others wouldn't always look
in this corner. There is not even comment why the multiple locking is correct.
In this case you are mostly right.. only based on global knowledge of DataNode.
Any one reading the code just around that, wouldn't easily see that.
w.r.t deletion, it is better to err on not deleting than to err on deletion...
if I am not mistaken, that is essence of the problem with HADOOP-4702, which
this jira proposes to fix.
Anyway, it is _not_ a -1 from me.
> Block reciever should not remove a finalized block when block replication
> fails
> -------------------------------------------------------------------------------
>
> Key: HADOOP-5192
> URL: https://issues.apache.org/jira/browse/HADOOP-5192
> Project: Hadoop Core
> Issue Type: Bug
> Components: dfs
> Affects Versions: 0.18.3
> Reporter: Hairong Kuang
> Assignee: Hairong Kuang
> Fix For: 0.18.4
>
> Attachments: blockCleanup.patch, blockRemove.patch
>
>
> HADOOP-4702 makes block receivers to remove the received block in case of a
> block replication failure. But the block should not be removed if the cause
> of the failure is that the block to be received already exists. The key is
> that a block receiver should allow to remove only partial blocks in case of
> block replication failures.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
