HBASE-3151 NPE when trying to read regioninfo from .META.

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1030293 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -642,6 +642,7 @@ Release 0.21.0 - Unreleased
    HBASE-3185  User-triggered compactions are triggering splits!
    HBASE-1932  Encourage use of 'lzo' compression... add the wiki page to
                getting started
+   HBASE-3151  NPE when trying to read regioninfo from .META.
 
 
   IMPROVEMENTS

src/main/java/org/apache/hadoop/hbase/catalog/MetaEditor.java

@@ -205,7 +205,7 @@ public class MetaEditor {
     catalogTracker.waitForMetaServerConnectionDefault().
       delete(CatalogTracker.META_REGION, delete);
     LOG.info("Deleted daughter " + daughter.getRegionNameAsString() +
-      " " + Bytes.toString(qualifier) + " from parent " +
+      " " + Bytes.toString(qualifier) + " reference in parent " +
       parent.getRegionNameAsString());
   }
 

src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java

@@ -136,6 +136,7 @@ public class MetaReader {
       public boolean visit(Result r) throws IOException {
         if (r ==  null || r.isEmpty()) return true;
         Pair<HRegionInfo,HServerAddress> region = metaRowToRegionPair(r);
+        if (region == null) return true;
         regions.put(region.getFirst(), region.getSecond());
         return true;
       }
@@ -296,13 +297,16 @@ public class MetaReader {
   /**
    * @param data A .META. table row.
    * @return A pair of the regioninfo and the server address from <code>data</code>
-   * (or null for server address if no address set in .META.).
+   * or null for server address if no address set in .META. or null for a result
+   * if no HRegionInfo found.
    * @throws IOException
    */
   public static Pair<HRegionInfo, HServerAddress> metaRowToRegionPair(
       Result data) throws IOException {
-    HRegionInfo info = Writables.getHRegionInfo(
+    byte [] bytes =
-      data.getValue(HConstants.CATALOG_FAMILY, HConstants.REGIONINFO_QUALIFIER));
+      data.getValue(HConstants.CATALOG_FAMILY, HConstants.REGIONINFO_QUALIFIER);
+    if (bytes == null) return null;
+    HRegionInfo info = Writables.getHRegionInfo(bytes);
     final byte[] value = data.getValue(HConstants.CATALOG_FAMILY,
       HConstants.SERVER_QUALIFIER);
     if (value != null && value.length > 0) {
@@ -463,6 +467,7 @@ public class MetaReader {
       while((data = metaServer.next(scannerid)) != null) {
         if (data != null && data.size() > 0) {
           Pair<HRegionInfo, HServerAddress> region = metaRowToRegionPair(data);
+          if (region == null) continue;
           if (region.getFirst().getTableDesc().getNameAsString().equals(
               tableName)) {
             regions.add(region);

src/main/java/org/apache/hadoop/hbase/client/MetaScanner.java

@@ -24,6 +24,8 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
@@ -40,6 +42,7 @@ import org.apache.hadoop.hbase.util.Writables;
  * minor releases.
  */
 public class MetaScanner {
+  private static final Log LOG = LogFactory.getLog(MetaScanner.class);
   /**
    * Scans the meta table and calls a visitor on each RowResult and uses a empty
    * start row value as table name.
@@ -93,7 +96,8 @@ public class MetaScanner {
     // if row is not null, we want to use the startKey of the row's region as
     // the startRow for the meta scan.
     byte[] startRow;
-    if (row != null) {
+    // row could be non-null but empty -- the HConstants.EMPTY_START_ROW
+    if (row != null && row.length > 0) {
       // Scan starting at a particular row in a particular table
       assert tableName != null;
       byte[] searchRow =
@@ -118,7 +122,9 @@ public class MetaScanner {
       byte[] rowBefore = regionInfo.getStartKey();
       startRow = HRegionInfo.createRegionName(tableName, rowBefore,
           HConstants.ZEROES, false);
-    } else if (tableName == null || tableName.length == 0) {
+    } else if (row == null ||
+        (row != null && Bytes.equals(HConstants.EMPTY_START_ROW, row)) ||
+        (tableName == null || tableName.length == 0)) {
       // Full META scan
       startRow = HConstants.EMPTY_START_ROW;
     } else {
@@ -133,8 +139,12 @@ public class MetaScanner {
         configuration.getInt("hbase.meta.scanner.caching", 100));
     do {
       final Scan scan = new Scan(startRow).addFamily(HConstants.CATALOG_FAMILY);
-      callable = new ScannerCallable(connection, HConstants.META_TABLE_NAME,
+      byte [] metaTableName = Bytes.equals(tableName, HConstants.ROOT_TABLE_NAME)?
-          scan);
+        HConstants.ROOT_TABLE_NAME: HConstants.META_TABLE_NAME;
+      LOG.debug("Scanning " + Bytes.toString(metaTableName) +
+        " starting at row=" + Bytes.toString(startRow) + " for max=" +
+        rowUpperLimit + " rows");
+      callable = new ScannerCallable(connection, metaTableName, scan);
       // Open scanner
       connection.getRegionServerWithRetries(callable);
 
@@ -172,10 +182,24 @@ public class MetaScanner {
 
   /**
    * Lists all of the regions currently in META.
-   * @return
+   * @param conf
+   * @return List of all user-space regions.
    * @throws IOException
    */
   public static List<HRegionInfo> listAllRegions(Configuration conf)
+  throws IOException {
+    return listAllRegions(conf, true);
+  }
+
+  /**
+   * Lists all of the regions currently in META.
+   * @param conf
+   * @param offlined True if we are to include offlined regions, false and we'll
+   * leave out offlined regions from returned list.
+   * @return List of all user-space regions.
+   * @throws IOException
+   */
+  public static List<HRegionInfo> listAllRegions(Configuration conf, final boolean offlined)
   throws IOException {
     final List<HRegionInfo> regions = new ArrayList<HRegionInfo>();
     MetaScannerVisitor visitor =
@@ -185,9 +209,15 @@ public class MetaScanner {
           if (result == null || result.isEmpty()) {
             return true;
           }
-          HRegionInfo regionInfo = Writables.getHRegionInfo(
+          byte [] bytes = result.getValue(HConstants.CATALOG_FAMILY,
-              result.getValue(HConstants.CATALOG_FAMILY,
+            HConstants.REGIONINFO_QUALIFIER);
-                  HConstants.REGIONINFO_QUALIFIER));
+          if (bytes == null) {
+            LOG.warn("Null REGIONINFO_QUALIFIER: " + result);
+            return true;
+          }
+          HRegionInfo regionInfo = Writables.getHRegionInfo(bytes);
+          // If region offline AND we are not to include offlined regions, return.
+          if (regionInfo.isOffline() && !offlined) return true;
           regions.add(regionInfo);
           return true;
         }

src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

@@ -519,9 +519,6 @@ public class AssignmentManager extends ZooKeeperListener {
         this.regionsInTransition.remove(regionInfo.getEncodedName());
       if (rs != null) {
         this.regionsInTransition.notifyAll();
-      } else {
-        LOG.warn("Asked online a region that was not in " +
-          "regionsInTransition: " + rs);
       }
     }
     synchronized (this.regions) {
@@ -597,7 +594,10 @@ public class AssignmentManager extends ZooKeeperListener {
       HServerInfo serverInfo = this.regions.remove(regionInfo);
       if (serverInfo != null) {
         List<HRegionInfo> serverRegions = this.servers.get(serverInfo);
-        serverRegions.remove(regionInfo);
+        if (!serverRegions.remove(regionInfo)) {
+          LOG.warn("Asked offline a region that was not on expected server: " +
+            regionInfo + ", " + serverInfo.getServerName());
+        }
       } else {
         LOG.warn("Asked offline a region that was not online: " + regionInfo);
       }
@@ -1104,9 +1104,9 @@ public class AssignmentManager extends ZooKeeperListener {
     // First experiment at synchronous assignment
     // Simpler because just wait for no regions in transition
 
-    // Scan META for all user regions
+    // Scan META for all user regions; do not include offlined regions in list.
     List<HRegionInfo> allRegions =
-      MetaScanner.listAllRegions(master.getConfiguration());
+      MetaScanner.listAllRegions(master.getConfiguration(), false);
     if (allRegions == null || allRegions.isEmpty()) return;
 
     // Get all available servers

src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java

@@ -98,6 +98,7 @@ class CatalogJanitor extends Chore {
         if (r == null || r.isEmpty()) return true;
         count.incrementAndGet();
         HRegionInfo info = getHRegionInfo(r);
+        if (info == null) return true; // Keep scanning
         if (info.isSplitParent()) splitParents.put(info, r);
         // Returning true means "keep scanning"
         return true;
@@ -157,7 +158,7 @@ class CatalogJanitor extends Chore {
     boolean hasReferencesB =
       checkDaughter(parent, rowContent, HConstants.SPLITB_QUALIFIER);
     if (!hasReferencesA && !hasReferencesB) {
-      LOG.info("Deleting region " + parent.getRegionNameAsString() +
+      LOG.debug("Deleting region " + parent.getRegionNameAsString() +
         " because daughter splits no longer hold references");
       FileSystem fs = this.services.getMasterFileSystem().getFileSystem();
       Path rootdir = this.services.getMasterFileSystem().getRootDir();