diff --git a/CHANGES.txt b/CHANGES.txt index 4042d9c93b2..9b105a915c8 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -43,6 +43,7 @@ Release 0.91.0 - Unreleased creating scanner HBASE-3641 LruBlockCache.CacheStats.getHitCount() is not using the correct variable + HBASE-3532 HRegion#equals is broken (Ted Yu via Stack) IMPROVEMENTS HBASE-3290 Max Compaction Size (Nicolas Spiegelberg via Stack) diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 21df6b3b72a..3549c255989 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -2357,7 +2357,7 @@ public class HRegion implements HeapSize { // , Writable{ if (!(o instanceof HRegion)) { return false; } - return this.hashCode() == ((HRegion)o).hashCode(); + return Bytes.equals(this.regionInfo.getRegionName(), ((HRegion)o).regionInfo.getRegionName()); } @Override diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index 1430ffab0e9..b894f35c51e 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -2410,31 +2410,6 @@ public class HRegionServer implements HRegionInterface, HBaseRPCErrorHandler, } } - /** - * @return Returns list of non-closed regions hosted on this server. If no - * regions to check, returns an empty list. - */ - protected Set getRegionsToCheck() { - HashSet regionsToCheck = new HashSet(); - // TODO: is this locking necessary? - lock.readLock().lock(); - try { - synchronized (this.onlineRegions) { - regionsToCheck.addAll(this.onlineRegions.values()); - } - } finally { - lock.readLock().unlock(); - } - // Purge closed regions. - for (final Iterator i = regionsToCheck.iterator(); i.hasNext();) { - HRegion r = i.next(); - if (r.isClosed()) { - i.remove(); - } - } - return regionsToCheck; - } - @Override @QosPriority(priority=HIGH_QOS) public long getProtocolVersion(final String protocol, final long clientVersion) diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java b/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java index 099ff4beaca..4b4c331cd01 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/MemStoreFlusher.java @@ -19,21 +19,9 @@ */ package org.apache.hadoop.hbase.regionserver; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; -import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.hbase.DroppedSnapshotException; -import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.RemoteExceptionHandler; -import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.util.StringUtils; - -import com.google.common.base.Preconditions; - import java.io.IOException; import java.lang.management.ManagementFactory; import java.util.ConcurrentModificationException; -import java.util.EnumSet; import java.util.HashMap; import java.util.Map; import java.util.Set; @@ -47,6 +35,17 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.DroppedSnapshotException; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.RemoteExceptionHandler; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.util.StringUtils; + +import com.google.common.base.Preconditions; + /** * Thread that flushes cache on request * @@ -152,19 +151,17 @@ class MemStoreFlusher extends Thread implements FlushRequester { SortedMap regionsBySize = server.getCopyOfOnlineRegionsSortedBySize(); - // TODO: HBASE-3532 - we can't use Set here because it doesn't - // implement equals correctly. So, set of region names. - Set excludedRegionNames = new TreeSet(Bytes.BYTES_COMPARATOR); + Set excludedRegions = new TreeSet(); boolean flushedOne = false; while (!flushedOne) { // Find the biggest region that doesn't have too many storefiles // (might be null!) HRegion bestFlushableRegion = getBiggestMemstoreRegion( - regionsBySize, excludedRegionNames, true); + regionsBySize, excludedRegions, true); // Find the biggest region, total, even if it might have too many flushes. HRegion bestAnyRegion = getBiggestMemstoreRegion( - regionsBySize, excludedRegionNames, false); + regionsBySize, excludedRegions, false); if (bestAnyRegion == null) { LOG.error("Above memory mark but there are no flushable regions!"); @@ -201,7 +198,7 @@ class MemStoreFlusher extends Thread implements FlushRequester { if (!flushedOne) { LOG.info("Excluding unflushable region " + regionToFlush + " - trying to find a different region to flush."); - excludedRegionNames.add(regionToFlush.getRegionName()); + excludedRegions.add(regionToFlush); } } return true; @@ -272,11 +269,11 @@ class MemStoreFlusher extends Thread implements FlushRequester { private HRegion getBiggestMemstoreRegion( SortedMap regionsBySize, - Set excludedRegionNames, + Set excludedRegions, boolean checkStoreFileCount) { synchronized (regionsInQueue) { for (HRegion region : regionsBySize.values()) { - if (excludedRegionNames.contains(region.getRegionName())) { + if (excludedRegions.contains(region)) { continue; }