HADOOP-1888 NullPointerException in HMemcacheScanner

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

src/contrib/hbase/CHANGES.txt

@@ -46,6 +46,7 @@ Trunk (unreleased changes)
     HADOOP-1847 Many HBase tests do not fail well. (phase 2)
     HADOOP-1870 Once file system failure has been detected, don't check it again
                 and get on with shutting down the hbase cluster.
+    HADOOP-1888 NullPointerException in HMemcacheScanner
 
   IMPROVEMENTS
     HADOOP-1737 Make HColumnDescriptor data publically members settable

src/contrib/hbase/src/java/org/apache/hadoop/hbase/HBaseConfiguration.java

@@ -28,7 +28,7 @@ public class HBaseConfiguration extends Configuration {
   /** constructor */
   public HBaseConfiguration() {
     super();
-    addDefaultResource("hbase-default.xml");
-    addDefaultResource("hbase-site.xml");
+    addResource("hbase-default.xml");
+    addResource("hbase-site.xml");
   }
 }

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

@@ -897,7 +897,7 @@ HMasterRegionInterface, Runnable {
       
     } catch (IOException e) {
       LOG.fatal("Not starting HMaster because:", e);
-      return;
+      throw e;
     }
 
     this.threadWakeFrequency = conf.getLong(THREAD_WAKE_FREQUENCY, 10 * 1000);
@@ -1145,6 +1145,11 @@ HMasterRegionInterface, Runnable {
    * by remote region servers have expired.
    */
   private void letRegionServersShutdown() {
+    if (!fsOk) {
+      // Forget waiting for the region servers if the file system has gone
+      // away. Just exit as quickly as possible.
+      return;
+    }
     synchronized (serversToServerInfo) {
       while (this.serversToServerInfo.size() > 0) {
         LOG.info("Waiting on following regionserver(s) to go down (or " +

src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMemcache.java

@@ -26,7 +26,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.SortedMap;
 import java.util.TreeMap;
-import java.util.Vector;
 import java.util.concurrent.atomic.AtomicLong;
 
 import org.apache.commons.logging.Log;
@@ -40,10 +39,13 @@ import org.apache.hadoop.io.Text;
  */
 public class HMemcache {
   static final Log LOG = LogFactory.getLog(HMemcache.class);
-  TreeMap<HStoreKey, byte []> memcache =
-    new TreeMap<HStoreKey, byte []>();
-  final Vector<TreeMap<HStoreKey, byte []>> history
-    = new Vector<TreeMap<HStoreKey, byte []>>();
+  
+  // Note that since these structures are always accessed with a lock held,
+  // no additional synchronization is required.
+  
+  TreeMap<HStoreKey, byte []> memcache = new TreeMap<HStoreKey, byte []>();
+  final ArrayList<TreeMap<HStoreKey, byte []>> history =
+    new ArrayList<TreeMap<HStoreKey, byte []>>();
   TreeMap<HStoreKey, byte []> snapshot = null;
 
   final HLocking lock = new HLocking();
@@ -124,7 +126,8 @@ public class HMemcache {
         throw new IOException("Snapshot not present!");
       }
       for (Iterator<TreeMap<HStoreKey, byte []>> it = history.iterator(); 
-          it.hasNext();) {
+        it.hasNext(); ) {
+        
         TreeMap<HStoreKey, byte []> cur = it.next();
         if (snapshot == cur) {
           it.remove();
@@ -183,10 +186,11 @@ public class HMemcache {
           break;
         }
         results.addAll(results.size(),
-          get(history.elementAt(i), key, numVersions - results.size()));
+            get(history.get(i), key, numVersions - results.size()));
       }
-      return (results.size() == 0)?
-        null: ImmutableBytesWritable.toArray(results);
+      return (results.size() == 0) ? null :
+        ImmutableBytesWritable.toArray(results);
+      
     } finally {
       this.lock.releaseReadLock();
     }
@@ -205,8 +209,8 @@ public class HMemcache {
     this.lock.obtainReadLock();
     try {
       internalGetFull(memcache, key, results);
-      for (int i = history.size()-1; i >= 0; i--) {
-        TreeMap<HStoreKey, byte []> cur = history.elementAt(i);
+      for (int i = history.size() - 1; i >= 0; i--) {
+        TreeMap<HStoreKey, byte []> cur = history.get(i);
         internalGetFull(cur, key, results);
       }
       return results;
@@ -285,9 +289,9 @@ public class HMemcache {
     try {
       List<HStoreKey> results = getKeys(this.memcache, origin, versions);
       for (int i = history.size() - 1; i >= 0; i--) {
-        results.addAll(results.size(), getKeys(history.elementAt(i), origin,
-          versions == HConstants.ALL_VERSIONS? versions:
-           (results != null? versions - results.size(): versions)));
+        results.addAll(results.size(), getKeys(history.get(i), origin,
+            versions == HConstants.ALL_VERSIONS ? versions :
+              (versions - results.size())));
       }
       return results;
     } finally {
@@ -345,9 +349,8 @@ public class HMemcache {
    * Return a scanner over the keys in the HMemcache
    */
   HInternalScannerInterface getScanner(long timestamp,
-      Text targetCols[], Text firstRow)
-  throws IOException {  
-    return new HMemcacheScanner(timestamp, targetCols, firstRow);
+      Text targetCols[], Text firstRow) throws IOException {  
+      return new HMemcacheScanner(timestamp, targetCols, firstRow);
   }
 
   //////////////////////////////////////////////////////////////////////////////
@@ -361,35 +364,38 @@ public class HMemcache {
 
     @SuppressWarnings("unchecked")
     HMemcacheScanner(final long timestamp, final Text targetCols[],
-        final Text firstRow)
-    throws IOException {
+        final Text firstRow) throws IOException {
+
       super(timestamp, targetCols);
       lock.obtainReadLock();
       try {
         this.backingMaps = new TreeMap[history.size() + 1];
-        
-        //NOTE: Since we iterate through the backing maps from 0 to n, we need
-        // to put the memcache first, the newest history second, ..., etc.
+
+        // Note that since we iterate through the backing maps from 0 to n, we
+        // need to put the memcache first, the newest history second, ..., etc.
+
         backingMaps[0] = memcache;
-        for(int i = history.size() - 1; i > 0; i--) {
-          backingMaps[i] = history.elementAt(i);
+        for (int i = history.size() - 1; i > 0; i--) {
+          backingMaps[i + 1] = history.get(i);
         }
-      
+
         this.keyIterators = new Iterator[backingMaps.length];
         this.keys = new HStoreKey[backingMaps.length];
         this.vals = new byte[backingMaps.length][];
 
         // Generate list of iterators
+        
         HStoreKey firstKey = new HStoreKey(firstRow);
-        for(int i = 0; i < backingMaps.length; i++) {
-          keyIterators[i] = (/*firstRow != null &&*/ firstRow.getLength() != 0)?
-            backingMaps[i].tailMap(firstKey).keySet().iterator():
-            backingMaps[i].keySet().iterator();
-          while(getNext(i)) {
-            if(! findFirstRow(i, firstRow)) {
+        for (int i = 0; i < backingMaps.length; i++) {
+          keyIterators[i] = firstRow.getLength() != 0 ?
+              backingMaps[i].tailMap(firstKey).keySet().iterator() :
+                backingMaps[i].keySet().iterator();
+
+          while (getNext(i)) {
+            if (!findFirstRow(i, firstRow)) {
               continue;
             }
-            if(columnMatch(i)) {
+            if (columnMatch(i)) {
               break;
             }
           }

src/contrib/hbase/src/java/org/apache/hadoop/hbase/HRegionServer.java

@@ -676,8 +676,10 @@ public class HRegionServer implements HConstants, HRegionInterface, Runnable {
                 if (LOG.isDebugEnabled()) {
                   LOG.debug("Got call server startup message");
                 }
-                closeAllRegions();
-                restart = true;
+                if (fsOk) {
+                  closeAllRegions();
+                  restart = true;
+                }
                 break;
 
               case HMsg.MSG_REGIONSERVER_STOP:
@@ -689,10 +691,12 @@ public class HRegionServer implements HConstants, HRegionInterface, Runnable {
                 break;
 
               default:
-                try {
-                  toDo.put(new ToDoEntry(msgs[i]));
-                } catch (InterruptedException e) {
-                  throw new RuntimeException("Putting into msgQueue was interrupted.", e);
+                if (fsOk) {
+                  try {
+                    toDo.put(new ToDoEntry(msgs[i]));
+                  } catch (InterruptedException e) {
+                    throw new RuntimeException("Putting into msgQueue was interrupted.", e);
+                  }
                 }
               }
             }
@@ -747,20 +751,24 @@ public class HRegionServer implements HConstants, HRegionInterface, Runnable {
     }
 
     if (abortRequested) {
-      try {
-        log.close();
-        LOG.info("On abort, closed hlog");
-      } catch (IOException e) {
-        if (e instanceof RemoteException) {
-          try {
-            e = RemoteExceptionHandler.decodeRemoteException((RemoteException) e);
-          } catch (IOException ex) {
-            e = ex;
+      if (fsOk) {
+        // Only try to clean up if the file system is available
+
+        try {
+          log.close();
+          LOG.info("On abort, closed hlog");
+        } catch (IOException e) {
+          if (e instanceof RemoteException) {
+            try {
+              e = RemoteExceptionHandler.decodeRemoteException((RemoteException) e);
+            } catch (IOException ex) {
+              e = ex;
+            }
           }
+          LOG.error("Unable to close log in abort", e);
         }
-        LOG.error("Unable to close log in abort", e);
+        closeAllRegions(); // Don't leave any open file handles
       }
-      closeAllRegions(); // Don't leave any open file handles
       LOG.info("aborting server at: " +
         serverInfo.getServerAddress().toString());
     } else {

src/contrib/hbase/src/test/org/apache/hadoop/hbase/TestDFSAbort.java

@@ -32,7 +32,6 @@ public class TestDFSAbort extends HBaseClusterTestCase {
     super();
     conf.setInt("ipc.client.timeout", 5000);            // reduce ipc client timeout
     conf.setInt("ipc.client.connect.max.retries", 5);   // and number of retries
-    conf.setInt("hbase.client.retries.number", 5);      // reduce HBase retries
     Logger.getRootLogger().setLevel(Level.WARN);
     Logger.getLogger(this.getClass().getPackage().getName()).setLevel(Level.DEBUG);
   }