You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by md...@apache.org on 2018/08/09 14:29:34 UTC

hbase git commit: HBASE-21027 Inconsistent synchronization in CacheableDeserializerIdManager

Repository: hbase
Updated Branches:
  refs/heads/master e2fcde2d6 -> c6ff1de7e


HBASE-21027 Inconsistent synchronization in CacheableDeserializerIdManager

Signed-off-by: Sean Busbey <bu...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/c6ff1de7
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/c6ff1de7
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/c6ff1de7

Branch: refs/heads/master
Commit: c6ff1de7e2cc08c40785780a4acd65097c8281d9
Parents: e2fcde2
Author: Mike Drob <md...@apache.org>
Authored: Wed Aug 8 14:31:07 2018 -0500
Committer: Mike Drob <md...@apache.org>
Committed: Thu Aug 9 09:29:21 2018 -0500

----------------------------------------------------------------------
 .../hfile/CacheableDeserializerIdManager.java   | 24 +++++++++-----------
 1 file changed, 11 insertions(+), 13 deletions(-)
----------------------------------------------------------------------


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