From e0ff9648ae38b99f4c59c9e6462a70bdce4237e2 Mon Sep 17 00:00:00 2001
From: Matt Ward <matt.ward@alfresco.com>
Date: Thu, 26 Sep 2013 16:29:02 +0000
Subject: [PATCH] Removed concurrency lock methods from LockStore and
 implementations (ALF-20031)

git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@56052 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
---
 .../repo/lock/mem/AbstractLockStore.java      | 22 ---------
 .../org/alfresco/repo/lock/mem/LockStore.java | 24 +---------
 .../alfresco/repo/lock/mem/LockStoreImpl.java | 47 -------------------
 .../lock/mem/AbstractLockStoreTestBase.java   | 42 -----------------
 4 files changed, 1 insertion(+), 134 deletions(-)

diff --git a/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java b/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java
index 119c2a8442..e842cdf19d 100644
--- a/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java
+++ b/source/java/org/alfresco/repo/lock/mem/AbstractLockStore.java
@@ -39,27 +39,12 @@ import org.springframework.transaction.support.TransactionSynchronizationManager
  */
 public abstract class AbstractLockStore<T extends ConcurrentMap<NodeRef, LockState>> implements LockStore
 {
-    protected long maxTryLockMillis = 100;
     protected T map;
     
     public AbstractLockStore(T map)
     {
         this.map = map;
     }
-    
-    /**
-     * Set the maximum time a lock store should wait while trying to acquire a concurrency lock.
-     * 
-     * @see #acquireConcurrencyLock(NodeRef)
-     * @param maxTryLockMillis
-     */
-    @Override
-    public void setMaxTryLockMillis(long maxTryLockMillis)
-    {
-        this.maxTryLockMillis = maxTryLockMillis;
-    }
-
-
 
     @Override
     public LockState get(NodeRef nodeRef)
@@ -161,13 +146,6 @@ public abstract class AbstractLockStore<T extends ConcurrentMap<NodeRef, LockSta
         }
     }
 
-    @Override
-    public abstract void acquireConcurrencyLock(NodeRef nodeRef);
-
-    @Override
-    public abstract void releaseConcurrencyLock(NodeRef nodeRef);
-
-
     /**
      * Returns a transactionally scoped Map that is used to provide repeatable lock store queries
      * for a given NodeRef. If no transaction is present, then null is returned.
diff --git a/source/java/org/alfresco/repo/lock/mem/LockStore.java b/source/java/org/alfresco/repo/lock/mem/LockStore.java
index 44718c6948..6dd334656b 100644
--- a/source/java/org/alfresco/repo/lock/mem/LockStore.java
+++ b/source/java/org/alfresco/repo/lock/mem/LockStore.java
@@ -25,26 +25,7 @@ import org.alfresco.service.cmr.repository.NodeRef;
 /**
  * Defines the in-memory lock storage interface.
  * <p>
- * Individual operations MUST be thread-safe, however clients are expected to synchronise
- * compound operations using {@link #acquireConcurrencyLock(NodeRef)} and
- * {@link #releaseConcurrencyLock(NodeRef)}, for example:
- * <pre>
- *    acquireConcurrencyLock(nodeRef);
- *    try
- *    {
- *       if (lockStore.contains(nodeRef))
- *       {
- *          if (someOtherCondition())
- *          {
- *             lockStore.setUnlocked(nodeRef);
- *          }
- *       }
- *    }
- *    finally
- *    {
- *       releaseConcurrencyLock(nodeRef);
- *    }
- * </pre>
+ * Operations MUST be thread-safe.
  * 
  * @author Matt Ward
  */
@@ -52,9 +33,6 @@ public interface LockStore
 {
     LockState get(NodeRef nodeRef);
     void set(NodeRef nodeRef, LockState lockState);
-    void acquireConcurrencyLock(NodeRef nodeRef);
-    void releaseConcurrencyLock(NodeRef nodeRef);
-    void setMaxTryLockMillis(long maxTryLockMillis);
     public Set<NodeRef> getNodes();
     
     /**
diff --git a/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java b/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java
index 3026ab191d..8819f11e75 100644
--- a/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java
+++ b/source/java/org/alfresco/repo/lock/mem/LockStoreImpl.java
@@ -20,12 +20,9 @@ package org.alfresco.repo.lock.mem;
 
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock;
 
 import org.alfresco.repo.lock.LockServiceImpl;
 import org.alfresco.service.cmr.repository.NodeRef;
-import org.alfresco.util.LockHelper;
 
 import com.google.common.collect.MapMaker;
 
@@ -36,20 +33,6 @@ import com.google.common.collect.MapMaker;
  */
 public class LockStoreImpl extends AbstractLockStore<ConcurrentMap<NodeRef, LockState>>
 {
-    /**
-     * Locks to provide atomicity for compound operations - ALWAYS use a power of 2 for the number of locks
-     * to avoid hideously unbalanced use of the locks.
-     */
-    private static final ReentrantReadWriteLock[] concurrencyLocks = new ReentrantReadWriteLock[256];
-    
-    static
-    {
-        for (int i = 0; i < concurrencyLocks.length; i++)
-        {
-            concurrencyLocks[i] = new ReentrantReadWriteLock();
-        }
-    }
-    
     /**
      * Default constructor.
      */
@@ -76,34 +59,4 @@ public class LockStoreImpl extends AbstractLockStore<ConcurrentMap<NodeRef, Lock
                     .makeMap();
         return map;
     }
-
-    @Override
-    public void acquireConcurrencyLock(NodeRef nodeRef)
-    {
-        WriteLock writeLock = getWriteLock(nodeRef);
-        // Block for a short time, if we can't acquire the lock, then throw
-        // an exception that the will allow transaction retry behaviour.
-        LockHelper.tryLock(writeLock, maxTryLockMillis);
-    }
-
-    @Override
-    public void releaseConcurrencyLock(NodeRef nodeRef)
-    {
-        WriteLock writeLock = getWriteLock(nodeRef);
-        writeLock.unlock();
-    }
-
-    private int concurrencyLockIndex(NodeRef nodeRef)
-    {
-        int lockIndex = nodeRef.hashCode() & (concurrencyLocks.length - 1);
-        return lockIndex;
-    }
-    
-    private WriteLock getWriteLock(NodeRef nodeRef)
-    {
-        int lockIndex = concurrencyLockIndex(nodeRef);
-        ReentrantReadWriteLock rwLock = concurrencyLocks[lockIndex];
-        WriteLock writeLock = rwLock.writeLock();
-        return writeLock;
-    }
 }
diff --git a/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java b/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java
index b71d5e6ef4..9a05213937 100644
--- a/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java
+++ b/source/test-java/org/alfresco/repo/lock/mem/AbstractLockStoreTestBase.java
@@ -154,46 +154,4 @@ public abstract class AbstractLockStoreTestBase<T extends LockStore>
         assertEquals(nodeRef3, it.next());
         assertEquals(nodeRef4, it.next());
     }
-    
-    @Test()
-    public void whenTryLockFailsTransactionWillRetry() throws InterruptedException
-    {
-        final NodeRef nodeRef = new NodeRef("workspace://SpacesStore/whenTryLockFailsTransactionWillRetry");
-        
-        // The "other" thread, that got there first and took the lock.
-        Thread thread = new Thread(getClass().getSimpleName()+"-testTryLock")
-        {
-            @Override
-            public void run()
-            {
-                try
-                {
-                    // Deliberately NOT releasing this lock, so we can check for tryLock failure.
-                    lockStore.acquireConcurrencyLock(nodeRef);
-                }
-                catch (Throwable e)
-                {
-                    // This shouldn't happen the first time.
-                    fail("Failed to a acquire lock");
-                }
-            }
-        };
-        thread.start();
-        
-        
-        // Wait for the other thread to perform the lock.
-        thread.join();
-        
-        // Second attempt will fail (already locked above), expected exception
-        // must be thrown for retrying behaviour to work correctly.
-        try
-        {
-            lockStore.acquireConcurrencyLock(nodeRef);
-            fail("Exception was not thrown when unable to acquire lock.");
-        }
-        catch (LockTryException e)
-        {
-            // Good, we got the correct exception.
-        }
-    }
 }