HBASE-21027 Inconsistent synchronization in CacheableDeserializerIdManager

Signed-off-by: Sean Busbey <busbey@apache.org>
This commit is contained in:
Mike Drob 2018-08-08 14:31:07 -05:00
parent e2fcde2d6f
commit c6ff1de7e2
No known key found for this signature in database
GPG Key ID: 3E48C0C6EF362B9E
1 changed files with 11 additions and 13 deletions

View File

@ -18,9 +18,10 @@
*/ */
package org.apache.hadoop.hbase.io.hfile; package org.apache.hadoop.hbase.io.hfile;
import java.util.HashMap;
import java.util.Map; import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import org.apache.yetus.audience.InterfaceAudience; import org.apache.yetus.audience.InterfaceAudience;
@ -33,7 +34,8 @@ import org.apache.yetus.audience.InterfaceAudience;
*/ */
@InterfaceAudience.Private @InterfaceAudience.Private
public class CacheableDeserializerIdManager { public class CacheableDeserializerIdManager {
private static final Map<Integer, CacheableDeserializer<Cacheable>> registeredDeserializers = new HashMap<>(); private static final Map<Integer, CacheableDeserializer<Cacheable>> registeredDeserializers =
new ConcurrentHashMap<>();
private static final AtomicInteger identifier = new AtomicInteger(0); private static final AtomicInteger identifier = new AtomicInteger(0);
/** /**
@ -45,9 +47,8 @@ public class CacheableDeserializerIdManager {
*/ */
public static int registerDeserializer(CacheableDeserializer<Cacheable> cd) { public static int registerDeserializer(CacheableDeserializer<Cacheable> cd) {
int idx = identifier.incrementAndGet(); int idx = identifier.incrementAndGet();
synchronized (registeredDeserializers) { // No synchronization here because keys will be unique
registeredDeserializers.put(idx, cd); registeredDeserializers.put(idx, cd);
}
return idx; return idx;
} }
@ -64,13 +65,10 @@ public class CacheableDeserializerIdManager {
* of a file. * of a file.
*/ */
public static Map<Integer,String> save() { public static Map<Integer,String> save() {
Map<Integer, String> snapshot = new HashMap<>(); // No synchronization here because weakly consistent view should be good enough
synchronized (registeredDeserializers) { // The assumed risk is that we might not see a new serializer that comes in while iterating,
for (Map.Entry<Integer, CacheableDeserializer<Cacheable>> entry : // but with a synchronized block, we won't see it anyway
registeredDeserializers.entrySet()) { return registeredDeserializers.entrySet().stream()
snapshot.put(entry.getKey(), entry.getValue().getClass().getName()); .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue().getClass().getName()));
}
}
return snapshot;
} }
} }