You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by la...@apache.org on 2012/01/07 02:40:07 UTC
svn commit: r1228539 - in /hbase/trunk: CHANGES.txt
src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java
Author: larsh
Date: Sat Jan 7 01:40:07 2012
New Revision: 1228539
URL: http://svn.apache.org/viewvc?rev=1228539&view=rev
Log:
HBASE-5088 A concurrency issue on SoftValueSortedMap (Jieshan Bean and Lars H)
Modified:
hbase/trunk/CHANGES.txt
hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java
Modified: hbase/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hbase/trunk/CHANGES.txt?rev=1228539&r1=1228538&r2=1228539&view=diff
==============================================================================
--- hbase/trunk/CHANGES.txt (original)
+++ hbase/trunk/CHANGES.txt Sat Jan 7 01:40:07 2012
@@ -472,6 +472,7 @@ Release 0.92.0 - Unreleased
actually in the AssignmentManager thus making the region inaccessible. (Ram)
HBASE-5081 Distributed log splitting deleteNode races against splitLog retry (Prakash)
HBASE-4357 Region stayed in transition - in closing state (Ming Ma)
+ HBASE-5088 A concurrency issue on SoftValueSortedMap (Jieshan Bean and Lars H)
TESTS
HBASE-4450 test for number of blocks read: to serve as baseline for expected
Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java?rev=1228539&r1=1228538&r2=1228539&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java Sat Jan 7 01:40:07 2012
@@ -36,6 +36,7 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.NoSuchElementException;
import java.util.Set;
+import java.util.SortedMap;
import java.util.TreeMap;
import java.util.concurrent.Callable;
import java.util.concurrent.ConcurrentHashMap;
@@ -1084,7 +1085,7 @@ public class HConnectionManager {
// Cut the cache so that we only get the part that could contain
// regions that match our key
- SoftValueSortedMap<byte[], HRegionLocation> matchingRegions =
+ SortedMap<byte[], HRegionLocation> matchingRegions =
tableLocations.headMap(row);
// if that portion of the map is empty, then we're done. otherwise,
@@ -1127,7 +1128,7 @@ public class HConnectionManager {
*/
void deleteCachedLocation(final byte [] tableName, final byte [] row) {
synchronized (this.cachedRegionLocations) {
- SoftValueSortedMap<byte [], HRegionLocation> tableLocations =
+ Map<byte[], HRegionLocation> tableLocations =
getTableLocations(tableName);
// start to examine the cache. we can only do cache actions
// if there's something in the cache for this table.
@@ -1159,8 +1160,8 @@ public class HConnectionManager {
if (!cachedServers.contains(server)) {
return;
}
- for (SoftValueSortedMap<byte[], HRegionLocation> tableLocations :
- cachedRegionLocations.values()) {
+ for (Map<byte[], HRegionLocation> tableLocations :
+ cachedRegionLocations.values()) {
for (Entry<byte[], HRegionLocation> e : tableLocations.entrySet()) {
if (e.getValue().getServerAddress().toString().equals(server)) {
tableLocations.remove(e.getKey());
@@ -1217,7 +1218,7 @@ public class HConnectionManager {
private void cacheLocation(final byte [] tableName,
final HRegionLocation location) {
byte [] startKey = location.getRegionInfo().getStartKey();
- SoftValueSortedMap<byte [], HRegionLocation> tableLocations =
+ Map<byte [], HRegionLocation> tableLocations =
getTableLocations(tableName);
boolean hasNewCache = false;
synchronized (this.cachedRegionLocations) {
@@ -1685,7 +1686,7 @@ public class HConnectionManager {
int getNumberOfCachedRegionLocations(final byte[] tableName) {
Integer key = Bytes.mapKey(tableName);
synchronized (this.cachedRegionLocations) {
- SoftValueSortedMap<byte[], HRegionLocation> tableLocs =
+ Map<byte[], HRegionLocation> tableLocs =
this.cachedRegionLocations.get(key);
if (tableLocs == null) {
Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java?rev=1228539&r1=1228538&r2=1228539&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java Sat Jan 7 01:40:07 2012
@@ -19,17 +19,18 @@
*/
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.LinkedHashSet;
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
@@ -41,7 +42,8 @@ import java.util.TreeSet;
*/
public class SoftValueSortedMap<K,V> implements SortedMap<K,V> {
private final SortedMap<K, SoftValue<K,V>> internalMap;
- private final ReferenceQueue rq = new ReferenceQueue();
+ private final ReferenceQueue<V> rq = new ReferenceQueue<V>();
+ private Object sync;
/** Constructor */
public SoftValueSortedMap() {
@@ -56,11 +58,21 @@ public class SoftValueSortedMap<K,V> imp
this(new TreeMap<K, SoftValue<K,V>>(c));
}
- /** For headMap and tailMap support
- * @param original object to wrap
+ /** Internal constructor
+ * @param original object to wrap and synchronize on
*/
private SoftValueSortedMap(SortedMap<K,SoftValue<K,V>> 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<K,SoftValue<K,V>> original, Object sync) {
this.internalMap = original;
+ this.sync = sync;
}
/**
@@ -69,133 +81,164 @@ public class SoftValueSortedMap<K,V> imp
* 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<? extends V> ref; (ref = this.rq.poll()) != null;) {
i++;
- //noinspection unchecked
- this.internalMap.remove(((SoftValue<K,V>)obj).key);
+ this.internalMap.remove(((SoftValue<K,V>)ref).key);
}
return i;
}
- public synchronized V put(K key, V value) {
- checkReferences();
- SoftValue<K,V> oldValue = this.internalMap.put(key,
- new SoftValue<K,V>(key, value, this.rq));
- return oldValue == null ? null : oldValue.get();
+ public V put(K key, V value) {
+ synchronized(sync) {
+ checkReferences();
+ SoftValue<K,V> oldValue = this.internalMap.put(key,
+ new SoftValue<K,V>(key, value, this.rq));
+ return oldValue == null ? null : oldValue.get();
+ }
}
- @SuppressWarnings("unchecked")
- public synchronized void putAll(Map map) {
+ @Override
+ public void putAll(Map<? extends K, ? extends V> m) {
throw new RuntimeException("Not implemented");
}
- @SuppressWarnings({"SuspiciousMethodCalls"})
- public synchronized V get(Object key) {
- checkReferences();
- SoftValue<K,V> value = this.internalMap.get(key);
- if (value == null) {
- return null;
- }
- if (value.get() == null) {
- this.internalMap.remove(key);
- return null;
+ public V get(Object key) {
+ synchronized (sync) {
+ checkReferences();
+ SoftValue<K, V> value = this.internalMap.get(key);
+ if (value == null) {
+ return null;
+ }
+ if (value.get() == null) {
+ this.internalMap.remove(key);
+ return null;
+ }
+ return value.get();
}
- return value.get();
}
- public synchronized V remove(Object key) {
- checkReferences();
- SoftValue<K,V> value = this.internalMap.remove(key);
- return value == null ? null : value.get();
+ public V remove(Object key) {
+ synchronized (sync) {
+ checkReferences();
+ SoftValue<K, V> 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<K,V> headMap(K key) {
- checkReferences();
- return new SoftValueSortedMap<K,V>(this.internalMap.headMap(key));
+ public SoftValueSortedMap<K,V> headMap(K key) {
+ synchronized(sync) {
+ checkReferences();
+ return new SoftValueSortedMap<K,V>(this.internalMap.headMap(key), sync);
+ }
}
- public synchronized SoftValueSortedMap<K,V> tailMap(K key) {
- checkReferences();
- return new SoftValueSortedMap<K,V>(this.internalMap.tailMap(key));
+ public SoftValueSortedMap<K,V> tailMap(K key) {
+ synchronized(sync) {
+ checkReferences();
+ return new SoftValueSortedMap<K,V>(this.internalMap.tailMap(key), sync);
+ }
}
- public synchronized SoftValueSortedMap<K,V> subMap(K fromKey, K toKey) {
- checkReferences();
- return new SoftValueSortedMap<K,V>(this.internalMap.subMap(fromKey, toKey));
+ public SoftValueSortedMap<K,V> subMap(K fromKey, K toKey) {
+ synchronized(sync) {
+ checkReferences();
+ return new SoftValueSortedMap<K,V>(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<K> keySet() {
- checkReferences();
- return this.internalMap.keySet();
+ public Set<K> 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<? super K> comparator() {
return this.internalMap.comparator();
}
- public synchronized Set<Map.Entry<K,V>> entrySet() {
- checkReferences();
- Set<Map.Entry<K, SoftValue<K, V>>> entries = this.internalMap.entrySet();
- Set<Map.Entry<K, V>> realEntries = new LinkedHashSet<Map.Entry<K, V>>();
- for (Map.Entry<K, SoftValue<K, V>> entry : entries) {
- realEntries.add(entry.getValue());
- }
- return realEntries;
- }
-
- public synchronized Collection<V> values() {
- checkReferences();
- Collection<SoftValue<K,V>> softValues = this.internalMap.values();
- ArrayList<V> hardValues = new ArrayList<V>();
- for(SoftValue<K,V> softValue : softValues) {
- hardValues.add(softValue.get());
+ public Set<Map.Entry<K,V>> entrySet() {
+ synchronized(sync) {
+ checkReferences();
+ // this is not correct as per SortedMap contract (entrySet should be
+ // backed by map)
+ Set<Map.Entry<K, V>> realEntries = new LinkedHashSet<Map.Entry<K, V>>();
+ for (Map.Entry<K, SoftValue<K, V>> entry : this.internalMap.entrySet()) {
+ realEntries.add(entry.getValue());
+ }
+ return realEntries;
+ }
+ }
+
+ public Collection<V> values() {
+ synchronized (sync) {
+ checkReferences();
+ ArrayList<V> hardValues = new ArrayList<V>();
+ for (SoftValue<K, V> softValue : this.internalMap.values()) {
+ hardValues.add(softValue.get());
+ }
+ return hardValues;
}
- return hardValues;
}
private static class SoftValue<K,V> extends SoftReference<V> implements Map.Entry<K, V> {
final K key;
- SoftValue(K key, V value, ReferenceQueue q) {
+ SoftValue(K key, V value, ReferenceQueue<V> q) {
super(value, q);
this.key = key;
}