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 Mon, 23 Aug 2010 17:02:16 GMT

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

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

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


bq.  On 2010-08-20 20:49:15, stack wrote:
bq.  > /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 716
bq.  > <http://review.cloudera.org/r/691/diff/2/?file=7680#file7680line716>
bq.  >
bq.  >     Any reason this does not follow the pattern used elsewhere?  You are not testing
closing before getting the read lock?

mmm didn't want to copy the same chunk of code over there, but yeah it will also give us a
fail-fast behavior which will speed up flushing.


bq.  On 2010-08-20 20:49:15, stack wrote:
bq.  > /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 785
bq.  > <http://review.cloudera.org/r/691/diff/2/?file=7680#file7680line785>
bq.  >
bq.  >     ditto

ditto


bq.  On 2010-08-20 20:49:15, stack wrote:
bq.  > /trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java, line 2960
bq.  > <http://review.cloudera.org/r/691/diff/2/?file=7680#file7680line2960>
bq.  >
bq.  >     Is this supposed to be inside the try?

Per that method's javadoc, no. The reason is that if it's all in a try block, and that closing
is true, then you won't hold a readLock and you can't do a isHeldByCurrent thread on that
lock. So I thought we could catch IllegalStateException in closeRegionOperation, but that
would be really ugly. This leaves us to the current situation where if an exception is thrown
when we are on this line:
     if (this.closed.get()) {
that it would leave the readLock locked by the thread, although in this case the region is
closing so we're getting rid of it anyways. Although, thinking about it, I should probably
do this instead:

    if (this.closed.get()) {
      lock.readLock().unlock();
      throw new NotServingRegionException(regionInfo.getRegionNameAsString() +
          " is closed");
    }


- Jean-Daniel


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





> 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