[Hadoop-common-dev] [jira] Created: (HADOOP-4951) Lease monitor does not own the LeaseManager lock in changing leases.

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tsz Wo (Nicholas), SZE updated HADOOP-4951:
-------------------------------------------

Attachment: 4951_20081229.patch

4951_20081229.patch: acquire the LeaseManager lock but not the Monitor lock

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tsz Wo (Nicholas), SZE updated HADOOP-4951:
-------------------------------------------

Attachment: 4951_20081229b.patch

4951_20081229b.patch: acquire the lock in checkLeases()

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

Raghu Angadi commented on HADOOP-4951:
--------------------------------------

Fix looks fine. I think it is better to properly indent the code even if it means a bigger patch.

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

Raghu Angadi commented on HADOOP-4951:
--------------------------------------

or you could move checkLeases() to LeaseMonitor class (where it actually belongs).

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tsz Wo (Nicholas), SZE updated HADOOP-4951:
-------------------------------------------

Attachment: 4951_20081229c.patch
You are right. I should move it out.

4951_20081229c.patch: moved checkLeases() and synchronized toString().

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

Raghu Angadi commented on HADOOP-4951:
--------------------------------------

+1. Looks good.

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tsz Wo (Nicholas), SZE updated HADOOP-4951:
-------------------------------------------

Priority: Blocker (was: Major)
Hadoop Flags: [Reviewed]

{noformat}
[exec] -1 overall.
[exec]
[exec] +1 @author. The patch does not contain any @author tags.
[exec]
[exec] -1 tests included. The patch doesn't appear to include any new or modified tests.
[exec] Please justify why no tests are needed for this patch.
[exec]
[exec] +1 javadoc. The javadoc tool did not generate any warning messages.
[exec]
[exec] +1 javac. The applied patch does not increase the total number of javac compiler warnings.
[exec]
[exec] +1 findbugs. The patch does not introduce any new Findbugs warnings.
[exec]
[exec] +1 Eclipse classpath. The patch retains Eclipse classpath integrity.
{noformat}

No new tests added since I don't know how to create a meaningful test for this.

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tsz Wo (Nicholas), SZE updated HADOOP-4951:
-------------------------------------------

Attachment: 4951_20081229c_0.18.patch

4951_20081229c_0.18.patch: for 0.18

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tsz Wo (Nicholas), SZE updated HADOOP-4951:
-------------------------------------------

Attachment: 4951_20081229c_0.19.patch

4951_20081229c_0.19.patch: for 0.19

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

Tsz Wo (Nicholas), SZE commented on HADOOP-4951:
------------------------------------------------

Tested locally. Only TestMapReduceLocal failed but it is not related to this. See HADOOP-4907.

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Tsz Wo (Nicholas), SZE resolved HADOOP-4951.
--------------------------------------------

Resolution: Fixed
Fix Version/s: 0.18.3

I just committed this.

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

Koji Noguchi commented on HADOOP-4951:
--------------------------------------

Nicholas, could you attach the stack trace you observed?
And what is the effect of this bug?

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

Konstantin Shvachko commented on HADOOP-4951:
---------------------------------------------

This is the exception and the stack trace.
{code}
Exception in thread "org.apache.hadoop.dfs.LeaseManager$[email protected]"
java.util.ConcurrentModificationException
at java.util.TreeMap$PrivateEntryIterator.nextEntry(TreeMap.java:1100)
at java.util.TreeMap$KeyIterator.next(TreeMap.java:1154)
at org.apache.hadoop.dfs.LeaseManager$Monitor.checkLeases(LeaseManager.java:370)
at org.apache.hadoop.dfs.LeaseManager$Monitor.run(LeaseManager.java:346)
at java.lang.Thread.run(Thread.java:619)
{code}
Another problem with this issue is that the name-node was running without lease monitor as if nothing happened. As a result, a lot of abandoned files have not been garbage collected.
This patch fixes the {{ConcurrentModificationException}} but does not fix the problem of NN running without the lease monitor. I'll file another jira for that.

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

Tsz Wo (Nicholas), SZE commented on HADOOP-4951:
------------------------------------------------

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Konstantin Shvachko reopened HADOOP-4951:
-----------------------------------------


This did not fix the ConcurrentModificationException. Although it corrected the synchronization problem.
The exception now looks as
{code}
Exception in thread "org.apache.hadoop.dfs.LeaseManager$[email protected]" java.util.ConcurrentModificationException
at java.util.TreeMap$PrivateEntryIterator.nextEntry(TreeMap.java:1100)
at java.util.TreeMap$KeyIterator.next(TreeMap.java:1154)
at org.apache.hadoop.dfs.LeaseManager.checkLeases(LeaseManager.java:371)
at org.apache.hadoop.dfs.LeaseManager.access$800(LeaseManager.java:51)
at org.apache.hadoop.dfs.LeaseManager$Monitor.run(LeaseManager.java:346)
at java.lang.Thread.run(Thread.java:619)
{code}

Looks like the problem is that {{internalReleaseLease()}} finalizes empty files, which removes these file path names from the lease. So this modifies the Collection of file names which we are iterating on.
An additional patch need to be supplied.

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Konstantin Shvachko updated HADOOP-4951:
----------------------------------------

Attachment: CMEinLeaseRecovery-0-18.patch

[ https://issues.apache.org/jira/browse/HADOOP-4951?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Konstantin Shvachko resolved HADOOP-4951.
-----------------------------------------

Resolution: Fixed

It will be easier to track changes if I open a new issue. Closing this one.