HADOOP-2688 IllegalArgumentException processing a shutdown stops server

going down and results in millions of lines of output


git-svn-id: https://svn.apache.org/repos/asf/hadoop/core/trunk/src/contrib/hbase@614796 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -146,6 +146,8 @@ Trunk (unreleased changes)
    HADOOP-2668 Documentation and improved logging so fact that hbase now
                requires migration comes as less of a surprise
    HADOOP-2686 Removed tables stick around in .META.
+   HADOOP-2688 IllegalArgumentException processing a shutdown stops
+               server going down and results in millions of lines of output
    
   IMPROVEMENTS
    HADOOP-2401 Add convenience put method that takes writable

src/java/org/apache/hadoop/hbase/HMaster.java

@@ -223,24 +223,19 @@ public class HMaster extends Thread implements HConstants, HMasterInterface,
 
         int numberOfRegionsFound = 0;
         while (true) {
-          SortedMap<Text, byte[]> results = new TreeMap<Text, byte[]>();
           HbaseMapWritable values = regionServer.next(scannerId);
           if (values == null || values.size() == 0) {
             break;
           }
-          for (Map.Entry<Writable, Writable> e: values.entrySet()) {
+
-            HStoreKey key = (HStoreKey) e.getKey();
+          // TODO: Why does this have to be a sorted map?
-            results.put(key.getColumn(),
+          SortedMap<Text, byte[]> results = toRowMap(values).getMap();
-                ((ImmutableBytesWritable) e.getValue()).get());
+          
-          }
+          HRegionInfo info = getHRegionInfo(results);
-          byte [] bytes = results.get(COL_REGIONINFO);
+          if (info == null) {
-          if (bytes == null) {
-            LOG.warn(COL_REGIONINFO.toString() + " is empty; has keys: " +
-              values.keySet().toString());
             continue;
           }
-          HRegionInfo info = (HRegionInfo) Writables.getWritable(bytes,
+
-            new HRegionInfo());
           String serverName = Writables.bytesToString(results.get(COL_SERVER));
           long startCode = Writables.bytesToLong(results.get(COL_STARTCODE));
           if (LOG.isDebugEnabled()) {
@@ -2007,28 +2002,13 @@ public class HMaster extends Thread implements HConstants, HMasterInterface,
               RemoteExceptionHandler.checkIOException(e));
             break;
           }
-
           if (values == null || values.size() == 0) {
             break;
           }
-
+          // TODO: Why does this have to be a sorted map?
-          SortedMap<Text, byte[]> results = new TreeMap<Text, byte[]>();
+          RowMap rm = toRowMap(values);
-          Text row = null;
+          Text row = rm.getRow();
-          for (Map.Entry<Writable, Writable> e: values.entrySet()) {
+          SortedMap<Text, byte[]> map = rm.getMap();
-            HStoreKey key = (HStoreKey) e.getKey();
-            Text thisRow = key.getRow();
-            if (row == null) {
-              row = thisRow;
-            } else {
-              if (!row.equals(thisRow)) {
-                LOG.error("Multiple rows in same scanner result set. firstRow="
-                    + row + ", currentRow=" + thisRow);
-              }
-            }
-            results.put(key.getColumn(),
-                ((ImmutableBytesWritable) e.getValue()).get());
-          }
-
           if (LOG.isDebugEnabled() && row != null) {
             LOG.debug("shutdown scanner looking at " + row.toString());
           }
@@ -2038,8 +2018,8 @@ public class HMaster extends Thread implements HConstants, HMasterInterface,
           // missed edits in hlog because hdfs does not do write-append).
           String serverName;
           try {
-            serverName = Writables.bytesToString(results.get(COL_SERVER));
+            serverName = Writables.bytesToString(map.get(COL_SERVER));
-          } catch(UnsupportedEncodingException e) {
+          } catch (UnsupportedEncodingException e) {
             LOG.error("Server name", e);
             break;
           }
@@ -2054,13 +2034,9 @@ public class HMaster extends Thread implements HConstants, HMasterInterface,
           }
 
           // Bingo! Found it.
-          HRegionInfo info = null;
+          HRegionInfo info = getHRegionInfo(map);
-          try {
+          if (info == null) {
-            info = (HRegionInfo) Writables.getWritable(
+            continue;
-                results.get(COL_REGIONINFO), new HRegionInfo());
-          } catch (IOException e) {
-            LOG.error("Read fields", e);
-            break;
           }
           LOG.info(info.getRegionName() + " was on shutdown server <" +
               serverName + "> (or server is null). Marking unassigned in " +
@@ -2720,52 +2696,21 @@ public class HMaster extends Thread implements HConstants, HMasterInterface,
 
               try {
                 while (true) {
-                  HRegionInfo info = new HRegionInfo();
-                  String serverName = null;
-                  long startCode = -1L;
-
                   HbaseMapWritable values = server.next(scannerId);
                   if(values == null || values.size() == 0) {
                     break;
                   }
-                  boolean haveRegionInfo = false;
+                  RowMap rm = toRowMap(values);
-                  for (Map.Entry<Writable, Writable> e: values.entrySet()) {
+                  SortedMap<Text, byte[]> map = rm.getMap();
-
+                  HRegionInfo info = getHRegionInfo(map);
-                    byte[] value = ((ImmutableBytesWritable) e.getValue()).get();
+                  if (info == null) {
-                    if (value == null || value.length == 0) {
+                    throw new IOException(COL_REGIONINFO + " not found on " +
-                      break;
+                      rm.getRow());
-                    }
-                    HStoreKey key = (HStoreKey) e.getKey();
-                    Text column = key.getColumn();
-                    if (column.equals(COL_REGIONINFO)) {
-                      haveRegionInfo = true;
-                      info = (HRegionInfo) Writables.getWritable(value, info);
-                    
-                    } else if (column.equals(COL_SERVER)) {
-                      try {
-                        serverName =
-                          Writables.bytesToString(value);
-                    
-                      } catch (UnsupportedEncodingException ex) {
-                        assert(false);
-                      }
-                    
-                    } else if (column.equals(COL_STARTCODE)) {
-                      try {
-                        startCode = Writables.bytesToLong(value);
-                      
-                      } catch (UnsupportedEncodingException ex) {
-                        assert(false);
-                      }
-                    }
                   }
-
+                  String serverName = Writables.bytesToString(map.get(COL_SERVER));
-                  if (!haveRegionInfo) {
+                  long startCode = Writables.bytesToLong(map.get(COL_STARTCODE));
-                    throw new IOException(COL_REGIONINFO + " not found");
-                  }
-
                   if (info.getTableDesc().getName().compareTo(tableName) > 0) {
-                    break;               // Beyond any more entries for this table
+                    break; // Beyond any more entries for this table
                   }
 
                   tableExists = true;
@@ -2773,9 +2718,7 @@ public class HMaster extends Thread implements HConstants, HMasterInterface,
                     unservedRegions.add(info);
                   }
                   processScanItem(serverName, startCode, info);
-
                 } // while(true)
-
               } finally {
                 if (scannerId != -1L) {
                   try {
@@ -2994,7 +2937,7 @@ public class HMaster extends Thread implements HConstants, HMasterInterface,
     protected void updateRegionInfo(BatchUpdate b,
         @SuppressWarnings("unused") HRegionInfo info) {
       for (int i = 0; i < ALL_META_COLUMNS.length; i++) {
-        // Be sure to clean all columns
+        // Be sure to clean all cells
         b.delete(lockid, ALL_META_COLUMNS[i]);
       }
     }
@@ -3171,6 +3114,75 @@ public class HMaster extends Thread implements HConstants, HMasterInterface,
     return this.conf;
   }
   
+  /*
+   * Data structure used to return results out of the toRowMap method.
+   */
+  private class RowMap {
+    final Text row;
+    final SortedMap<Text, byte[]> map;
+    
+    private RowMap(final Text r, final SortedMap<Text, byte[]> m) {
+      this.row = r;
+      this.map = m;
+    }
+
+    private Text getRow() {
+      return this.row;
+    }
+
+    private SortedMap<Text, byte[]> getMap() {
+      return this.map;
+    }
+  }
+  
+  /*
+   * Convert an HbaseMapWritable to a Map keyed by column.
+   * Utility method used scanning meta regions
+   * @param mw The MapWritable to convert.  Cannot be null.
+   * @return Returns a SortedMap currently.  TODO: This looks like it could
+   * be a plain Map.
+   */
+  protected RowMap toRowMap(final HbaseMapWritable mw) {
+    if (mw == null) {
+      throw new IllegalArgumentException("Passed MapWritable cannot be null");
+    }
+    SortedMap<Text, byte[]> m = new TreeMap<Text, byte[]>();
+    Text row = null;
+    for (Map.Entry<Writable, Writable> e: mw.entrySet()) {
+      HStoreKey key = (HStoreKey) e.getKey();
+      Text thisRow = key.getRow();
+      if (row == null) {
+        row = thisRow;
+      } else {
+        if (!row.equals(thisRow)) {
+          LOG.error("Multiple rows in same scanner result set. firstRow=" +
+            row + ", currentRow=" + thisRow);
+        }
+      }
+      m.put(key.getColumn(), ((ImmutableBytesWritable) e.getValue()).get());
+    }
+    return new RowMap(row, m);
+  }
+  
+  /*
+   * Get HRegionInfo from passed META map of row values.
+   * Returns null if none found (and logs fact that expected COL_REGIONINFO
+   * was missing).  Utility method used by scanners of META tables.
+   * @param map Map to do lookup in.
+   * @return Null or found HRegionInfo.
+   * @throws IOException
+   */
+  protected HRegionInfo getHRegionInfo(final Map<Text, byte[]> map)
+  throws IOException {
+    byte [] bytes = map.get(COL_REGIONINFO);
+    if (bytes == null) {
+      LOG.warn(COL_REGIONINFO.toString() + " is empty; has keys: " +
+        map.keySet().toString());
+      return null;
+    }
+    return (HRegionInfo)Writables.getWritable(bytes, new HRegionInfo());
+  }
+
   /*
    * Main program
    */

src/java/org/apache/hadoop/hbase/HStoreFile.java

@@ -413,15 +413,13 @@ public class HStoreFile implements HConstants {
    * @return MapFile.Reader
    * @throws IOException
    */
-  public synchronized MapFile.Reader getReader(final FileSystem fs,
+  public MapFile.Reader getReader(final FileSystem fs,
-      final Filter bloomFilter) throws IOException {
+    final Filter bloomFilter)
-    
+  throws IOException {
-    if (isReference()) {
+    return isReference()?
-      return new HStoreFile.HalfMapFileReader(fs,
+      new HStoreFile.HalfMapFileReader(fs, getMapFilePath(reference).toString(),
-          getMapFilePath(reference).toString(), conf, 
+        conf, reference.getFileRegion(), reference.getMidkey(), bloomFilter):
-          reference.getFileRegion(), reference.getMidkey(), bloomFilter);
+      new BloomFilterMapFile.Reader(fs, getMapFilePath().toString(),
-    }
-    return new BloomFilterMapFile.Reader(fs, getMapFilePath().toString(),
         conf, bloomFilter);
   }