Author: stack
Date: Wed Sep 8 16:55:02 2010
New Revision: 995148
URL: http://svn.apache.org/viewvc?rev=995148&view=rev
Log:
HBASE-2964 Deadlock when RS tries to RPC to itself inside SplitTransaction
Modified:
hbase/branches/0.89.20100830/CHANGES.txt
hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
hbase/branches/0.89.20100830/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
Modified: hbase/branches/0.89.20100830/CHANGES.txt
URL:
http://svn.apache.org/viewvc/hbase/branches/0.89.20100830/CHANGES.txt?rev=995148&r1=995147&r2=995148&view=diff
==============================================================================
--- hbase/branches/0.89.20100830/CHANGES.txt (original)
+++ hbase/branches/0.89.20100830/CHANGES.txt Wed Sep 8 16:55:02 2010
@@ -488,6 +488,7 @@ Release 0.89.20100830 - Mon Aug 30 13:03
(Karthik Ranganathan via Stack)
HBASE-2915 Deadlock between HRegion.ICV and HRegion.close
HBASE-2920 HTable.checkAndPut/Delete doesn't handle null values
+ HBASE-2961 Deadlock when RS tries to RPC to itself inside SplitTransaction
IMPROVEMENTS
HBASE-1760 Cleanup TODOs in HTable
Modified:
hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
URL:
http://svn.apache.org/viewvc/hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=995148&r1=995147&r2=995148&view=diff
==============================================================================
---
hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
(original)
+++
hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
Wed Sep 8 16:55:02 2010
@@ -214,7 +214,7 @@ public class HRegion implements HeapSize
final FlushRequester flushListener;
private final long blockingMemStoreSize;
final long threadWakeFrequency;
- // Used to guard splits and closes
+ // Used to guard closes
final ReentrantReadWriteLock lock =
new ReentrantReadWriteLock();
Modified:
hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
URL:
http://svn.apache.org/viewvc/hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java?rev=995148&r1=995147&r2=995148&view=diff
==============================================================================
---
hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
(original)
+++
hbase/branches/0.89.20100830/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
Wed Sep 8 16:55:02 2010
@@ -61,6 +61,8 @@ import org.apache.hadoop.hbase.util.Writ
* }
* }
* </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);
@@ -125,36 +127,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;
}
/**
@@ -197,9 +189,7 @@ class SplitTransaction {
PairOfSameType<HRegion> execute(final OnlineRegions or, final boolean
updateMeta)
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";
// We'll need one of these later but get it now because if we fail there
// is nothing to undo.
@@ -272,9 +262,6 @@ class SplitTransaction {
// running a buffer -- its immediately flushing its puts.
if (t != null) t.close();
- // 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.
@@ -446,9 +433,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());
@@ -486,17 +470,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;
}
@@ -537,4 +512,4 @@ class SplitTransaction {
cleanupSplitDir(r.getFilesystem(), splitdir);
LOG.info("Cleaned up old failed split transaction detritus: " + splitdir);
}
-}
\ No newline at end of file
+}
Modified:
hbase/branches/0.89.20100830/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
URL:
http://svn.apache.org/viewvc/hbase/branches/0.89.20100830/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java?rev=995148&r1=995147&r2=995148&view=diff
==============================================================================
---
hbase/branches/0.89.20100830/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
(original)
+++
hbase/branches/0.89.20100830/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransaction.java
Wed Sep 8 16:55:02 2010
@@ -92,8 +92,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;
}
