[GitHub] [accumulo] belugabehr commented on a change in pull request #1357: Use ConcurrentHashMap instead of synchronized blocks

[GitBox <gi...@apache.org>]

belugabehr commented on a change in pull request #1357: Use ConcurrentHashMap instead of synchronized blocks
URL: https://github.com/apache/accumulo/pull/1357#discussion_r323887724
 
 

 ##########
 File path: server/tserver/src/main/java/org/apache/accumulo/tserver/RowLocks.java
 ##########
 @@ -29,18 +31,20 @@
 import org.apache.accumulo.tserver.ConditionalMutationSet.DeferFilter;
 import org.apache.accumulo.tserver.data.ServerConditionalMutation;
 
+import com.google.common.base.Preconditions;
+
 class RowLocks {
 
-  private Map<ByteSequence,RowLock> rowLocks = new HashMap<>();
+  private final Map<ByteSequence,RowLock> rowLocks = new ConcurrentHashMap<>();
 
   static class RowLock {
     ReentrantLock rlock;
-    int count;
+    AtomicInteger count;
     ByteSequence rowSeq;
 
     RowLock(ReentrantLock rlock, ByteSequence rowSeq) {
       this.rlock = rlock;
-      this.count = 0;
+      this.count = new AtomicInteger(1);
 
 Review comment:
   The reason I did this is to ensure that multiple threads see the same count.  With the locking of the Concurrent Map, changes are protected from colliding, but it could be the case that different threads will cache the value and therefore see different values.  I could have also marked this variable transient.
   
   EDIT: Don't know what came over me.  None of that is correct.  I changed it back to primitive values.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services