[
https://issues.apache.org/jira/browse/HDFS-4882?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14211550#comment-14211550
]
Colin Patrick McCabe commented on HDFS-4882:
--------------------------------------------
The patch looks good.
bq. Here's a patch which uses SortedSet.tailSet. However I still like the
earlier patch more (because its a genuine case of two threads accessing the
same data-structure). With tailSet we are just trying to build our own
synchronization mechanism (which is likely more inefficient than the
ConcurrentinternalReleaseLease) .
I have to admit that I find the locking to be confusing in {{LeaseManager}}.
It seems like some methods which access the sets are synchronized, and some are
not. And we are exposing the sets to outside code, which accesses them without
synchronization. It's very inconsistent and we should file a follow-up JIRA to
look into this. I don't think there is necessarily a bug there, though... I
think it's possible that the access is actually gated on the FSN lock. If
that's true, we should document it and remove the synchronized blocks in
{{LeaseManager}} (since in that case they would not be doing anything useful).
Concurrent sets are not a magic bullet since we still may have to deal with
issues like Lease objects being deactivated while another caller holds on to
them, etc. If we dig too far into locking, this patch will get a lot bigger
and a lot messier. I think that we should keep this patch small and focused on
the problem.
In your patch is that you are requiring the FSN lock to be held in
{{LeaseManager#getNumUnderConstructionBlocks}}, but you don't document that
anywhere. Can you please add that to a JavaDoc or other comment for this
method?
{code}
+ Lease leaseToCheck = null;
+ try {
+ leaseToCheck = sortedLeases.first();
+ } catch(NoSuchElementException e) {}
{code}
You can replace this with {{leaseToCheck = sortedLeases.pollFirst();}}
{code}
+ try {
+ leaseToCheck = sortedLeases.tailSet(leaseToCheck, false).first();
+ } catch(NoSuchElementException e) {
+ leaseToCheck = null;
}
{code}
You don't need this. Just use {{leaseToCheck =
sortedLeases.higher(leaseToCheck)}}.
bq. I'd also request for this to make it into 2.6.0 because of this issue's
severity.
I'm fine with the second version going into 2.6. [~acmurthy], others, what do
you think?
> Namenode LeaseManager checkLeases() runs into infinite loop
> -----------------------------------------------------------
>
> Key: HDFS-4882
> URL: https://issues.apache.org/jira/browse/HDFS-4882
> Project: Hadoop HDFS
> Issue Type: Bug
> Components: hdfs-client, namenode
> Affects Versions: 2.0.0-alpha, 2.5.1
> Reporter: Zesheng Wu
> Assignee: Ravi Prakash
> Priority: Critical
> Attachments: 4882.1.patch, 4882.patch, 4882.patch, HDFS-4882.1.patch,
> HDFS-4882.2.patch, HDFS-4882.patch
>
>
> Scenario:
> 1. cluster with 4 DNs
> 2. the size of the file to be written is a little more than one block
> 3. write the first block to 3 DNs, DN1->DN2->DN3
> 4. all the data packets of first block is successfully acked and the client
> sets the pipeline stage to PIPELINE_CLOSE, but the last packet isn't sent out
> 5. DN2 and DN3 are down
> 6. client recovers the pipeline, but no new DN is added to the pipeline
> because of the current pipeline stage is PIPELINE_CLOSE
> 7. client continuously writes the last block, and try to close the file after
> written all the data
> 8. NN finds that the penultimate block doesn't has enough replica(our
> dfs.namenode.replication.min=2), and the client's close runs into indefinite
> loop(HDFS-2936), and at the same time, NN makes the last block's state to
> COMPLETE
> 9. shutdown the client
> 10. the file's lease exceeds hard limit
> 11. LeaseManager realizes that and begin to do lease recovery by call
> fsnamesystem.internalReleaseLease()
> 12. but the last block's state is COMPLETE, and this triggers lease manager's
> infinite loop and prints massive logs like this:
> {noformat}
> 2013-06-05,17:42:25,695 INFO
> org.apache.hadoop.hdfs.server.namenode.LeaseManager: Lease [Lease. Holder:
> DFSClient_NONMAPREDUCE_-1252656407_1, pendingcreates: 1] has expired hard
> limit
> 2013-06-05,17:42:25,695 INFO
> org.apache.hadoop.hdfs.server.namenode.FSNamesystem: Recovering lease=[Lease.
> Holder: DFSClient_NONMAPREDUCE_-1252656407_1, pendingcreates: 1], src=
> /user/h_wuzesheng/test.dat
> 2013-06-05,17:42:25,695 WARN org.apache.hadoop.hdfs.StateChange: DIR*
> NameSystem.internalReleaseLease: File = /user/h_wuzesheng/test.dat, block
> blk_-7028017402720175688_1202597,
> lastBLockState=COMPLETE
> 2013-06-05,17:42:25,695 INFO
> org.apache.hadoop.hdfs.server.namenode.LeaseManager: Started block recovery
> for file /user/h_wuzesheng/test.dat lease [Lease. Holder: DFSClient_NONM
> APREDUCE_-1252656407_1, pendingcreates: 1]
> {noformat}
> (the 3rd line log is a debug log added by us)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
