hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@apache.org
Subject svn commit: r995148 - in /hbase/branches/0.89.20100830: ./ src/main/java/org/apache/hadoop/hbase/regionserver/ src/test/java/org/apache/hadoop/hbase/regionserver/
Date Wed, 08 Sep 2010 16:55:03 GMT
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;
   }
 



Mime
View raw message