You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by to...@apache.org on 2010/06/21 22:36:39 UTC

svn commit: r956685 - in /hbase/trunk: ./ src/main/java/org/apache/hadoop/hbase/ src/main/java/org/apache/hadoop/hbase/client/ src/main/java/org/apache/hadoop/hbase/master/ src/test/java/org/apache/hadoop/hbase/master/

Author: todd
Date: Mon Jun 21 20:36:39 2010
New Revision: 956685

URL: http://svn.apache.org/viewvc?rev=956685&view=rev
Log:
HBASE-2656. HMaster.getRegionTableClosest should not return null for closed regions

Modified:
    hbase/trunk/CHANGES.txt
    hbase/trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
    hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java
    hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
    hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java

Modified: hbase/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hbase/trunk/CHANGES.txt?rev=956685&r1=956684&r2=956685&view=diff
==============================================================================
--- hbase/trunk/CHANGES.txt (original)
+++ hbase/trunk/CHANGES.txt Mon Jun 21 20:36:39 2010
@@ -398,6 +398,8 @@ Release 0.21.0 - Unreleased
    HBASE-2752  Don't retry forever when waiting on too many store files
    HBASE-2737  CME in ZKW introduced in HBASE-2694 (Karthik Ranganathan via JD)
    HBASE-2756  MetaScanner.metaScan doesn't take configurations
+   HBASE-2656  HMaster.getRegionTableClosest should not return null for closed
+               regions
 
   IMPROVEMENTS
    HBASE-1760  Cleanup TODOs in HTable

Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java?rev=956685&r1=956684&r2=956685&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java Mon Jun 21 20:36:39 2010
@@ -423,6 +423,15 @@ public class HRegionInfo extends Version
       Bytes.equals(endKey, HConstants.EMPTY_BYTE_ARRAY);
     return firstKeyInRange && lastKeyInRange;
   }
+  
+  /**
+   * Return true if the given row falls in this region.
+   */
+  public boolean containsRow(byte[] row) {
+    return Bytes.compareTo(row, startKey) >= 0 &&
+      (Bytes.compareTo(row, endKey) < 0 ||
+       Bytes.equals(endKey, HConstants.EMPTY_BYTE_ARRAY));
+  }
 
   /** @return the tableDesc */
   public HTableDescriptor getTableDesc(){

Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java?rev=956685&r1=956684&r2=956685&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java Mon Jun 21 20:36:39 2010
@@ -33,8 +33,11 @@ import java.io.IOException;
  * Scanner class that contains the <code>.META.</code> table scanning logic
  * and uses a Retryable scanner. Provided visitors will be called
  * for each row.
+ * 
+ * Although public visibility, this is not a public-facing API and may evolve in
+ * minor releases.
  */
-class MetaScanner {
+public class MetaScanner {
 
   /**
    * Scans the meta table and calls a visitor on each RowResult and uses a empty
@@ -160,7 +163,7 @@ class MetaScanner {
   /**
    * Visitor class called to process each row of the .META. table
    */
-  interface MetaScannerVisitor {
+  public interface MetaScannerVisitor {
     /**
      * Visitor method that accepts a RowResult and the meta region location.
      * Implementations can return false to stop the region's loop if it becomes

Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java?rev=956685&r1=956684&r2=956685&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java Mon Jun 21 20:36:39 2010
@@ -33,6 +33,7 @@ import java.util.NavigableMap;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
@@ -62,6 +63,8 @@ import org.apache.hadoop.hbase.TableExis
 import org.apache.hadoop.hbase.client.Get;
 import org.apache.hadoop.hbase.client.HBaseAdmin;
 import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.MetaScanner;
+import org.apache.hadoop.hbase.client.MetaScanner.MetaScannerVisitor;
 import org.apache.hadoop.hbase.client.Scan;
 import org.apache.hadoop.hbase.client.ServerConnection;
 import org.apache.hadoop.hbase.client.ServerConnectionManager;
@@ -97,6 +100,8 @@ import org.apache.zookeeper.Watcher;
 import org.apache.zookeeper.Watcher.Event.EventType;
 import org.apache.zookeeper.Watcher.Event.KeeperState;
 
+import com.google.common.collect.Lists;
+
 /**
  * HMaster is the "master server" for HBase. An HBase cluster has one active
  * master.  If many masters are started, all compete.  Whichever wins goes on to
@@ -838,99 +843,94 @@ public class HMaster extends Thread impl
     new ChangeTableState(this, tableName, false).process();
   }
 
-  // TODO: Redo so this method does not duplicate code with subsequent methods.
+  /**
+   * Get a list of the regions for a given table. The pairs may have
+   * null for their second element in the case that they are not
+   * currently deployed.
+   * TODO: Redo so this method does not duplicate code with subsequent methods.
+   */
   List<Pair<HRegionInfo,HServerAddress>> getTableRegions(
       final byte [] tableName)
   throws IOException {
-    List<Pair<HRegionInfo,HServerAddress>> result =
-      new ArrayList<Pair<HRegionInfo,HServerAddress>>();
-    Set<MetaRegion> regions =
-      this.regionManager.getMetaRegionsForTable(tableName);
-    byte [] firstRowInTable = Bytes.toBytes(Bytes.toString(tableName) + ",,");
-    for (MetaRegion m: regions) {
-      byte [] metaRegionName = m.getRegionName();
-      HRegionInterface srvr =
-        this.connection.getHRegionConnection(m.getServer());
-      Scan scan = new Scan(firstRowInTable);
-      scan.addColumn(HConstants.CATALOG_FAMILY,
-          HConstants.REGIONINFO_QUALIFIER);
-      scan.addColumn(HConstants.CATALOG_FAMILY, HConstants.SERVER_QUALIFIER);
-      // TODO: Use caching.
-      long scannerid = srvr.openScanner(metaRegionName, scan);
-      try {
-        while (true) {
-          Result data = srvr.next(scannerid);
+    final ArrayList<Pair<HRegionInfo, HServerAddress>> result =
+      Lists.newArrayList();
+    MetaScannerVisitor visitor =
+      new MetaScannerVisitor() {
+        @Override
+        public boolean processRow(Result data) throws IOException {
           if (data == null || data.size() <= 0)
-            break;
-          HRegionInfo info = Writables.getHRegionInfo(
-              data.getValue(HConstants.CATALOG_FAMILY,
-                  HConstants.REGIONINFO_QUALIFIER));
-          if (Bytes.equals(info.getTableDesc().getName(), tableName)) {
-            final byte[] value = data.getValue(HConstants.CATALOG_FAMILY,
-                HConstants.SERVER_QUALIFIER);
-            if (value != null && value.length > 0) {
-              HServerAddress server = new HServerAddress(Bytes.toString(value));
-              result.add(new Pair<HRegionInfo,HServerAddress>(info, server));
-            }
-          } else {
-            break;
+            return true;
+          Pair<HRegionInfo, HServerAddress> pair =
+            metaRowToRegionPair(data);
+          if (pair == null) return false;
+          if (!Bytes.equals(pair.getFirst().getTableDesc().getName(),
+                tableName)) {
+            return false;
           }
+          result.add(pair);
+          return true;
         }
-      } finally {
-        srvr.close(scannerid);
-      }
-    }
+    };
+
+    MetaScanner.metaScan(conf, visitor, tableName); 
     return result;
   }
+  
+  private Pair<HRegionInfo, HServerAddress> metaRowToRegionPair(
+      Result data) throws IOException {
+    HRegionInfo info = Writables.getHRegionInfo(
+        data.getValue(HConstants.CATALOG_FAMILY,
+            HConstants.REGIONINFO_QUALIFIER));
+    final byte[] value = data.getValue(HConstants.CATALOG_FAMILY,
+        HConstants.SERVER_QUALIFIER);
+    if (value != null && value.length > 0) {
+      HServerAddress server = new HServerAddress(Bytes.toString(value));
+      return new Pair<HRegionInfo,HServerAddress>(info, server);
+    } else {
+      //undeployed
+      return new Pair<HRegionInfo, HServerAddress>(info, null);
+    }    
+  }
 
-  Pair<HRegionInfo,HServerAddress> getTableRegionClosest(
+  /**
+   * Return the region and current deployment for the region containing
+   * the given row. If the region cannot be found, returns null. If it
+   * is found, but not currently deployed, the second element of the pair
+   * may be null.
+   */
+  Pair<HRegionInfo,HServerAddress> getTableRegionForRow(
       final byte [] tableName, final byte [] rowKey)
   throws IOException {
-    Set<MetaRegion> regions =
-      this.regionManager.getMetaRegionsForTable(tableName);
-    for (MetaRegion m: regions) {
-      byte [] firstRowInTable = Bytes.toBytes(Bytes.toString(tableName) + ",,");
-      byte [] metaRegionName = m.getRegionName();
-      HRegionInterface srvr = this.connection.getHRegionConnection(m.getServer());
-      Scan scan = new Scan(firstRowInTable);
-      scan.addColumn(HConstants.CATALOG_FAMILY,
-          HConstants.REGIONINFO_QUALIFIER);
-      scan.addColumn(HConstants.CATALOG_FAMILY, HConstants.SERVER_QUALIFIER);
-      long scannerid = srvr.openScanner(metaRegionName, scan);
-      try {
-        while (true) {
-          Result data = srvr.next(scannerid);
+    final AtomicReference<Pair<HRegionInfo, HServerAddress>> result =
+      new AtomicReference<Pair<HRegionInfo, HServerAddress>>(null);
+    
+    MetaScannerVisitor visitor =
+      new MetaScannerVisitor() {
+        @Override
+        public boolean processRow(Result data) throws IOException {
           if (data == null || data.size() <= 0)
-            break;
-          HRegionInfo info = Writables.getHRegionInfo(
-              data.getValue(HConstants.CATALOG_FAMILY,
-                  HConstants.REGIONINFO_QUALIFIER));
-          if (Bytes.compareTo(info.getTableDesc().getName(), tableName) == 0) {
-            if ((Bytes.compareTo(info.getStartKey(), rowKey) >= 0) &&
-                (Bytes.compareTo(info.getEndKey(), rowKey) < 0)) {
-                final byte[] value = data.getValue(HConstants.CATALOG_FAMILY,
-                    HConstants.SERVER_QUALIFIER);
-                if (value != null && value.length > 0) {
-                  HServerAddress server =
-                    new HServerAddress(Bytes.toString(value));
-                  return new Pair<HRegionInfo,HServerAddress>(info, server);
-                }
-            }
-          } else {
-            break;
+            return true;
+          Pair<HRegionInfo, HServerAddress> pair =
+            metaRowToRegionPair(data);
+          if (pair == null) return false;
+          if (!Bytes.equals(pair.getFirst().getTableDesc().getName(),
+                tableName)) {
+            return false;
           }
+          result.set(pair);
+          return true;
         }
-      } finally {
-        srvr.close(scannerid);
-      }
-    }
-    return null;
-  }
+    };
 
+    MetaScanner.metaScan(conf, visitor, tableName, rowKey, 1);
+    return result.get();
+  }
+  
   Pair<HRegionInfo,HServerAddress> getTableRegionFromName(
       final byte [] regionName)
   throws IOException {
     byte [] tableName = HRegionInfo.parseRegionName(regionName)[0];
+    
     Set<MetaRegion> regions = regionManager.getMetaRegionsForTable(tableName);
     for (MetaRegion m: regions) {
       byte [] metaRegionName = m.getRegionName();
@@ -941,16 +941,7 @@ public class HMaster extends Thread impl
       get.addColumn(HConstants.CATALOG_FAMILY, HConstants.SERVER_QUALIFIER);
       Result data = srvr.get(metaRegionName, get);
       if(data == null || data.size() <= 0) continue;
-      HRegionInfo info = Writables.getHRegionInfo(
-          data.getValue(HConstants.CATALOG_FAMILY,
-              HConstants.REGIONINFO_QUALIFIER));
-      final byte[] value = data.getValue(HConstants.CATALOG_FAMILY,
-          HConstants.SERVER_QUALIFIER);
-      if(value != null && value.length > 0) {
-        HServerAddress server =
-          new HServerAddress(Bytes.toString(value));
-        return new Pair<HRegionInfo,HServerAddress>(info, server);
-      }
+      return metaRowToRegionPair(data);
     }
     return null;
   }
@@ -1008,16 +999,18 @@ public class HMaster extends Thread impl
           pair = getTableRegionFromName(regionName);
         } else {
           byte [] rowKey = ((ImmutableBytesWritable)args[0]).get();
-          pair = getTableRegionClosest(tableName, rowKey);
+          pair = getTableRegionForRow(tableName, rowKey);
         }
-        if (pair != null) {
+        if (pair != null && pair.getSecond() != null) {
           this.regionManager.startAction(pair.getFirst().getRegionName(),
             pair.getFirst(), pair.getSecond(), op);
         }
       } else {
-        for (Pair<HRegionInfo,HServerAddress> pair: getTableRegions(tableName))
+        for (Pair<HRegionInfo,HServerAddress> pair: getTableRegions(tableName)) {
+          if (pair.getSecond() == null) continue; // undeployed
           this.regionManager.startAction(pair.getFirst().getRegionName(),
             pair.getFirst(), pair.getSecond(), op);
+        }
       }
       break;
 

Modified: hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java?rev=956685&r1=956684&r2=956685&view=diff
==============================================================================
--- hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java (original)
+++ hbase/trunk/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java Mon Jun 21 20:36:39 2010
@@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.master;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.MiniHBaseCluster;
 import org.apache.hadoop.hbase.HMsg;
 import org.apache.hadoop.hbase.HServerInfo;
@@ -36,12 +37,16 @@ import org.apache.hadoop.hbase.util.Byte
 import org.apache.hadoop.hbase.util.Pair;
 
 import java.io.IOException;
+import java.util.List;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+
+import com.google.common.base.Joiner;
+
 import static org.junit.Assert.*;
 
 public class TestMaster {
@@ -70,30 +75,38 @@ public class TestMaster {
     TEST_UTIL.createTable(TABLENAME, FAMILYNAME);
     TEST_UTIL.loadTable(new HTable(TABLENAME), FAMILYNAME);
 
+    List<Pair<HRegionInfo, HServerAddress>> tableRegions =
+      m.getTableRegions(TABLENAME);
+    LOG.info("Regions after load: " + Joiner.on(',').join(tableRegions));
+    assertEquals(1, tableRegions.size());
+    assertArrayEquals(HConstants.EMPTY_START_ROW,
+        tableRegions.get(0).getFirst().getStartKey());
+    assertArrayEquals(HConstants.EMPTY_END_ROW,
+        tableRegions.get(0).getFirst().getEndKey());
+
+    // Now trigger a split and stop when the split is in progress
+    
     CountDownLatch aboutToOpen = new CountDownLatch(1);
     CountDownLatch proceed = new CountDownLatch(1);
     RegionOpenListener list = new RegionOpenListener(aboutToOpen, proceed);
     HBaseEventHandler.registerListener(list);
 
+    LOG.info("Splitting table");
     admin.split(TABLENAME);
+    LOG.info("Waiting for split result to be about to open");
     aboutToOpen.await(60, TimeUnit.SECONDS);
-
     try {
-      m.getTableRegions(TABLENAME);
+      LOG.info("Making sure we can call getTableRegions while opening");
+      tableRegions = m.getTableRegions(TABLENAME);
+      LOG.info("Regions: " + Joiner.on(',').join(tableRegions));
+      // We have three regions because one is split-in-progress
+      assertEquals(3, tableRegions.size());
+      LOG.info("Making sure we can call getTableRegionClosest while opening");
       Pair<HRegionInfo,HServerAddress> pair =
-        m.getTableRegionClosest(TABLENAME, Bytes.toBytes("cde"));
-      /**
-       * TODO: The assertNull below used to work before moving all RS->M 
-       * communication to ZK, find out why this test's behavior has changed.
-       * Tracked in HBASE-2656.
-        assertNull(pair);
-        assertNotNull(pair);
-        
-        We used to assert NotNull for the pair but it seems that ain't
-        always true either. For now disabling this assertion.  Filing
-        an issue for it to be checked -- St.Ack.
-      */
-      m.getTableRegionFromName(pair.getFirst().getRegionName());
+        m.getTableRegionForRow(TABLENAME, Bytes.toBytes("cde"));
+      LOG.info("Result is: " + pair);
+      Pair<HRegionInfo, HServerAddress> tableRegionFromName = m.getTableRegionFromName(pair.getFirst().getRegionName());
+      assertEquals(tableRegionFromName.getFirst(), pair.getFirst());
     } finally {
       proceed.countDown();
     }