hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "HBase Review Board (JIRA)" <j...@apache.org>
Subject [jira] Commented: (HBASE-2915) Deadlock between HRegion.ICV and HRegion.close
Date Fri, 20 Aug 2010 23:34:16 GMT

    [ https://issues.apache.org/jira/browse/HBASE-2915?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900916#action_12900916
] 

HBase Review Board commented on HBASE-2915:
-------------------------------------------

Message from: "Jean-Daniel Cryans" <jdcryans@apache.org>


bq.  On 2010-08-20 15:57:34, stack wrote:
bq.  > /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 507
bq.  > <http://review.cloudera.org/r/691/diff/1/?file=7612#file7612line507>
bq.  >
bq.  >     I suppose this order is ok if the first thing we do on entrance to HRegion is
get the read lock before check of closing.

So I just redid that part. setClosing is first taken so that when the client threads arrive
they can fast fail by looking at closing.get before trying to get the readLock.


bq.  On 2010-08-20 15:57:34, stack wrote:
bq.  > /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 712
bq.  > <http://review.cloudera.org/r/691/diff/1/?file=7612#file7612line712>
bq.  >
bq.  >     Seems like you could use your opentransaction/closetransaction methods here
and in flush too to be consistent?

Yeah the issue with compact and flush is that the callers don't expect to see NSRE, the want
null values.


bq.  On 2010-08-20 15:57:34, stack wrote:
bq.  > /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 1115
bq.  > <http://review.cloudera.org/r/691/diff/1/?file=7612#file7612line1115>
bq.  >
bq.  >     Aren't these lines unnecessary?  openRegionTransaction does it?

Good catch.


bq.  On 2010-08-20 15:57:34, stack wrote:
bq.  > /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 3142
bq.  > <http://review.cloudera.org/r/691/diff/1/?file=7612#file7612line3142>
bq.  >
bq.  >     So, this javadoc is good but do you think we need some more doc?  Does there
need to be more detail on new locking regime? Maybe there is no more to be said that what
is here in this paragraph.  You've done all the work unravelling our lock mess.  With time
your nice unravelling will rot unless its clear what the pattern is.    I'm just trying to
think of ways of preventing that happening.

Yeah I'll included some more javadoc, maybe with code examples?


- Jean-Daniel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://review.cloudera.org/r/691/#review974
-----------------------------------------------------------





> Deadlock between HRegion.ICV and HRegion.close
> ----------------------------------------------
>
>                 Key: HBASE-2915
>                 URL: https://issues.apache.org/jira/browse/HBASE-2915
>             Project: HBase
>          Issue Type: Bug
>            Reporter: Jean-Daniel Cryans
>            Assignee: Jean-Daniel Cryans
>            Priority: Blocker
>             Fix For: 0.90.0
>
>
> HRegion.ICV gets a row lock then gets a newScanner lock.
> HRegion.close gets a newScanner lock, slitCloseLock and finally waits for all row locks
to finish.
> If the ICV got the row lock and then close got the newScannerLock, both end up waiting
on the other. This was introduced when Get became a Scan.
> Stack thinks we can get rid of the newScannerLock in close since we setClosing to true.

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


Mime
View raw message