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/08 02:14:06 UTC
svn commit: r1228759 - in /hbase/branches/0.90: 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: Sun Jan 8 01:14:06 2012
New Revision: 1228759
URL: http://svn.apache.org/viewvc?rev=1228759&view=rev
Log:
HBASE-5088 A concurrency issue on SoftValueSortedMap (Jieshan Bean and Lars H)
Modified:
hbase/branches/0.90/CHANGES.txt
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java
Modified: hbase/branches/0.90/CHANGES.txt
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/CHANGES.txt?rev=1228759&r1=1228758&r2=1228759&view=diff
==============================================================================
--- hbase/branches/0.90/CHANGES.txt (original)
+++ hbase/branches/0.90/CHANGES.txt Sun Jan 8 01:14:06 2012
@@ -12,6 +12,7 @@ Release 0.90.6 - Unreleased
(Ramkrishna)
HBASE-5009 Failure of creating split dir if it already exists prevents splits from happening further
HBASE-5041 Major compaction on non existing table does not throw error (Shrijeet)
+ HBASE-5088 A concurrency issue on SoftValueSortedMap (Jieshan Bean and Lars H)
IMPROVEMENT
HBASE-5102 Change the default value of the property "hbase.connection.per.config" to false in
Modified: hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java?rev=1228759&r1=1228758&r2=1228759&view=diff
==============================================================================
--- hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java (original)
+++ hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/client/HConnectionManager.java Sun Jan 8 01:14:06 2012
@@ -32,6 +32,7 @@ import java.util.Map;
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 @@ public class HConnectionManager {
* 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<Integer, SoftValueSortedMap<byte [], HRegionLocation>>
+ private final Map<Integer, SortedMap<byte [], HRegionLocation>>
cachedRegionLocations =
- new HashMap<Integer, SoftValueSortedMap<byte [], HRegionLocation>>();
+ new HashMap<Integer, SortedMap<byte [], HRegionLocation>>();
// region cache prefetch is enabled by default. this set contains all
// tables whose region cache prefetch are disabled.
@@ -1036,7 +1037,7 @@ public class HConnectionManager {
*/
HRegionLocation getCachedLocation(final byte [] tableName,
final byte [] row) {
- SoftValueSortedMap<byte [], HRegionLocation> tableLocations =
+ SortedMap<byte [], HRegionLocation> tableLocations =
getTableLocations(tableName);
// start to examine the cache. we can only do cache actions
@@ -1052,7 +1053,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,
@@ -1095,7 +1096,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.
@@ -1118,11 +1119,11 @@ public class HConnectionManager {
* @param tableName
* @return Map of cached locations for passed <code>tableName</code>
*/
- private SoftValueSortedMap<byte [], HRegionLocation> getTableLocations(
+ private SortedMap<byte [], HRegionLocation> getTableLocations(
final byte [] tableName) {
// find the map of cached locations for this table
Integer key = Bytes.mapKey(tableName);
- SoftValueSortedMap<byte [], HRegionLocation> result;
+ SortedMap<byte [], HRegionLocation> result;
synchronized (this.cachedRegionLocations) {
result = this.cachedRegionLocations.get(key);
// if tableLocations for this table isn't built yet, make one
@@ -1155,7 +1156,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);
if (tableLocations.put(startKey, location) == null) {
LOG.debug("Cached location for " +
@@ -1483,7 +1484,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/branches/0.90/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java?rev=1228759&r1=1228758&r2=1228759&view=diff
==============================================================================
--- hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java (original)
+++ hbase/branches/0.90/src/main/java/org/apache/hadoop/hbase/util/SoftValueSortedMap.java Sun Jan 8 01:14:06 2012
@@ -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 @@ 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() {
@@ -55,11 +57,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;
}
/**
@@ -68,127 +80,155 @@ 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() {
+ public Set<Map.Entry<K,V>> entrySet() {
throw new RuntimeException("Not implemented");
}
- 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 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> {
final K key;
- SoftValue(K key, V value, ReferenceQueue q) {
+ SoftValue(K key, V value, ReferenceQueue<V> q) {
super(value, q);
this.key = key;
}