directory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Knecht <fel...@apache.org>
Subject Re: svn commit: r1057707 - /directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java
Date Tue, 11 Jan 2011 16:13:56 GMT
I don't see the need of locking in methods like "public boolean 
hasSubentry(...)". What's the benefit of locking?
A normal use case is IMO that some time after calling the has... method 
one will either read or write the cache. But there's no garantee, that 
when now read/write the cache is still in the same state as when calling 
the has... method. So isn't it just a waste of time to lock in this case?

Curious Felix


On 01/11/2011 04:57 PM, elecharny@apache.org wrote:
> Author: elecharny
> Date: Tue Jan 11 15:57:56 2011
> New Revision: 1057707
>
> URL: http://svn.apache.org/viewvc?rev=1057707&view=rev
> Log:
> Added synchronization around the cache
>
> Modified:
>      directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java
>
> Modified: directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java
> URL: http://svn.apache.org/viewvc/directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java?rev=1057707&r1=1057706&r2=1057707&view=diff
> ==============================================================================
> --- directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java
(original)
> +++ directory/apacheds/branches/apacheds-AP/core-api/src/main/java/org/apache/directory/server/core/administrative/SubentryCache.java
Tue Jan 11 15:57:56 2011
> @@ -23,6 +23,7 @@ package org.apache.directory.server.core
>   import java.util.HashMap;
>   import java.util.Iterator;
>   import java.util.Map;
> +import java.util.concurrent.locks.ReentrantReadWriteLock;
>
>   import org.apache.directory.shared.ldap.exception.LdapException;
>   import org.apache.directory.shared.ldap.name.DN;
> @@ -44,6 +45,49 @@ public class SubentryCache implements It
>       /** The Subentry DN cache */
>       private final DnNode<Subentry[]>  dnCache;
>
> +    /** A lock to guarantee the Subentry cache consistency */
> +    private ReentrantReadWriteLock mutex = new ReentrantReadWriteLock();
> +
> +    /**
> +     * Get a read-lock on the Subentry cache.
> +     * No read operation can be done on the AP cache if this
> +     * method is not called before.
> +     */
> +    public void lockRead()
> +    {
> +        mutex.readLock().lock();
> +    }
> +
> +
> +    /**
> +     * Get a write-lock on the Subentry cache.
> +     * No write operation can be done on the apCache if this
> +     * method is not called before.
> +     */
> +    public void lockWrite()
> +    {
> +        mutex.writeLock().lock();
> +    }
> +
> +
> +    /**
> +     * Release the read-write lock on the AP cache.
> +     * This method must be called after having read or modified the
> +     * AP cache
> +     */
> +    public void unlock()
> +    {
> +        if ( mutex.isWriteLockedByCurrentThread() )
> +        {
> +            mutex.writeLock().unlock();
> +        }
> +        else
> +        {
> +            mutex.readLock().unlock();
> +        }
> +    }
> +
> +
>       /**
>        * Creates a new instance of SubentryCache with a default maximum size.
>        */
> @@ -63,10 +107,18 @@ public class SubentryCache implements It
>        */
>       public final Subentry getSubentry( String uuid, AdministrativeRoleEnum role )
>       {
> -        Subentry[] subentries = uuidCache.get( uuid );
> -        Subentry subentry = subentries[role.getValue()];
> -
> -        return subentry;
> +        try
> +        {
> +            lockRead();
> +            Subentry[] subentries = uuidCache.get( uuid );
> +            Subentry subentry = subentries[role.getValue()];
> +
> +            return subentry;
> +        }
> +        finally
> +        {
> +            unlock();
> +        }
>       }
>
>
> @@ -78,9 +130,17 @@ public class SubentryCache implements It
>        */
>       public final Subentry[] getSubentries( String uuid )
>       {
> -        Subentry[] subentries = uuidCache.get( uuid );
> -
> -        return subentries;
> +        try
> +        {
> +            lockRead();
> +            Subentry[] subentries = uuidCache.get( uuid );
> +
> +            return subentries;
> +        }
> +        finally
> +        {
> +            unlock();
> +        }
>       }
>
>
> @@ -93,10 +153,18 @@ public class SubentryCache implements It
>        */
>       public final Subentry getSubentry( DN dn, AdministrativeRoleEnum role )
>       {
> -        Subentry[] subentries = dnCache.getElement( dn );
> -        Subentry subentry = subentries[role.getValue()];
> -
> -        return subentry;
> +        try
> +        {
> +            lockRead();
> +            Subentry[] subentries = dnCache.getElement( dn );
> +            Subentry subentry = subentries[role.getValue()];
> +
> +            return subentry;
> +        }
> +        finally
> +        {
> +            unlock();
> +        }
>       }
>
>
> @@ -108,9 +176,17 @@ public class SubentryCache implements It
>        */
>       public final Subentry[] getSubentries( DN dn )
>       {
> -        Subentry[] subentries = dnCache.getElement( dn );
> -
> -        return subentries;
> +        try
> +        {
> +            lockRead();
> +            Subentry[] subentries = dnCache.getElement( dn );
> +
> +            return subentries;
> +        }
> +        finally
> +        {
> +            unlock();
> +        }
>       }
>
>
> @@ -123,9 +199,17 @@ public class SubentryCache implements It
>        */
>       public final Subentry[] removeSubentry( String uuid )
>       {
> -        Subentry[] oldSubentry = uuidCache.remove( uuid );
> -
> -        return oldSubentry;
> +        try
> +        {
> +            lockWrite();
> +            Subentry[] oldSubentry = uuidCache.remove( uuid );
> +
> +            return oldSubentry;
> +        }
> +        finally
> +        {
> +            unlock();
> +        }
>       }
>
>
> @@ -138,22 +222,30 @@ public class SubentryCache implements It
>        */
>       public final Subentry[] removeSubentry( DN dn ) throws LdapException
>       {
> -        Subentry[] oldSubentry = dnCache.getElement( dn );
> -        dnCache.remove( dn );
> -
> -        // Update the UUID cache
> -        if ( oldSubentry != null )
> -        {
> -            for ( Subentry subentry : oldSubentry )
> -            {
> -                if ( subentry != null )
> +        try
> +        {
> +            lockWrite();
> +            Subentry[] oldSubentry = dnCache.getElement( dn );
> +            dnCache.remove( dn );
> +
> +            // Update the UUID cache
> +            if ( oldSubentry != null )
> +            {
> +                for ( Subentry subentry : oldSubentry )
>                   {
> -                    uuidCache.remove( subentry.getUuid() );
> +                    if ( subentry != null )
> +                    {
> +                        uuidCache.remove( subentry.getUuid() );
> +                    }
>                   }
>               }
> +
> +            return oldSubentry;
> +        }
> +        finally
> +        {
> +            unlock();
>           }
> -
> -        return oldSubentry;
>       }
>
>
> @@ -166,28 +258,36 @@ public class SubentryCache implements It
>        */
>       public Subentry[] addSubentry( DN dn, Subentry subentry ) throws LdapException
>       {
> -        Subentry[] subentries = uuidCache.get( subentry.getUuid() );
> -
> -        if ( subentries == null )
> -        {
> -            subentries = new Subentry[4];
> -        }
> -
> -        subentries[subentry.getAdministrativeRole().getValue()] = subentry;
> -        Subentry[] oldSubentry = uuidCache.put( subentry.getUuid(), subentries );
> -
> -        Subentry[] dnSubentries = dnCache.getElement( dn );
> -
> -        if ( dnSubentries != null )
> +        try
>           {
> -            dnSubentries[subentry.getAdministrativeRole().getValue()] = subentry;
> +            lockWrite();
> +            Subentry[] subentries = uuidCache.get( subentry.getUuid() );
> +
> +            if ( subentries == null )
> +            {
> +                subentries = new Subentry[4];
> +            }
> +
> +            subentries[subentry.getAdministrativeRole().getValue()] = subentry;
> +            Subentry[] oldSubentry = uuidCache.put( subentry.getUuid(), subentries );
> +
> +            Subentry[] dnSubentries = dnCache.getElement( dn );
> +
> +            if ( dnSubentries != null )
> +            {
> +                dnSubentries[subentry.getAdministrativeRole().getValue()] = subentry;
> +            }
> +            else
> +            {
> +                dnCache.add( dn, subentries );
> +            }
> +
> +            return oldSubentry;
>           }
> -        else
> +        finally
>           {
> -            dnCache.add( dn, subentries );
> +            unlock();
>           }
> -
> -        return oldSubentry;
>       }
>
>
> @@ -198,7 +298,15 @@ public class SubentryCache implements It
>        */
>       public boolean hasSubentry( String uuid )
>       {
> -        return uuidCache.containsKey( uuid );
> +        try
> +        {
> +            lockRead();
> +            return uuidCache.containsKey( uuid );
> +        }
> +        finally
> +        {
> +            unlock();
> +        }
>       }
>
>
> @@ -209,7 +317,15 @@ public class SubentryCache implements It
>        */
>       public boolean hasSubentry( DN dn )
>       {
> -        return dnCache.hasElement( dn );
> +        try
> +        {
> +            lockRead();
> +            return dnCache.hasElement( dn );
> +        }
> +        finally
> +        {
> +            unlock();
> +        }
>       }
>
>
> @@ -222,15 +338,23 @@ public class SubentryCache implements It
>        */
>       public boolean hasSubentry( String uuid, AdministrativeRoleEnum role )
>       {
> -        Subentry[] subentries = uuidCache.get( uuid );
> -
> -        if ( subentries == null )
> +        try
>           {
> -            return false;
> +            lockRead();
> +            Subentry[] subentries = uuidCache.get( uuid );
> +
> +            if ( subentries == null )
> +            {
> +                return false;
> +            }
> +            else
> +            {
> +                return subentries[ role.getValue() ] != null;
> +            }
>           }
> -        else
> +        finally
>           {
> -            return subentries[ role.getValue() ] != null;
> +            unlock();
>           }
>       }
>
> @@ -243,15 +367,23 @@ public class SubentryCache implements It
>        */
>       public boolean hasSubentry( DN dn, AdministrativeRoleEnum role )
>       {
> -        Subentry[] subentries = dnCache.getElement( dn );
> -
> -        if ( subentries == null )
> +        try
>           {
> -            return false;
> +            lockRead();
> +            Subentry[] subentries = dnCache.getElement( dn );
> +
> +            if ( subentries == null )
> +            {
> +                return false;
> +            }
> +            else
> +            {
> +                return subentries[ role.getValue() ] != null;
> +            }
>           }
> -        else
> +        finally
>           {
> -            return subentries[ role.getValue() ] != null;
> +            unlock();
>           }
>       }
>
> @@ -261,6 +393,14 @@ public class SubentryCache implements It
>        */
>       public Iterator<String>  iterator()
>       {
> -        return uuidCache.keySet().iterator();
> +        try
> +        {
> +            lockRead();
> +            return uuidCache.keySet().iterator();
> +        }
> +        finally
> +        {
> +            unlock();
> +        }
>       }
>   }
>
>


Mime
View raw message