Author: stack
Date: Wed Sep 8 16:57:52 2010
New Revision: 995151
URL: http://svn.apache.org/viewvc?rev=995151&view=rev
Log:
HBASE-2964 Deadlock when RS tries to RPC to itself inside SplitTransaction
Modified:
hbase/trunk/CHANGES.txt
hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
Modified: hbase/trunk/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hbase/trunk/CHANGES.txt?rev=995151&r1=995150&r2=995151&view=diff
==============================================================================
--- hbase/trunk/CHANGES.txt (original)
+++ hbase/trunk/CHANGES.txt Wed Sep 8 16:57:52 2010
@@ -506,6 +506,7 @@ Release 0.21.0 - Unreleased
HBASE-2925 LRU of HConnectionManager.HBASE_INSTANCES breaks if
HBaseConfiguration is changed
(Robert Mahfoud via Stack)
+ HBASE-2964 Deadlock when RS tries to RPC to itself inside SplitTransaction
IMPROVEMENTS
HBASE-1760 Cleanup TODOs in HTable
Modified:
hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL:
http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=995151&r1=995150&r2=995151&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
(original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
Wed Sep 8 16:57:52 2010
@@ -214,7 +214,7 @@ public class HRegion implements HeapSize
final FlushRequester flushRequester;
private final long blockingMemStoreSize;
final long threadWakeFrequency;
- // Used to guard splits and closes
+ // Used to guard closes
final ReentrantReadWriteLock lock =
new ReentrantReadWriteLock();
Modified:
hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
URL:
http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java?rev=995151&r1=995150&r2=995151&view=diff
==============================================================================
---
hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
(original)
+++
hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
Wed Sep 8 16:57:52 2010
@@ -61,6 +61,8 @@ import org.apache.zookeeper.KeeperExcept
* }
* }
* </Pre>
+ * <p>This class is not thread safe. Caller needs ensure split is run by
+ * one thread only.
*/
class SplitTransaction {
private static final Log LOG = LogFactory.getLog(SplitTransaction.class);
@@ -127,36 +129,26 @@ class SplitTransaction {
/**
* Does checks on split inputs.
* @return <code>true</code> if the region is splittable else
- * <code>false</code> if it is not (e.g. its already closed, etc.). If we
- * return <code>true</code>, we'll have taken out the parent's
- * <code>splitsAndClosesLock</code> and only way to unlock is successful
- * {...@link #execute(OnlineRegions)} or {...@link #rollback(OnlineRegions)}
+ * <code>false</code> if it is not (e.g. its already closed, etc.).
*/
public boolean prepare() {
- boolean prepared = false;
- this.parent.lock.writeLock().lock();
- try {
- if (this.parent.isClosed() || this.parent.isClosing()) return prepared;
- HRegionInfo hri = this.parent.getRegionInfo();
- // Check splitrow.
- byte [] startKey = hri.getStartKey();
- byte [] endKey = hri.getEndKey();
- if (Bytes.equals(startKey, splitrow) ||
- !this.parent.getRegionInfo().containsRow(splitrow)) {
- LOG.info("Split row is not inside region key range or is equal to " +
+ if (this.parent.isClosed() || this.parent.isClosing()) return false;
+ HRegionInfo hri = this.parent.getRegionInfo();
+ // Check splitrow.
+ byte [] startKey = hri.getStartKey();
+ byte [] endKey = hri.getEndKey();
+ if (Bytes.equals(startKey, splitrow) ||
+ !this.parent.getRegionInfo().containsRow(splitrow)) {
+ LOG.info("Split row is not inside region key range or is equal to " +
"startkey: " + Bytes.toString(this.splitrow));
- return prepared;
- }
- long rid = getDaughterRegionIdTimestamp(hri);
- this.hri_a = new HRegionInfo(hri.getTableDesc(), startKey, this.splitrow,
- false, rid);
- this.hri_b = new HRegionInfo(hri.getTableDesc(), this.splitrow, endKey,
- false, rid);
- prepared = true;
- } finally {
- if (!prepared) this.parent.lock.writeLock().unlock();
+ return false;
}
- return prepared;
+ long rid = getDaughterRegionIdTimestamp(hri);
+ this.hri_a = new HRegionInfo(hri.getTableDesc(), startKey, this.splitrow,
+ false, rid);
+ this.hri_b = new HRegionInfo(hri.getTableDesc(), this.splitrow, endKey,
+ false, rid);
+ return true;
}
/**
@@ -188,9 +180,7 @@ class SplitTransaction {
final RegionServerServices services)
throws IOException {
LOG.info("Starting split of region " + this.parent);
- if (!this.parent.lock.writeLock().isHeldByCurrentThread()) {
- throw new SplitAndCloseWriteLockNotHeld();
- }
+ assert !this.parent.lock.writeLock().isHeldByCurrentThread() : "Unsafe to
hold write lock while performing RPCs";
// If true, no cluster to write meta edits into.
boolean testing =
@@ -251,9 +241,6 @@ class SplitTransaction {
}
}
- // Unlock if successful split.
- this.parent.lock.writeLock().unlock();
-
// Leaving here, the splitdir with its dross will be in place but since the
// split was successful, just leave it; it'll be cleaned when parent is
// deleted and cleaned up.
@@ -441,9 +428,6 @@ class SplitTransaction {
* @throws IOException If thrown, rollback failed. Take drastic action.
*/
public void rollback(final OnlineRegions or) throws IOException {
- if (!this.parent.lock.writeLock().isHeldByCurrentThread()) {
- throw new SplitAndCloseWriteLockNotHeld();
- }
FileSystem fs = this.parent.getFilesystem();
ListIterator<JournalEntry> iterator =
this.journal.listIterator(this.journal.size());
@@ -481,17 +465,8 @@ class SplitTransaction {
throw new RuntimeException("Unhandled journal entry: " + je);
}
}
- if (this.parent.lock.writeLock().isHeldByCurrentThread()) {
- this.parent.lock.writeLock().unlock();
- }
}
- /**
- * Thrown if lock not held.
- */
- @SuppressWarnings("serial")
- public class SplitAndCloseWriteLockNotHeld extends IOException {}
-
HRegionInfo getFirstDaughter() {
return hri_a;
}
Modified:
hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
URL:
http://svn.apache.org/viewvc/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java?rev=995151&r1=995150&r2=995151&view=diff
==============================================================================
---
hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
(original)
+++
hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
Wed Sep 8 16:57:52 2010
@@ -95,8 +95,6 @@ public class TestSplitTransaction {
private SplitTransaction prepareGOOD_SPLIT_ROW() {
SplitTransaction st = new SplitTransaction(this.parent, GOOD_SPLIT_ROW);
assertTrue(st.prepare());
- // Assert the write lock is held on successful prepare as the javadoc
asserts.
- assertTrue(this.parent.lock.writeLock().isHeldByCurrentThread());
return st;
}
