Index: src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java (revision 1228758) +++ src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java (working copy) @@ -19,16 +19,17 @@ */ package org.apache.hadoop.hbase.util; +import java.lang.ref.Reference; import java.lang.ref.ReferenceQueue; import java.lang.ref.SoftReference; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.Comparator; import java.util.Map; import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; -import java.util.TreeSet; /** * A SortedMap implementation that uses Soft Reference values @@ -40,7 +41,8 @@ */ public class SoftValueSortedMap implements SortedMap { private final SortedMap> internalMap; - private final ReferenceQueue rq = new ReferenceQueue(); + private final ReferenceQueue rq = new ReferenceQueue(); + private Object sync; /** Constructor */ public SoftValueSortedMap() { @@ -55,11 +57,21 @@ this(new TreeMap>(c)); } - /** For headMap and tailMap support + /** Internal constructor + * @param original object to wrap and synchronize on + */ + private SoftValueSortedMap(SortedMap> original) { + this(original, original); + } + + /** Internal constructor + * For headMap, tailMap, and subMap support * @param original object to wrap + * @param sync object to synchronize on */ - private SoftValueSortedMap(SortedMap> original) { + private SoftValueSortedMap(SortedMap> original, Object sync) { this.internalMap = original; + this.sync = sync; } /** @@ -68,127 +80,155 @@ * Internally these call checkReferences on each access. * @return How many references cleared. */ + @SuppressWarnings("unchecked") private int checkReferences() { int i = 0; - for (Object obj; (obj = this.rq.poll()) != null;) { + for (Reference ref; (ref = this.rq.poll()) != null;) { i++; - //noinspection unchecked - this.internalMap.remove(((SoftValue)obj).key); + this.internalMap.remove(((SoftValue)ref).key); } return i; } - public synchronized V put(K key, V value) { - checkReferences(); - SoftValue oldValue = this.internalMap.put(key, - new SoftValue(key, value, this.rq)); - return oldValue == null ? null : oldValue.get(); + public V put(K key, V value) { + synchronized(sync) { + checkReferences(); + SoftValue oldValue = this.internalMap.put(key, + new SoftValue(key, value, this.rq)); + return oldValue == null ? null : oldValue.get(); + } } - @SuppressWarnings("unchecked") - public synchronized void putAll(Map map) { + @Override + public void putAll(Map m) { throw new RuntimeException("Not implemented"); } - @SuppressWarnings({"SuspiciousMethodCalls"}) - public synchronized V get(Object key) { - checkReferences(); - SoftValue value = this.internalMap.get(key); - if (value == null) { - return null; + public V get(Object key) { + synchronized(sync) { + checkReferences(); + SoftValue value = this.internalMap.get(key); + if (value == null) { + return null; + } + if (value.get() == null) { + this.internalMap.remove(key); + return null; + } + return value.get(); } - if (value.get() == null) { - this.internalMap.remove(key); - return null; - } - return value.get(); } - public synchronized V remove(Object key) { - checkReferences(); - SoftValue value = this.internalMap.remove(key); - return value == null ? null : value.get(); + public V remove(Object key) { + synchronized(sync) { + checkReferences(); + SoftValue value = this.internalMap.remove(key); + return value == null ? null : value.get(); + } } - public synchronized boolean containsKey(Object key) { - checkReferences(); - return this.internalMap.containsKey(key); + public boolean containsKey(Object key) { + synchronized(sync) { + checkReferences(); + return this.internalMap.containsKey(key); + } } - public synchronized boolean containsValue(Object value) { -/* checkReferences(); - return internalMap.containsValue(value);*/ + public boolean containsValue(Object value) { throw new UnsupportedOperationException("Don't support containsValue!"); } - public synchronized K firstKey() { - checkReferences(); - return internalMap.firstKey(); + public K firstKey() { + synchronized(sync) { + checkReferences(); + return internalMap.firstKey(); + } } - public synchronized K lastKey() { - checkReferences(); - return internalMap.lastKey(); + public K lastKey() { + synchronized(sync) { + checkReferences(); + return internalMap.lastKey(); + } } - public synchronized SoftValueSortedMap headMap(K key) { - checkReferences(); - return new SoftValueSortedMap(this.internalMap.headMap(key)); + public SoftValueSortedMap headMap(K key) { + synchronized(sync) { + checkReferences(); + return new SoftValueSortedMap(this.internalMap.headMap(key), sync); + } } - public synchronized SoftValueSortedMap tailMap(K key) { - checkReferences(); - return new SoftValueSortedMap(this.internalMap.tailMap(key)); + public SoftValueSortedMap tailMap(K key) { + synchronized(sync) { + checkReferences(); + return new SoftValueSortedMap(this.internalMap.tailMap(key), sync); + } } - public synchronized SoftValueSortedMap subMap(K fromKey, K toKey) { - checkReferences(); - return new SoftValueSortedMap(this.internalMap.subMap(fromKey, toKey)); + public SoftValueSortedMap subMap(K fromKey, K toKey) { + synchronized(sync) { + checkReferences(); + return new SoftValueSortedMap(this.internalMap.subMap(fromKey, + toKey), sync); + } } - public synchronized boolean isEmpty() { - checkReferences(); - return this.internalMap.isEmpty(); + public boolean isEmpty() { + synchronized(sync) { + checkReferences(); + return this.internalMap.isEmpty(); + } } - public synchronized int size() { - checkReferences(); - return this.internalMap.size(); + public int size() { + synchronized(sync) { + checkReferences(); + return this.internalMap.size(); + } } - public synchronized void clear() { - checkReferences(); - this.internalMap.clear(); + public void clear() { + synchronized(sync) { + checkReferences(); + this.internalMap.clear(); + } } - public synchronized Set keySet() { - checkReferences(); - return this.internalMap.keySet(); + public Set keySet() { + synchronized(sync) { + checkReferences(); + // this is not correct as per SortedMap contract (keySet should be + // modifiable) + // needed here so that another thread cannot modify the keyset + // without locking + return Collections.unmodifiableSet(this.internalMap.keySet()); + } } - @SuppressWarnings("unchecked") - public synchronized Comparator comparator() { + public Comparator comparator() { return this.internalMap.comparator(); } - public synchronized Set> entrySet() { + public Set> entrySet() { throw new RuntimeException("Not implemented"); } - public synchronized Collection values() { - checkReferences(); - Collection> softValues = this.internalMap.values(); - ArrayList hardValues = new ArrayList(); - for(SoftValue softValue : softValues) { - hardValues.add(softValue.get()); + public Collection values() { + synchronized(sync) { + checkReferences(); + ArrayList hardValues = new ArrayList(); + for(SoftValue softValue : this.internalMap.values()) { + hardValues.add(softValue.get()); + } + return hardValues; } - return hardValues; } private static class SoftValue extends SoftReference { final K key; - SoftValue(K key, V value, ReferenceQueue q) { + SoftValue(K key, V value, ReferenceQueue q) { super(value, q); this.key = key; } Index: src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java =================================================================== --- src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java (revision 1228758) +++ src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java (working copy) @@ -32,6 +32,7 @@ import java.util.NoSuchElementException; import java.util.Map.Entry; import java.util.Set; +import java.util.SortedMap; import java.util.TreeSet; import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; @@ -464,9 +465,9 @@ * Map of table to table {@link HRegionLocation}s. The table key is made * by doing a {@link Bytes#mapKey(byte[])} of the table's name. */ - private final Map> + private final Map> cachedRegionLocations = - new HashMap>(); + new HashMap>(); // region cache prefetch is enabled by default. this set contains all // tables whose region cache prefetch are disabled. @@ -1036,7 +1037,7 @@ */ HRegionLocation getCachedLocation(final byte [] tableName, final byte [] row) { - SoftValueSortedMap tableLocations = + SortedMap tableLocations = getTableLocations(tableName); // start to examine the cache. we can only do cache actions @@ -1052,7 +1053,7 @@ // Cut the cache so that we only get the part that could contain // regions that match our key - SoftValueSortedMap matchingRegions = + SortedMap matchingRegions = tableLocations.headMap(row); // if that portion of the map is empty, then we're done. otherwise, @@ -1095,7 +1096,7 @@ */ void deleteCachedLocation(final byte [] tableName, final byte [] row) { synchronized (this.cachedRegionLocations) { - SoftValueSortedMap tableLocations = + Map tableLocations = getTableLocations(tableName); // start to examine the cache. we can only do cache actions // if there's something in the cache for this table. @@ -1118,11 +1119,11 @@ * @param tableName * @return Map of cached locations for passed tableName */ - private SoftValueSortedMap getTableLocations( + private SortedMap getTableLocations( final byte [] tableName) { // find the map of cached locations for this table Integer key = Bytes.mapKey(tableName); - SoftValueSortedMap result; + SortedMap result; synchronized (this.cachedRegionLocations) { result = this.cachedRegionLocations.get(key); // if tableLocations for this table isn't built yet, make one @@ -1155,7 +1156,7 @@ private void cacheLocation(final byte [] tableName, final HRegionLocation location) { byte [] startKey = location.getRegionInfo().getStartKey(); - SoftValueSortedMap tableLocations = + Map tableLocations = getTableLocations(tableName); if (tableLocations.put(startKey, location) == null) { LOG.debug("Cached location for " + @@ -1483,7 +1484,7 @@ int getNumberOfCachedRegionLocations(final byte[] tableName) { Integer key = Bytes.mapKey(tableName); synchronized (this.cachedRegionLocations) { - SoftValueSortedMap tableLocs = + Map tableLocs = this.cachedRegionLocations.get(key); if (tableLocs == null) {