[jboss-cvs] JBossCache/src/org/jboss/cache/loader ...

Manik Surtani manik at jboss.org
Tue Jun 19 06:45:43 EDT 2007


  User: msurtani
  Date: 07/06/19 06:45:43

  Modified:    src/org/jboss/cache/loader    JDBCCacheLoader.java
                        FileCacheLoader.java AdjListJDBCCacheLoader.java
  Log:
  CL thread safety
  
  Revision  Changes    Path
  1.36      +243 -219  JBossCache/src/org/jboss/cache/loader/JDBCCacheLoader.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: JDBCCacheLoader.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossCache/src/org/jboss/cache/loader/JDBCCacheLoader.java,v
  retrieving revision 1.35
  retrieving revision 1.36
  diff -u -b -r1.35 -r1.36
  --- JDBCCacheLoader.java	7 Mar 2007 23:32:05 -0000	1.35
  +++ JDBCCacheLoader.java	19 Jun 2007 10:45:42 -0000	1.36
  @@ -1,5 +1,6 @@
   package org.jboss.cache.loader;
   
  +import net.jcip.annotations.ThreadSafe;
   import org.apache.commons.logging.Log;
   import org.apache.commons.logging.LogFactory;
   import org.jboss.cache.Fqn;
  @@ -24,7 +25,7 @@
    * All configuration elements described there {@link org.jboss.cache.loader.JDBCCacheLoaderOld} also apply for this
    * implementation.
    * <p/>
  - *
  + * <p/>
    * Additional configuration info: <br>
    * cache.jdbc.sql-concat : DBMS specific function for concat strings. Most likely this will be concat(1,2), but might
    * be different for proprietary systems.
  @@ -33,6 +34,7 @@
    * @author <a href="mailto:galder.zamarreno at jboss.com">Galder Zamarreno</a>
    * @version 1.0
    */
  + at ThreadSafe
   public class JDBCCacheLoader extends AdjListJDBCCacheLoader
   {
   
  @@ -48,7 +50,8 @@
           if (base instanceof JDBCCacheLoaderConfig)
           {
               config = (JDBCCacheLoaderConfig) base;
  -        } else
  +      }
  +      else
           {
               config = new JDBCCacheLoaderConfig(base);
           }
  @@ -90,15 +93,19 @@
               ps = conn.prepareStatement(config.getDeleteNodeSql());
               //apend / at the end avoids this issue: 'a/b/cd' is not a child of 'a/b/c'
               ps.setString(1, fqn.isRoot() ? fqn.toString() : fqn + Fqn.SEPARATOR);
  +         lock.acquireLock(fqn, true);
               ps.executeUpdate();
  -        } catch (SQLException e)
  +      }
  +      catch (SQLException e)
           {
               log.error("Failed to remove the node : " + fqn, e);
               throw new IllegalStateException("Failure while removing sub-tree (" + fqn + ")" + e.getMessage());
  -        } finally
  +      }
  +      finally
           {
               safeClose(ps);
               cf.close(conn);
  +         lock.releaseLock(fqn);
           }
       }
   
  @@ -130,11 +137,13 @@
                   NodeData nodeData = (attributes == null || attributes.isEmpty()) ? new NodeData(path) : new NodeData(path, attributes);
                   list.add(nodeData);
               }
  -        } catch (SQLException e)
  +      }
  +      catch (SQLException e)
           {
               log.error("Failed to load state for node(" + fqn + ") :" + e.getMessage(), e);
               throw new IllegalStateException("Failed to load state for node(" + fqn + ") :" + e.getMessage());
  -        } finally
  +      }
  +      finally
           {
               safeClose(rs);
               safeClose(ps);
  @@ -159,7 +168,8 @@
                   log.error("Failure while reading attribute set from db", e);
                   throw new SQLException("Failure while reading attribute set from db " + e);
               }
  -        } else
  +      }
  +      else
           {
               result = null;
           }
  @@ -168,15 +178,20 @@
   
       private Map _put(Fqn name, Map attributes) throws Exception
       {
  +      lock.acquireLock(name, true);
  +      try
  +      {
           Map result = null;
           Map treeNode = loadNode(name);
           if (treeNode == null)
           {
               addNewSubtree(name, attributes);
  -        } else if (treeNode == NULL_NODE_IN_ROW)
  +         }
  +         else if (treeNode == NULL_NODE_IN_ROW)
           {
               updateNode(name, attributes);
  -        } else
  +         }
  +         else
           {//the node exists and the attribute map is NOT null
               Map<Object, Object> newAttributes = new HashMap<Object, Object>(treeNode);
               newAttributes.putAll(attributes);//creation sequnce important - we need to overwrite old values
  @@ -185,6 +200,11 @@
           }
           return result;
       }
  +      finally
  +      {
  +         lock.releaseLock(name);
  +      }
  +   }
   
       private void addNewSubtree(Fqn name, Map attributes) throws Exception
       {
  @@ -194,7 +214,8 @@
               if (currentNode.equals(name))
               {
                   insertNode(currentNode, attributes);
  -            } else
  +         }
  +         else
               {
                   insertNode(currentNode, null);
               }
  @@ -203,7 +224,8 @@
                   break;
               }
               currentNode = currentNode.getParent();
  -        } while (!exists(currentNode));
  +      }
  +      while (!exists(currentNode));
       }
   
   
  @@ -245,11 +267,13 @@
               rs = ps.executeQuery();
               rs.next();//count(*) will always return one row
               return rs.getInt(1);
  -        } catch (Exception e)
  +      }
  +      catch (Exception e)
           {
               log.error("Failure while trying to get the count of persisted nodes: " + e.getMessage(), e);
               throw new IllegalStateException("Failure while trying to get the count of persisted nodes: " + e.getMessage());
  -        } finally
  +      }
  +      finally
           {
               safeClose(rs);
               safeClose(ps);
  
  
  
  1.34      +156 -69   JBossCache/src/org/jboss/cache/loader/FileCacheLoader.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: FileCacheLoader.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossCache/src/org/jboss/cache/loader/FileCacheLoader.java,v
  retrieving revision 1.33
  retrieving revision 1.34
  diff -u -b -r1.33 -r1.34
  --- FileCacheLoader.java	11 Jun 2007 12:58:16 -0000	1.33
  +++ FileCacheLoader.java	19 Jun 2007 10:45:42 -0000	1.34
  @@ -1,18 +1,22 @@
   package org.jboss.cache.loader;
   
  +import net.jcip.annotations.ThreadSafe;
   import org.apache.commons.logging.Log;
   import org.apache.commons.logging.LogFactory;
   import org.jboss.cache.Fqn;
   import org.jboss.cache.Modification;
   import org.jboss.cache.config.CacheLoaderConfig.IndividualCacheLoaderConfig;
  +import org.jboss.cache.lock.StripedLock;
   import org.jboss.cache.marshall.ObjectSerializationFactory;
   
   import java.io.File;
   import java.io.FileInputStream;
  +import java.io.FileNotFoundException;
   import java.io.FileOutputStream;
   import java.io.IOException;
   import java.io.ObjectInputStream;
   import java.io.ObjectOutputStream;
  +import java.util.Collections;
   import java.util.HashMap;
   import java.util.HashSet;
   import java.util.List;
  @@ -38,24 +42,30 @@
    * As a rule of thumb, it is recommended that the FileCacheLoader not be used in a highly concurrent,
    * transactional or stressful environment, and it's use is restricted to testing.
    * <p/>
  + * In terms of concurrency, file systems are notoriously inconsistent in their implementations of concurrent locks.  To get around
  + * this and to meet the <b>thread safety</b> contracts set out in {@link CacheLoader}, this implementation uses a {@link org.jboss.cache.lock.StripedLock}
    *
    * @author Bela Ban
    * @author <a href="mailto:galder.zamarreno at jboss.com">Galder Zamarreno</a>
  - * @version $Id: FileCacheLoader.java,v 1.33 2007/06/11 12:58:16 msurtani Exp $
  + * @author <a href="mailto:manik at jboss.org">Manik Surtani</a>
  + * @version $Id: FileCacheLoader.java,v 1.34 2007/06/19 10:45:42 msurtani Exp $
    */
  + at ThreadSafe
   public class FileCacheLoader extends AbstractCacheLoader
   {
      File root = null;
      String rootPath = null;
      Log log = LogFactory.getLog(getClass());
   
  +   protected final StripedLock lock = new StripedLock();
  +
      private FileCacheLoaderConfig config;
   
      /**
       * HashMap<Object,List<Modification>>. List of open transactions. Note that this is purely transient, as
       * we don't use a log, recovery is not available
       */
  -   Map<Object, List<Modification>> transactions = new ConcurrentHashMap();
  +   Map<Object, List<Modification>> transactions = new ConcurrentHashMap<Object, List<Modification>>();
   
      /**
       * CacheImpl data file.
  @@ -77,10 +87,6 @@
       */
      public static final Pattern FQN_PATTERN = Pattern.compile("[\\\\\\/:*<>|\"?]");
   
  -   public FileCacheLoader()
  -   {
  -   }
  -
      public void setConfig(IndividualCacheLoaderConfig base)
      {
         if (base instanceof FileCacheLoaderConfig)
  @@ -107,6 +113,9 @@
   
      public void create() throws Exception
      {
  +      lock.acquireLock(Fqn.ROOT, true);
  +      try
  +      {
         if (root == null)
         {
            String tmpLocation = System.getProperty("java.io.tmpdir", "C:\\tmp");
  @@ -139,6 +148,11 @@
            throw new IOException("Cache loader location [" + root + "] is not a directory!");
         }
      }
  +      finally
  +      {
  +         lock.releaseLock(Fqn.ROOT);
  +      }
  +   }
   
      public void start() throws Exception
      {
  @@ -154,6 +168,9 @@
   
      public Set<String> getChildrenNames(Fqn fqn) throws Exception
      {
  +      lock.acquireLock(fqn, false);
  +      try
  +      {
         File parent = getDirectory(fqn, false);
         if (parent == null) return null;
         File[] children = parent.listFiles();
  @@ -169,20 +186,44 @@
         }
         return s.size() == 0 ? null : s;
      }
  +      finally
  +      {
  +         lock.releaseLock(fqn);
  +      }
  +   }
   
      public Map get(Fqn fqn) throws Exception
      {
  +      lock.acquireLock(fqn, false);
  +      try
  +      {
         return loadAttributes(fqn);
      }
  +      finally
  +      {
  +         lock.releaseLock(fqn);
  +      }
  +   }
   
      public boolean exists(Fqn fqn) throws Exception
      {
  +      lock.acquireLock(fqn, false);
  +      try
  +      {
         File f = getDirectory(fqn, false);
         return f != null;
      }
  +      finally
  +      {
  +         lock.releaseLock(fqn);
  +      }
  +   }
   
      public Object put(Fqn fqn, Object key, Object value) throws Exception
      {
  +      lock.acquireLock(fqn, true);
  +      try
  +      {
         Object retval;
         Map m = loadAttributes(fqn);
         if (m == null) m = new HashMap();
  @@ -190,6 +231,11 @@
         storeAttributes(fqn, m);
         return retval;
      }
  +      finally
  +      {
  +         lock.releaseLock(fqn);
  +      }
  +   }
   
      public void put(Fqn fqn, Map attributes) throws Exception
      {
  @@ -199,6 +245,9 @@
   
      public void put(Fqn fqn, Map attributes, boolean erase) throws Exception
      {
  +      lock.acquireLock(fqn, false);
  +      try
  +      {
         Map m = erase ? new HashMap() : loadAttributes(fqn);
         if (m == null) m = new HashMap();
         if (attributes != null)
  @@ -207,6 +256,11 @@
         }
         storeAttributes(fqn, m);
      }
  +      finally
  +      {
  +         lock.releaseLock(fqn);
  +      }
  +   }
   
      void put(Fqn fqn) throws Exception
      {
  @@ -215,6 +269,9 @@
   
      public Object remove(Fqn fqn, Object key) throws Exception
      {
  +      lock.acquireLock(fqn, true);
  +      try
  +      {
         Object retval;
         Map m = loadAttributes(fqn);
         if (m == null) return null;
  @@ -222,9 +279,17 @@
         storeAttributes(fqn, m);
         return retval;
      }
  +      finally
  +      {
  +         lock.releaseLock(fqn);
  +      }
  +   }
   
      public void remove(Fqn fqn) throws Exception
      {
  +      lock.acquireLock(fqn, true);
  +      try
  +      {
         File dir = getDirectory(fqn, false);
         if (dir != null)
         {
  @@ -235,9 +300,17 @@
            }
         }
      }
  +      finally
  +      {
  +         lock.releaseLock(fqn);
  +      }
  +   }
   
      public void removeData(Fqn fqn) throws Exception
      {
  +      lock.acquireLock(fqn, true);
  +      try
  +      {
         File f = getDirectory(fqn, false);
         if (f != null)
         {
  @@ -252,6 +325,11 @@
            }
         }
      }
  +      finally
  +      {
  +         lock.releaseLock(fqn);
  +      }
  +   }
   
      public void prepare(Object tx, List<Modification> modifications, boolean one_phase) throws Exception
      {
  @@ -372,7 +450,16 @@
         if (!child.exists()) return new HashMap(0); // no node attribs exist hence the empty HashMap.
         //if(!child.exists()) return null;
   
  -      Map m = (Map) unmarshall(child);
  +      Map m = null;
  +      try
  +      {
  +         m = (Map) unmarshall(child);
  +      }
  +      catch (FileNotFoundException fnfe)
  +      {
  +         // child no longer exists!
  +         m = Collections.emptyMap();
  +      }
         return m;
      }
   
  
  
  
  1.6       +26 -12    JBossCache/src/org/jboss/cache/loader/AdjListJDBCCacheLoader.java
  
  (In the diff below, changes in quantity of whitespace are not shown.)
  
  Index: AdjListJDBCCacheLoader.java
  ===================================================================
  RCS file: /cvsroot/jboss/JBossCache/src/org/jboss/cache/loader/AdjListJDBCCacheLoader.java,v
  retrieving revision 1.5
  retrieving revision 1.6
  diff -u -b -r1.5 -r1.6
  --- AdjListJDBCCacheLoader.java	19 Apr 2007 21:51:19 -0000	1.5
  +++ AdjListJDBCCacheLoader.java	19 Jun 2007 10:45:42 -0000	1.6
  @@ -1,9 +1,11 @@
   package org.jboss.cache.loader;
   
  +import net.jcip.annotations.ThreadSafe;
   import org.apache.commons.logging.Log;
   import org.jboss.cache.Fqn;
   import org.jboss.cache.Modification;
   import org.jboss.cache.config.CacheLoaderConfig;
  +import org.jboss.cache.lock.StripedLock;
   import org.jboss.cache.util.Util;
   
   import java.io.ByteArrayInputStream;
  @@ -40,11 +42,13 @@
    * @author <a href="mailto:galder.zamarreno at jboss.com">Galder Zamarreno</a>
    * @version 1.0
    */
  + at ThreadSafe
   public abstract class AdjListJDBCCacheLoader extends AbstractCacheLoader
   {
      protected ConnectionFactory cf;
      protected String driverName;
      private AdjListJDBCCacheLoaderConfig config;
  +   protected StripedLock lock = new StripedLock();
   
      public void setConfig(CacheLoaderConfig.IndividualCacheLoaderConfig base)
      {
  @@ -114,6 +118,7 @@
            con = cf.getConnection();
            ps = con.prepareStatement(config.getSelectChildNamesSql());
            ps.setString(1, fqn.toString());
  +         lock.acquireLock(fqn, false);
            rs = ps.executeQuery();
            if (rs.next())
            {
  @@ -139,6 +144,7 @@
            safeClose(rs);
            safeClose(ps);
            cf.close(con);
  +         lock.releaseLock(fqn);
         }
   
         return children == null ? null : Collections.unmodifiableSet(children);
  @@ -307,6 +313,9 @@
       */
      public Object remove(Fqn name, Object key) throws Exception
      {
  +      lock.acquireLock(name, true);
  +      try
  +      {
         Object removedValue = null;
         Map node = loadNode(name);
         if (node != null && node != NULL_NODE_IN_ROW)
  @@ -323,6 +332,11 @@
         }
         return removedValue;
      }
  +      finally
  +      {
  +         lock.releaseLock(name);
  +      }
  +   }
   
   
      /**
  
  
  



More information about the jboss-cvs-commits mailing list