You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@directory.apache.org by el...@apache.org on 2013/06/20 01:28:49 UTC

svn commit: r1494803 - in /directory/apacheds/trunk: core-api/src/main/java/org/apache/directory/server/core/api/ core-api/src/test/java/org/apache/directory/server/core/api/ core-integ/src/test/java/org/apache/directory/server/core/operations/add/ cor...

Author: elecharny
Date: Wed Jun 19 23:28:48 2013
New Revision: 1494803

URL: http://svn.apache.org/r1494803
Log:
o Exposed the OperationManager RW lock so that we can use it when processing a Search cursor, to avoid breaking the backend when running a search while modifying some entries (fix for DIRSERVER-1862)

Modified:
    directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/api/OperationManager.java
    directory/apacheds/trunk/core-api/src/test/java/org/apache/directory/server/core/api/MockOperationManager.java
    directory/apacheds/trunk/core-integ/src/test/java/org/apache/directory/server/core/operations/add/ConcurrentAddSearchIT.java
    directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultOperationManager.java
    directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/core/partition/impl/btree/AbstractBTreePartition.java
    directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/xdbm/Store.java

Modified: directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/api/OperationManager.java
URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/api/OperationManager.java?rev=1494803&r1=1494802&r2=1494803&view=diff
==============================================================================
--- directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/api/OperationManager.java (original)
+++ directory/apacheds/trunk/core-api/src/main/java/org/apache/directory/server/core/api/OperationManager.java Wed Jun 19 23:28:48 2013
@@ -20,6 +20,8 @@
 package org.apache.directory.server.core.api;
 
 
+import java.util.concurrent.locks.ReadWriteLock;
+
 import org.apache.directory.api.ldap.model.entry.Entry;
 import org.apache.directory.api.ldap.model.exception.LdapException;
 import org.apache.directory.server.core.api.filtering.EntryFilteringCursor;
@@ -141,4 +143,22 @@ public interface OperationManager
      * Releases a WriteLock
      */
     void unlockWrite();
+
+
+    /**
+     * Acquires a ReadLock
+     */
+    void lockRead();
+
+
+    /**
+     * Releases a ReadLock
+     */
+    void unlockRead();
+
+
+    /**
+     * @return the OperationManager R/W lock
+     */
+    ReadWriteLock getRWLock();
 }

Modified: directory/apacheds/trunk/core-api/src/test/java/org/apache/directory/server/core/api/MockOperationManager.java
URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/core-api/src/test/java/org/apache/directory/server/core/api/MockOperationManager.java?rev=1494803&r1=1494802&r2=1494803&view=diff
==============================================================================
--- directory/apacheds/trunk/core-api/src/test/java/org/apache/directory/server/core/api/MockOperationManager.java (original)
+++ directory/apacheds/trunk/core-api/src/test/java/org/apache/directory/server/core/api/MockOperationManager.java Wed Jun 19 23:28:48 2013
@@ -20,6 +20,9 @@
 package org.apache.directory.server.core.api;
 
 
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
 import org.apache.directory.api.ldap.model.entry.Entry;
 import org.apache.directory.api.ldap.model.exception.LdapException;
 import org.apache.directory.server.core.api.filtering.BaseEntryFilteringCursor;
@@ -130,4 +133,25 @@ public class MockOperationManager implem
     public void unlockWrite()
     {
     }
+
+
+    @Override
+    public void lockRead()
+    {
+    }
+
+
+    @Override
+    public void unlockRead()
+    {
+    }
+
+
+    /**
+     * {@inheritDoc}
+     */
+    public ReadWriteLock getRWLock()
+    {
+        return new ReentrantReadWriteLock();
+    }
 }

Modified: directory/apacheds/trunk/core-integ/src/test/java/org/apache/directory/server/core/operations/add/ConcurrentAddSearchIT.java
URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/core-integ/src/test/java/org/apache/directory/server/core/operations/add/ConcurrentAddSearchIT.java?rev=1494803&r1=1494802&r2=1494803&view=diff
==============================================================================
--- directory/apacheds/trunk/core-integ/src/test/java/org/apache/directory/server/core/operations/add/ConcurrentAddSearchIT.java (original)
+++ directory/apacheds/trunk/core-integ/src/test/java/org/apache/directory/server/core/operations/add/ConcurrentAddSearchIT.java Wed Jun 19 23:28:48 2013
@@ -127,7 +127,7 @@ public class ConcurrentAddSearchIT exten
 
         // Now that we have started the add thread, let's do some searches
 
-        for ( int i = 0; i < 120; i++ )
+        for ( int i = 0; i < 100; i++ )
         {
             try
             {
@@ -135,15 +135,22 @@ public class ConcurrentAddSearchIT exten
 
                 int nbFound = 0;
 
-                while ( results.next() )
+                while ( results.next() && ( nbFound < 1000 ) )
                 {
                     Entry result = results.get();
                     nbFound++;
                 }
 
-                System.out.println( "Running " + i + "th search, getting back " + nbFound + " entries" );
+                System.out.println( "Running " + i + "th search, getting back " + nbFound
+                    + " entries, nb added entries : " + nbAdded );
 
                 results.close();
+
+                if ( nbAdded >= 10000 )
+                {
+                    break;
+                }
+
                 Thread.sleep( 1000 );
             }
             catch ( Exception e )

Modified: directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultOperationManager.java
URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultOperationManager.java?rev=1494803&r1=1494802&r2=1494803&view=diff
==============================================================================
--- directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultOperationManager.java (original)
+++ directory/apacheds/trunk/core/src/main/java/org/apache/directory/server/core/DefaultOperationManager.java Wed Jun 19 23:28:48 2013
@@ -102,9 +102,18 @@ public class DefaultOperationManager imp
 
 
     /**
+     * {@inheritDoc}
+     */
+    public ReadWriteLock getRWLock()
+    {
+        return rwLock;
+    }
+
+
+    /**
      * Acquires a ReadLock
      */
-    private void lockRead()
+    public void lockRead()
     {
         rwLock.readLock().lock();
     }
@@ -131,7 +140,7 @@ public class DefaultOperationManager imp
     /**
      * Releases a ReadLock
      */
-    private void unlockRead()
+    public void unlockRead()
     {
         rwLock.readLock().unlock();
     }

Modified: directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/core/partition/impl/btree/AbstractBTreePartition.java
URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/core/partition/impl/btree/AbstractBTreePartition.java?rev=1494803&r1=1494802&r2=1494803&view=diff
==============================================================================
--- directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/core/partition/impl/btree/AbstractBTreePartition.java (original)
+++ directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/core/partition/impl/btree/AbstractBTreePartition.java Wed Jun 19 23:28:48 2013
@@ -173,8 +173,8 @@ public abstract class AbstractBTreeParti
     protected static final boolean ADD_CHILD = true;
     protected static final boolean REMOVE_CHILD = false;
 
-    /** A lock to protect the MasterTable from concurrent access */
-    private ReadWriteLock masterTableLock = new ReentrantReadWriteLock();
+    /** A lock to protect the backend from concurrent reads/writes */
+    private ReadWriteLock rwLock;
 
 
     // ------------------------------------------------------------------------
@@ -598,6 +598,7 @@ public abstract class AbstractBTreeParti
     {
         try
         {
+            setRWLock( addContext );
             Entry entry = ( ( ClonedServerEntry ) addContext.getEntry() ).getClonedEntry();
 
             Dn entryDn = entry.getDn();
@@ -802,6 +803,7 @@ public abstract class AbstractBTreeParti
     {
         try
         {
+            setRWLock( deleteContext );
             Dn dn = deleteContext.getDn();
             String id = null;
 
@@ -1021,6 +1023,7 @@ public abstract class AbstractBTreeParti
     {
         try
         {
+            setRWLock( searchContext );
 
             PartitionSearchResult searchResult = searchEngine.computeResult( schemaManager, searchContext );
 
@@ -1048,6 +1051,7 @@ public abstract class AbstractBTreeParti
      */
     public Entry lookup( LookupOperationContext lookupContext ) throws LdapException
     {
+        setRWLock( lookupContext );
         String id = getEntryId( lookupContext.getDn() );
 
         if ( id == null )
@@ -1070,10 +1074,10 @@ public abstract class AbstractBTreeParti
      */
     public Entry fetch( String id ) throws LdapException
     {
-        lockRead();
-
         try
         {
+            rwLock.readLock().lock();
+
             Dn dn = buildEntryDn( id );
 
             return fetch( id, dn );
@@ -1084,7 +1088,7 @@ public abstract class AbstractBTreeParti
         }
         finally
         {
-            unlockRead();
+            rwLock.readLock().unlock();
         }
     }
 
@@ -1112,15 +1116,14 @@ public abstract class AbstractBTreeParti
                 return new ClonedServerEntry( entry );
             }
 
-            lockRead();
-
             try
             {
+                rwLock.readLock().lock();
                 entry = master.get( id );
             }
             finally
             {
-                unlockRead();
+                rwLock.readLock().unlock();
             }
 
             if ( entry != null )
@@ -1157,6 +1160,8 @@ public abstract class AbstractBTreeParti
     {
         try
         {
+            setRWLock( modifyContext );
+
             Entry modifiedEntry = modify( modifyContext.getDn(),
                 modifyContext.getModItems().toArray( new Modification[]
                     {} ) );
@@ -1569,6 +1574,7 @@ public abstract class AbstractBTreeParti
 
         try
         {
+            setRWLock( moveContext );
             Dn oldDn = moveContext.getDn();
             Dn newSuperior = moveContext.getNewSuperior();
             Dn newDn = moveContext.getNewDn();
@@ -1587,7 +1593,8 @@ public abstract class AbstractBTreeParti
     /**
      * {@inheritDoc}
      */
-    public synchronized final void move( Dn oldDn, Dn newSuperiorDn, Dn newDn, Entry modifiedEntry ) throws Exception
+    public synchronized final void move( Dn oldDn, Dn newSuperiorDn, Dn newDn, Entry modifiedEntry )
+        throws Exception
     {
         // Check that the parent Dn exists
         String newParentId = getEntryId( newSuperiorDn );
@@ -1697,6 +1704,7 @@ public abstract class AbstractBTreeParti
 
         try
         {
+            setRWLock( moveAndRenameContext );
             Dn oldDn = moveAndRenameContext.getDn();
             Dn newSuperiorDn = moveAndRenameContext.getNewSuperiorDn();
             Rdn newRdn = moveAndRenameContext.getNewRdn();
@@ -1722,7 +1730,8 @@ public abstract class AbstractBTreeParti
     /**
      * {@inheritDoc}
      */
-    public synchronized final void moveAndRename( Dn oldDn, Dn newSuperiorDn, Rdn newRdn, Entry modifiedEntry,
+    public synchronized final void moveAndRename( Dn oldDn, Dn newSuperiorDn, Rdn newRdn,
+        Entry modifiedEntry,
         boolean deleteOldRdn ) throws Exception
     {
         // Check that the old entry exists
@@ -1793,7 +1802,8 @@ public abstract class AbstractBTreeParti
      * @param modifiedEntry the modified entry
      * @throws Exception if something goes wrong
      */
-    private void moveAndRename( Dn oldDn, String entryId, Dn newSuperior, Rdn newRdn, Entry modifiedEntry )
+    private void moveAndRename( Dn oldDn, String entryId, Dn newSuperior, Rdn newRdn,
+        Entry modifiedEntry )
         throws Exception
     {
         // Get the child and the new parent to be entries and Ids
@@ -1860,6 +1870,7 @@ public abstract class AbstractBTreeParti
     {
         try
         {
+            setRWLock( renameContext );
             Dn oldDn = renameContext.getDn();
             Rdn newRdn = renameContext.getNewRdn();
             boolean deleteOldRdn = renameContext.getDeleteOldRdn();
@@ -2051,6 +2062,8 @@ public abstract class AbstractBTreeParti
     {
         try
         {
+            setRWLock( entryContext );
+
             String id = getEntryId( entryContext.getDn() );
 
             Entry entry = fetch( id, entryContext.getDn() );
@@ -2101,33 +2114,42 @@ public abstract class AbstractBTreeParti
         Rdn[] rdnArray = new Rdn[10];
         int pos = 0;
 
-        do
+        try
         {
-            ParentIdAndRdn cur = rdnIdx.reverseLookup( parentId );
+            rwLock.readLock().lock();
 
-            if ( cur == null )
+            do
             {
-                return null;
-            }
+                ParentIdAndRdn cur = rdnIdx.reverseLookup( parentId );
 
-            Rdn[] rdns = cur.getRdns();
+                if ( cur == null )
+                {
+                    return null;
+                }
 
-            for ( Rdn rdn : rdns )
-            {
-                if ( ( pos > 0 ) && ( pos % 10 == 0 ) )
+                Rdn[] rdns = cur.getRdns();
+
+                for ( Rdn rdn : rdns )
                 {
-                    // extend the array
-                    Rdn[] newRdnArray = new Rdn[pos + 10];
-                    System.arraycopy( rdnArray, 0, newRdnArray, 0, pos );
-                    rdnArray = newRdnArray;
+                    if ( ( pos > 0 ) && ( pos % 10 == 0 ) )
+                    {
+                        // extend the array
+                        Rdn[] newRdnArray = new Rdn[pos + 10];
+                        System.arraycopy( rdnArray, 0, newRdnArray, 0, pos );
+                        rdnArray = newRdnArray;
+                    }
+
+                    rdnArray[pos++] = rdn;
                 }
 
-                rdnArray[pos++] = rdn;
+                parentId = cur.getParentId();
             }
-
-            parentId = cur.getParentId();
+            while ( !parentId.equals( rootId ) );
+        }
+        finally
+        {
+            rwLock.readLock().unlock();
         }
-        while ( !parentId.equals( rootId ) );
 
         Dn dn = new Dn( schemaManager, Arrays.copyOf( rdnArray, pos ) );
 
@@ -2186,21 +2208,29 @@ public abstract class AbstractBTreeParti
             ParentIdAndRdn suffixKey = new ParentIdAndRdn( Partition.ROOT_ID, suffixDn.getRdns() );
 
             // Check into the Rdn index, starting with the partition Suffix
-            String currentId = rdnIdx.forwardLookup( suffixKey );
-
-            for ( int i = dn.size() - suffixDn.size(); i > 0; i-- )
+            try
             {
-                Rdn rdn = dn.getRdn( i - 1 );
-                ParentIdAndRdn currentRdn = new ParentIdAndRdn( currentId, rdn );
-                currentId = rdnIdx.forwardLookup( currentRdn );
+                rwLock.readLock().lock();
+                String currentId = rdnIdx.forwardLookup( suffixKey );
 
-                if ( currentId == null )
+                for ( int i = dn.size() - suffixDn.size(); i > 0; i-- )
                 {
-                    break;
+                    Rdn rdn = dn.getRdn( i - 1 );
+                    ParentIdAndRdn currentRdn = new ParentIdAndRdn( currentId, rdn );
+                    currentId = rdnIdx.forwardLookup( currentRdn );
+
+                    if ( currentId == null )
+                    {
+                        break;
+                    }
                 }
-            }
 
-            return currentId;
+                return currentId;
+            }
+            finally
+            {
+                rwLock.readLock().unlock();
+            }
         }
         catch ( Exception e )
         {
@@ -2214,14 +2244,22 @@ public abstract class AbstractBTreeParti
      */
     public String getParentId( String childId ) throws Exception
     {
-        ParentIdAndRdn key = rdnIdx.reverseLookup( childId );
+        try
+        {
+            rwLock.readLock().lock();
+            ParentIdAndRdn key = rdnIdx.reverseLookup( childId );
 
-        if ( key == null )
+            if ( key == null )
+            {
+                return null;
+            }
+
+            return key.getParentId();
+        }
+        finally
         {
-            return null;
+            rwLock.readLock().unlock();
         }
-
-        return key.getParentId();
     }
 
 
@@ -2234,7 +2272,15 @@ public abstract class AbstractBTreeParti
         {
             ParentIdAndRdn key = new ParentIdAndRdn( Partition.ROOT_ID, suffixDn.getRdns() );
 
-            suffixId = rdnIdx.forwardLookup( key );
+            try
+            {
+                rwLock.readLock().lock();
+                suffixId = rdnIdx.forwardLookup( key );
+            }
+            finally
+            {
+                rwLock.readLock().unlock();
+            }
         }
 
         return suffixId;
@@ -2830,7 +2876,7 @@ public abstract class AbstractBTreeParti
      */
     private void lockRead()
     {
-        masterTableLock.readLock().lock();
+        rwLock.readLock().lock();
     }
 
 
@@ -2839,7 +2885,7 @@ public abstract class AbstractBTreeParti
      */
     private void unlockRead()
     {
-        masterTableLock.readLock().unlock();
+        rwLock.readLock().unlock();
     }
 
 
@@ -2848,7 +2894,7 @@ public abstract class AbstractBTreeParti
      */
     private void lockWrite()
     {
-        masterTableLock.writeLock().lock();
+        rwLock.writeLock().lock();
     }
 
 
@@ -2857,7 +2903,7 @@ public abstract class AbstractBTreeParti
      */
     private void unlockWrite()
     {
-        masterTableLock.writeLock().unlock();
+        rwLock.writeLock().unlock();
     }
 
 
@@ -2923,4 +2969,35 @@ public abstract class AbstractBTreeParti
     {
         this.searchEngine = searchEngine;
     }
+
+
+    /**
+     * Set and return the ReadWrite lock we use to protect the backend against concurrent modifications
+     * 
+     * @param operationContext The OperationContext which contain the reference to the OperationManager
+     */
+    private void setRWLock( OperationContext operationContext )
+    {
+        if ( operationContext.getSession() != null )
+        {
+            rwLock = operationContext.getSession().getDirectoryService().getOperationManager().getRWLock();
+        }
+        else
+        {
+            if ( rwLock == null )
+            {
+                // Create a ReadWrite lock from scratch
+                rwLock = new ReentrantReadWriteLock();
+            }
+        }
+    }
+
+
+    /**
+     * {@inheritDoc}
+     */
+    public ReadWriteLock getReadWriteLock()
+    {
+        return rwLock;
+    }
 }

Modified: directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/xdbm/Store.java
URL: http://svn.apache.org/viewvc/directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/xdbm/Store.java?rev=1494803&r1=1494802&r2=1494803&view=diff
==============================================================================
--- directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/xdbm/Store.java (original)
+++ directory/apacheds/trunk/xdbm-partition/src/main/java/org/apache/directory/server/xdbm/Store.java Wed Jun 19 23:28:48 2013
@@ -26,6 +26,7 @@ import java.util.Collections;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Set;
+import java.util.concurrent.locks.ReadWriteLock;
 
 import org.apache.directory.api.ldap.model.constants.SchemaConstants;
 import org.apache.directory.api.ldap.model.entry.Entry;
@@ -427,4 +428,10 @@ public interface Store
      * @return The masterTable instance
      */
     MasterTable getMasterTable();
+
+
+    /**
+     * @return The ReadWrite lock used to protect the server against concurrent read and writes
+     */
+    ReadWriteLock getReadWriteLock();
 }