Index: src/contrib/hbase/src/java/org/apache/hadoop/hbase/HRegionServer.java =================================================================== --- src/contrib/hbase/src/java/org/apache/hadoop/hbase/HRegionServer.java (revision 601205) +++ src/contrib/hbase/src/java/org/apache/hadoop/hbase/HRegionServer.java (working copy) @@ -1152,12 +1152,19 @@ } } - void openRegion(final HRegionInfo regionInfo) throws IOException { + void openRegion(final HRegionInfo regionInfo) { HRegion region = onlineRegions.get(regionInfo.getRegionName()); if(region == null) { - region = new HRegion(new Path(this.conf.get(HConstants.HBASE_DIR)), - this.log, FileSystem.get(conf), conf, regionInfo, null, - this.cacheFlusher); + try { + region = new HRegion(new Path(this.conf.get(HConstants.HBASE_DIR)), + this.log, FileSystem.get(conf), conf, regionInfo, null, + this.cacheFlusher); + + } catch (IOException e) { + LOG.error("error opening region " + regionInfo.getRegionName(), e); + reportClose(region); + return; + } this.lock.writeLock().lock(); try { this.log.setSequenceNumber(region.getMinSequenceId()); Index: src/contrib/hbase/src/java/org/apache/hadoop/hbase/HTableDescriptor.java =================================================================== --- src/contrib/hbase/src/java/org/apache/hadoop/hbase/HTableDescriptor.java (revision 601205) +++ src/contrib/hbase/src/java/org/apache/hadoop/hbase/HTableDescriptor.java (working copy) @@ -53,6 +53,7 @@ null)); + private boolean metatable; private Text name; // TODO: Does this need to be a treemap? Can it be a HashMap? private final TreeMap families; @@ -69,6 +70,7 @@ /** Used to construct the table descriptors for root and meta tables */ private HTableDescriptor(Text name, HColumnDescriptor family) { + this.metatable = true; this.name = new Text(name); this.families = new TreeMap(); families.put(family.getName(), family); @@ -92,14 +94,20 @@ * [a-zA-Z_0-9] */ public HTableDescriptor(String name) { + this(); Matcher m = LEGAL_TABLE_NAME.matcher(name); if (m == null || !m.matches()) { throw new IllegalArgumentException( "Table names can only contain 'word characters': i.e. [a-zA-Z_0-9"); } - this.name = new Text(name); - this.families = new TreeMap(); + this.name.set(name); + this.metatable = false; } + + /** @return if table is a meta table */ + public boolean isMetaTable() { + return metatable; + } /** @return name of table */ public Text getName() { @@ -165,6 +173,7 @@ /** {@inheritDoc} */ public void write(DataOutput out) throws IOException { + out.writeBoolean(metatable); name.write(out); out.writeInt(families.size()); for(Iterator it = families.values().iterator(); @@ -175,6 +184,7 @@ /** {@inheritDoc} */ public void readFields(DataInput in) throws IOException { + this.metatable = in.readBoolean(); this.name.readFields(in); int numCols = in.readInt(); families.clear(); Index: src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMaster.java =================================================================== --- src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMaster.java (revision 601205) +++ src/contrib/hbase/src/java/org/apache/hadoop/hbase/HMaster.java (working copy) @@ -74,6 +74,7 @@ HMasterRegionInterface { static final Log LOG = LogFactory.getLog(HMaster.class.getName()); + static final Long ZERO_L = Long.valueOf(0L); /** {@inheritDoc} */ public long getProtocolVersion(String protocol, @@ -424,8 +425,7 @@ || killedRegions.contains(info.getRegionName()) // queued for offline || regionsToDelete.contains(info.getRegionName())) { // queued for delete - unassignedRegions.remove(info.getRegionName()); - assignAttempts.remove(info.getRegionName()); + unassignedRegions.remove(info); return; } HServerInfo storedInfo = null; @@ -460,7 +460,7 @@ if (!deadServer && ((storedInfo != null && storedInfo.getStartCode() != startCode) || (storedInfo == null && - !unassignedRegions.containsKey(info.getRegionName()) && + !unassignedRegions.containsKey(info) && !pendingRegions.contains(info.getRegionName()) ) ) @@ -497,8 +497,7 @@ } } // Now get the region assigned - unassignedRegions.put(info.getRegionName(), info); - assignAttempts.put(info.getRegionName(), Long.valueOf(0L)); + unassignedRegions.put(info, ZERO_L); } } } @@ -820,8 +819,9 @@ new ConcurrentHashMap(); /** - * The 'unassignedRegions' table maps from a region name to a HRegionInfo - * record, which includes the region's table, its id, and its start/end keys. + * The 'unassignedRegions' table maps from a HRegionInfo to a timestamp that + * indicates the last time we *tried* to assign the region to a RegionServer. + * If the timestamp is out of date, then we can try to reassign it. * * We fill 'unassignedRecords' by scanning ROOT and META tables, learning the * set of all known valid regions. @@ -829,17 +829,10 @@ *

Items are removed from this list when a region server reports in that * the region has been deployed. */ - final SortedMap unassignedRegions = - Collections.synchronizedSortedMap(new TreeMap()); + final SortedMap unassignedRegions = + Collections.synchronizedSortedMap(new TreeMap()); /** - * The 'assignAttempts' table maps from regions to a timestamp that indicates - * the last time we *tried* to assign the region to a RegionServer. If the - * timestamp is out of date, then we can try to reassign it. - */ - final Map assignAttempts = new ConcurrentHashMap(); - - /** * Regions that have been assigned, and the server has reported that it has * started serving it, but that we have not yet recorded in the meta table. */ @@ -980,10 +973,7 @@ */ void unassignRootRegion() { this.rootRegionLocation.set(null); - this.unassignedRegions.put(HRegionInfo.rootRegionInfo.getRegionName(), - HRegionInfo.rootRegionInfo); - this.assignAttempts.put(HRegionInfo.rootRegionInfo.getRegionName(), - Long.valueOf(0L)); + this.unassignedRegions.put(HRegionInfo.rootRegionInfo, ZERO_L); } /** @@ -1331,8 +1321,7 @@ onlineMetaRegions.remove(info.getStartKey()); } - this.unassignedRegions.put(info.getRegionName(), info); - this.assignAttempts.put(info.getRegionName(), Long.valueOf(0L)); + this.unassignedRegions.put(info, ZERO_L); } } } @@ -1478,62 +1467,86 @@ switch (incomingMsgs[i].getMsg()) { case HMsg.MSG_REPORT_PROCESS_OPEN: - synchronized (this.assignAttempts) { + synchronized (unassignedRegions) { // Region server has acknowledged request to open region. - // Extend region open time by 1/2 max region open time. - assignAttempts.put(region.getRegionName(), - Long.valueOf(assignAttempts.get( - region.getRegionName()).longValue() + - (this.maxRegionOpenTime / 2))); + // Extend region open time by max region open time. + unassignedRegions.put(region, + System.currentTimeMillis() + this.maxRegionOpenTime); } break; case HMsg.MSG_REPORT_OPEN: - HRegionInfo regionInfo = unassignedRegions.get(region.getRegionName()); - - if (regionInfo == null) { - - if (LOG.isDebugEnabled()) { - LOG.debug("region server " + info.getServerAddress().toString() - + " should not have opened region " + region.getRegionName()); + boolean duplicateAssignment = false; + synchronized (unassignedRegions) { + if (unassignedRegions.remove(region) == null) { + if (region.getRegionName().compareTo( + HRegionInfo.rootRegionInfo.getRegionName()) == 0) { + // Root region + HServerAddress rootServer = rootRegionLocation.get(); + if (rootServer != null) { + if (rootServer.toString().compareTo(serverName) == 0) { + // A duplicate open report from the correct server + break; + } + // We received an open report on the root region, but it is + // assigned to a different server + duplicateAssignment = true; + } + } else { + // Not root region. If it is not a pending region, then we are + // going to treat it as a duplicate assignment + if (pendingRegions.contains(region.getRegionName())) { + // A duplicate report from the correct server + break; + } + // Although we can't tell for certain if this is a duplicate + // report from the correct server, we are going to treat it + // as such + duplicateAssignment = true; + } } + if (duplicateAssignment) { + if (LOG.isDebugEnabled()) { + LOG.debug("region server " + info.getServerAddress().toString() + + " should not have opened region " + region.getRegionName()); + } - // This Region should not have been opened. - // Ask the server to shut it down, but don't report it as closed. - // Otherwise the HMaster will think the Region was closed on purpose, - // and then try to reopen it elsewhere; that's not what we want. + // This Region should not have been opened. + // Ask the server to shut it down, but don't report it as closed. + // Otherwise the HMaster will think the Region was closed on purpose, + // and then try to reopen it elsewhere; that's not what we want. - returnMsgs.add(new HMsg(HMsg.MSG_REGION_CLOSE_WITHOUT_REPORT, region)); + returnMsgs.add( + new HMsg(HMsg.MSG_REGION_CLOSE_WITHOUT_REPORT, region)); - } else { - LOG.info(info.getServerAddress().toString() + " serving " + - region.getRegionName()); - - if (region.getRegionName().compareTo( - HRegionInfo.rootRegionInfo.getRegionName()) == 0) { - // Store the Root Region location (in memory) - synchronized (rootRegionLocation) { - this.rootRegionLocation.set( - new HServerAddress(info.getServerAddress())); - this.rootRegionLocation.notifyAll(); - } } else { - // Note that the table has been assigned and is waiting for the meta - // table to be updated. + LOG.info(info.getServerAddress().toString() + " serving " + + region.getRegionName()); - pendingRegions.add(region.getRegionName()); + if (region.getRegionName().compareTo( + HRegionInfo.rootRegionInfo.getRegionName()) == 0) { + // Store the Root Region location (in memory) + synchronized (rootRegionLocation) { + this.rootRegionLocation.set( + new HServerAddress(info.getServerAddress())); + this.rootRegionLocation.notifyAll(); + } + } else { + // Note that the table has been assigned and is waiting for the + // meta table to be updated. - // Queue up an update to note the region location. + pendingRegions.add(region.getRegionName()); - try { - msgQueue.put(new ProcessRegionOpen(info, region)); - } catch (InterruptedException e) { - throw new RuntimeException("Putting into msgQueue was interrupted.", e); - } - } - // Remove from unassigned list so we don't assign it to someone else - this.unassignedRegions.remove(region.getRegionName()); - this.assignAttempts.remove(region.getRegionName()); + // Queue up an update to note the region location. + + try { + msgQueue.put(new ProcessRegionOpen(info, region)); + } catch (InterruptedException e) { + throw new RuntimeException( + "Putting into msgQueue was interrupted.", e); + } + } + } } break; @@ -1565,15 +1578,15 @@ // could create a race with the pending close if it gets // reassigned before the close is processed. - unassignedRegions.remove(region.getRegionName()); - assignAttempts.remove(region.getRegionName()); + unassignedRegions.remove(region); try { msgQueue.put(new ProcessRegionClose(region, reassignRegion, deleteRegion)); } catch (InterruptedException e) { - throw new RuntimeException("Putting into msgQueue was interrupted.", e); + throw new RuntimeException( + "Putting into msgQueue was interrupted.", e); } } break; @@ -1582,12 +1595,10 @@ // A region has split. HRegionInfo newRegionA = incomingMsgs[++i].getRegionInfo(); - unassignedRegions.put(newRegionA.getRegionName(), newRegionA); - assignAttempts.put(newRegionA.getRegionName(), Long.valueOf(0L)); + unassignedRegions.put(newRegionA, ZERO_L); HRegionInfo newRegionB = incomingMsgs[++i].getRegionInfo(); - unassignedRegions.put(newRegionB.getRegionName(), newRegionB); - assignAttempts.put(newRegionB.getRegionName(), Long.valueOf(0L)); + unassignedRegions.put(newRegionB, ZERO_L); LOG.info("region " + region.getRegionName() + " split. New regions are: " + newRegionA.getRegionName() + ", " + @@ -1633,15 +1644,22 @@ private void assignRegions(HServerInfo info, String serverName, ArrayList returnMsgs) { - synchronized (this.assignAttempts) { + synchronized (this.unassignedRegions) { // We need to hold a lock on assign attempts while we figure out what to // do so that multiple threads do not execute this method in parallel // resulting in assigning the same region to multiple servers. long now = System.currentTimeMillis(); - Set regionsToAssign = new HashSet(); - for (Map.Entry e: this.assignAttempts.entrySet()) { + Set regionsToAssign = new HashSet(); + for (Map.Entry e: this.unassignedRegions.entrySet()) { + HRegionInfo i = e.getKey(); + if (numberOfMetaRegions.get() != onlineMetaRegions.size() && + !i.isMetaRegion()) { + // Can't assign user regions until all meta regions have been assigned + // and are on-line + continue; + } long diff = now - e.getValue().longValue(); if (diff > this.maxRegionOpenTime) { regionsToAssign.add(e.getKey()); @@ -1722,11 +1740,10 @@ } now = System.currentTimeMillis(); - for (Text regionName: regionsToAssign) { - HRegionInfo regionInfo = this.unassignedRegions.get(regionName); - LOG.info("assigning region " + regionName + " to server " + - serverName); - this.assignAttempts.put(regionName, Long.valueOf(now)); + for (HRegionInfo regionInfo: regionsToAssign) { + LOG.info("assigning region " + regionInfo.getRegionName() + + " to server " + serverName); + this.unassignedRegions.put(regionInfo, Long.valueOf(now)); returnMsgs.add(new HMsg(HMsg.MSG_REGION_OPEN, regionInfo)); if (--nregions <= 0) { break; @@ -1775,14 +1792,13 @@ * @param serverName * @param returnMsgs */ - private void assignRegionsToOneServer(final Set regionsToAssign, + private void assignRegionsToOneServer(final Set regionsToAssign, final String serverName, final ArrayList returnMsgs) { long now = System.currentTimeMillis(); - for (Text regionName: regionsToAssign) { - HRegionInfo regionInfo = this.unassignedRegions.get(regionName); - LOG.info("assigning region " + regionName + " to the only server " + - serverName); - this.assignAttempts.put(regionName, Long.valueOf(now)); + for (HRegionInfo regionInfo: regionsToAssign) { + LOG.info("assigning region " + regionInfo.getRegionName() + + " to the only server " + serverName); + this.unassignedRegions.put(regionInfo, Long.valueOf(now)); returnMsgs.add(new HMsg(HMsg.MSG_REGION_OPEN, regionInfo)); } } @@ -1868,7 +1884,7 @@ Text regionName) throws IOException { ArrayList toDoList = new ArrayList(); - TreeMap regions = new TreeMap(); + HashSet regions = new HashSet(); try { while (true) { @@ -1960,8 +1976,7 @@ if (regionsToKill.containsKey(info.getRegionName())) { regionsToKill.remove(info.getRegionName()); killList.put(deadServerName, regionsToKill); - unassignedRegions.remove(info.getRegionName()); - assignAttempts.remove(info.getRegionName()); + unassignedRegions.remove(info); synchronized (regionsToDelete) { if (regionsToDelete.contains(info.getRegionName())) { // Delete this region @@ -1976,7 +1991,7 @@ } else { // Get region reassigned - regions.put(info.getRegionName(), info); + regions.add(info); // If it was pending, remove. // Otherwise will obstruct its getting reassigned. @@ -2010,11 +2025,8 @@ } // Get regions reassigned - for (Map.Entry e: regions.entrySet()) { - Text region = e.getKey(); - HRegionInfo regionInfo = e.getValue(); - unassignedRegions.put(region, regionInfo); - assignAttempts.put(region, Long.valueOf(0L)); + for (HRegionInfo info: regions) { + unassignedRegions.put(info, ZERO_L); } } @@ -2300,8 +2312,7 @@ if (reassignRegion) { LOG.info("reassign region: " + regionInfo.getRegionName()); - unassignedRegions.put(regionInfo.getRegionName(), regionInfo); - assignAttempts.put(regionInfo.getRegionName(), Long.valueOf(0L)); + unassignedRegions.put(regionInfo, ZERO_L); } else if (deleteRegion) { try { @@ -2565,8 +2576,7 @@ // 5. Get it assigned to a server - this.unassignedRegions.put(regionName, info); - this.assignAttempts.put(regionName, Long.valueOf(0L)); + this.unassignedRegions.put(info, ZERO_L); } finally { tableInCreation.remove(newRegion.getTableDesc().getName()); @@ -2840,14 +2850,12 @@ } if (online) { // Bring offline regions on-line - if (!unassignedRegions.containsKey(i.getRegionName())) { - unassignedRegions.put(i.getRegionName(), i); - assignAttempts.put(i.getRegionName(), Long.valueOf(0L)); + if (!unassignedRegions.containsKey(i)) { + unassignedRegions.put(i, ZERO_L); } } else { // Prevent region from getting assigned. - unassignedRegions.remove(i.getRegionName()); - assignAttempts.remove(i.getRegionName()); + unassignedRegions.remove(i); } } Index: src/contrib/hbase/src/java/org/apache/hadoop/hbase/HRegionInfo.java =================================================================== --- src/contrib/hbase/src/java/org/apache/hadoop/hbase/HRegionInfo.java (revision 601205) +++ src/contrib/hbase/src/java/org/apache/hadoop/hbase/HRegionInfo.java (working copy) @@ -192,6 +192,11 @@ public HTableDescriptor getTableDesc(){ return tableDesc; } + + /** @return if this region is a meta region */ + public boolean isMetaRegion() { + return this.tableDesc.isMetaTable(); + } /** * @return True if has been split and has daughters.