diff --git a/CHANGES.txt b/CHANGES.txt index 23ecd2110e3..e47f49259ce 100644 --- a/CHANGES.txt +++ b/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 diff --git a/src/java/org/apache/hadoop/hbase/HBaseConfiguration.java b/src/java/org/apache/hadoop/hbase/HBaseConfiguration.java index 6e89e76231d..71f53173e46 100644 --- a/src/java/org/apache/hadoop/hbase/HBaseConfiguration.java +++ b/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"); } } diff --git a/src/java/org/apache/hadoop/hbase/HMaster.java b/src/java/org/apache/hadoop/hbase/HMaster.java index ebf217a62fd..5d7aa06a444 100644 --- a/src/java/org/apache/hadoop/hbase/HMaster.java +++ b/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 " + diff --git a/src/java/org/apache/hadoop/hbase/HMemcache.java b/src/java/org/apache/hadoop/hbase/HMemcache.java index 9a3120c67ea..b029e987b92 100644 --- a/src/java/org/apache/hadoop/hbase/HMemcache.java +++ b/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 memcache = - new TreeMap(); - final Vector> history - = new Vector>(); + + // Note that since these structures are always accessed with a lock held, + // no additional synchronization is required. + + TreeMap memcache = new TreeMap(); + final ArrayList> history = + new ArrayList>(); TreeMap snapshot = null; final HLocking lock = new HLocking(); @@ -124,7 +126,8 @@ public class HMemcache { throw new IOException("Snapshot not present!"); } for (Iterator> it = history.iterator(); - it.hasNext();) { + it.hasNext(); ) { + TreeMap 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 cur = history.elementAt(i); + for (int i = history.size() - 1; i >= 0; i--) { + TreeMap cur = history.get(i); internalGetFull(cur, key, results); } return results; @@ -285,9 +289,9 @@ public class HMemcache { try { List 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; } } diff --git a/src/java/org/apache/hadoop/hbase/HRegionServer.java b/src/java/org/apache/hadoop/hbase/HRegionServer.java index 91f22aacde0..c3fd41d059d 100644 --- a/src/java/org/apache/hadoop/hbase/HRegionServer.java +++ b/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 { diff --git a/src/test/org/apache/hadoop/hbase/TestDFSAbort.java b/src/test/org/apache/hadoop/hbase/TestDFSAbort.java index 5acde9e6c95..88779c9fbea 100644 --- a/src/test/org/apache/hadoop/hbase/TestDFSAbort.java +++ b/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); }