You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2010/05/01 21:58:26 UTC

svn commit: r940099 - in /hadoop/hbase/branches/0.20: ./ src/contrib/transactional/src/java/org/apache/hadoop/hbase/client/transactional/ src/contrib/transactional/src/java/org/apache/hadoop/hbase/regionserver/tableindexed/

Author: stack
Date: Sat May  1 19:58:26 2010
New Revision: 940099

URL: http://svn.apache.org/viewvc?rev=940099&view=rev
Log:
HBASE-2493 [Transactional Contrib] Avoid unsafe concurrent use of HTable

Modified:
    hadoop/hbase/branches/0.20/CHANGES.txt
    hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/client/transactional/HBaseBackedTransactionLogger.java
    hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/regionserver/tableindexed/IndexedRegion.java

Modified: hadoop/hbase/branches/0.20/CHANGES.txt
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.20/CHANGES.txt?rev=940099&r1=940098&r2=940099&view=diff
==============================================================================
--- hadoop/hbase/branches/0.20/CHANGES.txt (original)
+++ hadoop/hbase/branches/0.20/CHANGES.txt Sat May  1 19:58:26 2010
@@ -110,6 +110,8 @@ Release 0.20.4 - Unreleased
    HBASE-2499  Race condition when disabling a table leaves regions in transition
    HBASE-2421  Put hangs for 10 retries on failed region servers
                (Todd Lipcon via Stack)
+   HBASE-2493  [Transactional Contrib] Avoid unsafe concurrent use of HTable
+               (Clint Morgan via Stack)
 
   IMPROVEMENTS
    HBASE-2180  Bad read performance from synchronizing hfile.fddatainputstream

Modified: hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/client/transactional/HBaseBackedTransactionLogger.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/client/transactional/HBaseBackedTransactionLogger.java?rev=940099&r1=940098&r2=940099&view=diff
==============================================================================
--- hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/client/transactional/HBaseBackedTransactionLogger.java (original)
+++ hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/client/transactional/HBaseBackedTransactionLogger.java Sat May  1 19:58:26 2010
@@ -30,6 +30,7 @@ import org.apache.hadoop.hbase.client.De
 import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
 import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.client.HTablePool;
 import org.apache.hadoop.hbase.client.Put;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -63,8 +64,19 @@ public class HBaseBackedTransactionLogge
   }
 
   private Random random = new Random();
-  private HTable table;
+  private HTablePool tablePool = new HTablePool();
 
+  private HTable getTable() {
+    return tablePool.getTable(TABLE_NAME);
+  }
+  
+  private void putTable(HTable t) {
+    if (t == null) {
+      return;
+    }
+    tablePool.putTable(t);
+  }
+  
   public HBaseBackedTransactionLogger() throws IOException {
     initTable();
   }
@@ -75,8 +87,6 @@ public class HBaseBackedTransactionLogge
     if (!admin.tableExists(TABLE_NAME)) {
       throw new RuntimeException("Table not created. Call createTable() first");
     }
-    this.table = new HTable(TABLE_NAME);
-
   }
 
   public long createNewTransactionLog() {
@@ -85,7 +95,11 @@ public class HBaseBackedTransactionLogge
 
     do {
       id = random.nextLong();
+      try {
       existing = getStatusForTransaction(id);
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
     } while (existing != null);
     
     setStatusForTransaction(id, TransactionStatus.PENDING);
@@ -93,7 +107,8 @@ public class HBaseBackedTransactionLogge
     return id;
   }
 
-  public TransactionStatus getStatusForTransaction(long transactionId) {
+  public TransactionStatus getStatusForTransaction(long transactionId) throws IOException {
+    HTable table = getTable();
     try {
       Result result = table.get(new Get(getRow(transactionId)));
       if (result == null || result.isEmpty()) {
@@ -108,6 +123,8 @@ public class HBaseBackedTransactionLogge
 
     } catch (IOException e) {
       throw new RuntimeException(e);
+    }finally {
+      putTable(table);
     }
   }
   
@@ -120,20 +137,26 @@ public class HBaseBackedTransactionLogge
     Put update = new Put(getRow(transactionId));
     update.add(STATUS_COLUMN_BYTES, HConstants.LATEST_TIMESTAMP, Bytes.toBytes(status.name()));
 
+    HTable table = getTable();
     try {
       table.put(update);
     } catch (IOException e) {
       throw new RuntimeException(e);
+    } finally {
+      putTable(table);
     }
   }
 
   public void forgetTransaction(long transactionId) {
     Delete delete = new Delete(getRow(transactionId));
-
+    
+    HTable table = getTable();
     try {
       table.delete(delete);
     } catch (IOException e) {
       throw new RuntimeException(e);
+    }finally {
+      putTable(table);
     }
   }
 

Modified: hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/regionserver/tableindexed/IndexedRegion.java
URL: http://svn.apache.org/viewvc/hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/regionserver/tableindexed/IndexedRegion.java?rev=940099&r1=940098&r2=940099&view=diff
==============================================================================
--- hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/regionserver/tableindexed/IndexedRegion.java (original)
+++ hadoop/hbase/branches/0.20/src/contrib/transactional/src/java/org/apache/hadoop/hbase/regionserver/tableindexed/IndexedRegion.java Sat May  1 19:58:26 2010
@@ -45,6 +45,7 @@ import org.apache.hadoop.hbase.Leases;
 import org.apache.hadoop.hbase.client.Delete;
 import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.client.HTable;
+import org.apache.hadoop.hbase.client.HTablePool;
 import org.apache.hadoop.hbase.client.Put;
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.client.tableindexed.IndexSpecification;
@@ -60,7 +61,7 @@ class IndexedRegion extends Transactiona
 
   private final HBaseConfiguration conf;
   private final IndexedTableDescriptor indexTableDescriptor;
-  private Map<IndexSpecification, HTable> indexSpecToTable = new HashMap<IndexSpecification, HTable>();
+  private final HTablePool tablePool;
 
   public IndexedRegion(final Path basedir, final HLog log, final FileSystem fs,
       final HBaseConfiguration conf, final HRegionInfo regionInfo,
@@ -68,17 +69,20 @@ class IndexedRegion extends Transactiona
     super(basedir, log, fs, conf, regionInfo, flushListener, trxLeases);
     this.indexTableDescriptor = new IndexedTableDescriptor(regionInfo.getTableDesc());
     this.conf = conf;
+    this.tablePool = new HTablePool();
   }
 
-  private synchronized HTable getIndexTable(IndexSpecification index)
+  private HTable getIndexTable(IndexSpecification index)
       throws IOException {
-    HTable indexTable = indexSpecToTable.get(index);
-    if (indexTable == null) {
-      indexTable = new HTable(conf, index.getIndexedTableName(super
+    return tablePool.getTable(index.getIndexedTableName(super
           .getRegionInfo().getTableDesc().getName()));
-      indexSpecToTable.put(index, indexTable);
+  }
+  
+  private void putTable(HTable t) {
+    if (t==null) {
+      return;
     }
-    return indexTable;
+    tablePool.putTable(t);
   }
 
   private Collection<IndexSpecification> getIndexes() {
@@ -156,6 +160,7 @@ class IndexedRegion extends Transactiona
     Put indexPut = makeIndexUpdate(indexSpec, put.getRow(), newColumnValues);
     
     HTable indexTable = getIndexTable(indexSpec);
+    try {
     if (indexDelete != null && !Bytes.equals(indexDelete.getRow(), indexPut.getRow())) {
       // Only do the delete if the row changed. This way we save the put after delete issues in HBASE-2256
       LOG.debug("Deleting old index row ["+Bytes.toString(indexDelete.getRow())+"]. New row is ["+Bytes.toString(indexPut.getRow())+"].");
@@ -164,6 +169,9 @@ class IndexedRegion extends Transactiona
       LOG.debug("Skipping deleting index row ["+Bytes.toString(indexDelete.getRow())+"] because it has not changed.");
     }
     indexTable.put(indexPut);
+    } finally {
+      putTable(indexTable);
+    }
   }
   
  
@@ -283,6 +291,7 @@ class IndexedRegion extends Transactiona
         }
 
         HTable indexTable = getIndexTable(indexSpec);
+        try {
         if (indexDelete != null
             && (indexPut == null || !Bytes.equals(indexDelete.getRow(),
                 indexPut.getRow()))) {
@@ -308,19 +317,21 @@ class IndexedRegion extends Transactiona
                     indexTable.delete(columnDelete);
                   }
                 }
-                
+
               }
+            }
           }
-        }
 
-        if (indexPut != null) {
-          getIndexTable(indexSpec).put(indexPut);
+          if (indexPut != null) {
+            indexTable.put(indexPut);
+          }
+        } finally {
+          putTable(indexTable);
         }
       }
-
     }
+
   }
-   
   
 
   private SortedMap<byte[], byte[]> convertToValueMap(Result result) {