You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by ju...@apache.org on 2010/10/04 12:12:55 UTC

svn commit: r1004184 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence: bundle/ pool/

Author: jukka
Date: Mon Oct  4 10:12:54 2010
New Revision: 1004184

URL: http://svn.apache.org/viewvc?rev=1004184&view=rev
Log:
JCR-2699: Improve read/write concurrency

Drop read synchronization from bundle PMs based on pooled databases or the filesystem.

Also remove the existsBundle() method as even the exists() methods use loadBundle() nowadays to better prime the bundle cache.

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleDbPersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleFsPersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleFsPersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/PostgreSQLPersistenceManager.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java?rev=1004184&r1=1004183&r2=1004184&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/AbstractBundlePersistenceManager.java Mon Oct  4 10:12:54 2010
@@ -320,17 +320,6 @@ public abstract class AbstractBundlePers
             throws ItemStateException;
 
     /**
-     * Checks if a bundle exists in the underlying system.
-     *
-     * @param id the node id of the bundle
-     * @return <code>true</code> if the bundle exists;
-     *         <code>false</code> otherwise.
-     * @throws ItemStateException if an error while checking occurs.
-     */
-    protected abstract boolean existsBundle(NodeId id)
-            throws ItemStateException;
-
-    /**
      * Stores a bundle to the underlying system.
      *
      * @param bundle the bundle to store
@@ -634,7 +623,9 @@ public abstract class AbstractBundlePers
     }
 
     /**
-     * Gets the bundle for the given node id.
+     * Gets the bundle for the given node id. Read/write synchronization
+     * happens higher up at the SISM level, so we don't need to worry about
+     * conflicts here.
      *
      * @param id the id of the bundle to retrieve.
      * @return the bundle or <code>null</code> if the bundle does not exist
@@ -646,14 +637,12 @@ public abstract class AbstractBundlePers
         if (bundle == MISSING) {
             return null;
         } else if (bundle == null) {
-            synchronized (this) {
-                bundle = loadBundle(id);
-                if (bundle != null) {
-                    bundle.markOld();
-                    bundles.put(id, bundle, bundle.getSize());
-                } else {
-                    bundles.put(id, MISSING, 16);
-                }
+            bundle = loadBundle(id);
+            if (bundle != null) {
+                bundle.markOld();
+                bundles.put(id, bundle, bundle.getSize());
+            } else {
+                bundles.put(id, MISSING, 16);
             }
         }
         return bundle;

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleDbPersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleDbPersistenceManager.java?rev=1004184&r1=1004183&r2=1004184&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleDbPersistenceManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleDbPersistenceManager.java Mon Oct  4 10:12:54 2010
@@ -778,7 +778,7 @@ public class BundleDbPersistenceManager 
         try {
             // skip root nodes (that point to itself)
             if (parentId != null && !id.toString().endsWith("babecafebabe")) {
-                if (!existsBundle(parentId)) {
+                if (loadBundle(parentId) == null) {
                     log.error("NodeState '" + id + "' references inexistent parent uuid '" + parentId + "'");
                 }
             }
@@ -1171,25 +1171,6 @@ public class BundleDbPersistenceManager 
     /**
      * {@inheritDoc}
      */
-    protected synchronized boolean existsBundle(NodeId id) throws ItemStateException {
-        ResultSet rs = null;
-        try {
-            Statement stmt = connectionManager.executeStmt(bundleSelectSQL, getKey(id));
-            rs = stmt.getResultSet();
-            // a bundle exists, if the result has at least one entry
-            return rs.next();
-        } catch (Exception e) {
-            String msg = "failed to check existence of bundle: " + id;
-            log.error(msg, e);
-            throw new ItemStateException(msg, e);
-        } finally {
-            closeResultSet(rs);
-        }
-    }
-
-    /**
-     * {@inheritDoc}
-     */
     protected synchronized void storeBundle(NodePropBundle bundle) throws ItemStateException {
         try {
             ByteArrayOutputStream out =

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleFsPersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleFsPersistenceManager.java?rev=1004184&r1=1004183&r2=1004184&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleFsPersistenceManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/bundle/BundleFsPersistenceManager.java Mon Oct  4 10:12:54 2010
@@ -231,8 +231,7 @@ public class BundleFsPersistenceManager 
     /**
      * {@inheritDoc}
      */
-    protected synchronized NodePropBundle loadBundle(NodeId id)
-            throws ItemStateException {
+    protected NodePropBundle loadBundle(NodeId id) throws ItemStateException {
         try {
             String path = buildNodeFilePath(null, id).toString();
             if (!itemFs.exists(path)) {
@@ -255,20 +254,6 @@ public class BundleFsPersistenceManager 
     }
 
     /**
-     * {@inheritDoc}
-     */
-    protected synchronized boolean existsBundle(NodeId id) throws ItemStateException {
-        try {
-            StringBuffer buf = buildNodeFilePath(null, id);
-            return itemFs.exists(buf.toString());
-        } catch (Exception e) {
-            String msg = "failed to check existence of bundle: " + id;
-            BundleFsPersistenceManager.log.error(msg, e);
-            throw new ItemStateException(msg, e);
-        }
-    }
-
-    /**
      * Creates the file path for the given node id that is
      * suitable for storing node states in a filesystem.
      *

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java?rev=1004184&r1=1004183&r2=1004184&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/AbstractBundlePersistenceManager.java Mon Oct  4 10:12:54 2010
@@ -320,17 +320,6 @@ public abstract class AbstractBundlePers
             throws ItemStateException;
 
     /**
-     * Checks if a bundle exists in the underlying system.
-     *
-     * @param id the node id of the bundle
-     * @return <code>true</code> if the bundle exists;
-     *         <code>false</code> otherwise.
-     * @throws ItemStateException if an error while checking occurs.
-     */
-    protected abstract boolean existsBundle(NodeId id)
-            throws ItemStateException;
-
-    /**
      * Stores a bundle to the underlying system.
      *
      * @param bundle the bundle to store
@@ -632,7 +621,9 @@ public abstract class AbstractBundlePers
     }
 
     /**
-     * Gets the bundle for the given node id.
+     * Gets the bundle for the given node id. Read/write synchronization
+     * happens higher up at the SISM level, so we don't need to worry about
+     * conflicts here.
      *
      * @param id the id of the bundle to retrieve.
      * @return the bundle or <code>null</code> if the bundle does not exist
@@ -644,14 +635,12 @@ public abstract class AbstractBundlePers
         if (bundle == MISSING) {
             return null;
         } else if (bundle == null) {
-            synchronized (this) {
-                bundle = loadBundle(id);
-                if (bundle != null) {
-                    bundle.markOld();
-                    bundles.put(id, bundle, bundle.getSize());
-                } else {
-                    bundles.put(id, MISSING, 16);
-                }
+            bundle = loadBundle(id);
+            if (bundle != null) {
+                bundle.markOld();
+                bundles.put(id, bundle, bundle.getSize());
+            } else {
+                bundles.put(id, MISSING, 16);
             }
         }
         return bundle;

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java?rev=1004184&r1=1004183&r2=1004184&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java Mon Oct  4 10:12:54 2010
@@ -745,7 +745,7 @@ public class BundleDbPersistenceManager 
         try {
             // skip root nodes (that point to itself)
             if (parentId != null && !id.toString().endsWith("babecafebabe")) {
-                if (!existsBundle(parentId)) {
+                if (loadBundle(parentId) == null) {
                     log.error("NodeState '" + id + "' references inexistent parent uuid '" + parentId + "'");
                 }
             }
@@ -1023,8 +1023,7 @@ public class BundleDbPersistenceManager 
     /**
      * {@inheritDoc}
      */
-    protected synchronized NodePropBundle loadBundle(NodeId id)
-            throws ItemStateException {
+    protected NodePropBundle loadBundle(NodeId id) throws ItemStateException {
         return loadBundle(id, false);
     }
 
@@ -1063,7 +1062,7 @@ public class BundleDbPersistenceManager 
      *         exist.
      * @throws ItemStateException if an error while loading occurs.
      */
-    protected synchronized NodePropBundle loadBundle(NodeId id, boolean checkBeforeLoading)
+    protected NodePropBundle loadBundle(NodeId id, boolean checkBeforeLoading)
             throws ItemStateException {
         ResultSet rs = null;
         
@@ -1098,24 +1097,6 @@ public class BundleDbPersistenceManager 
     /**
      * {@inheritDoc}
      */
-    protected synchronized boolean existsBundle(NodeId id) throws ItemStateException {
-        ResultSet rs = null;
-        try {
-            rs = conHelper.exec(bundleSelectSQL, getKey(id), false, 0);
-            // a bundle exists, if the result has at least one entry
-            return rs.next();
-        } catch (Exception e) {
-            String msg = "failed to check existence of bundle: " + id;
-            log.error(msg, e);
-            throw new ItemStateException(msg, e);
-        } finally {
-            DbUtility.close(rs);
-        }
-    }
-
-    /**
-     * {@inheritDoc}
-     */
     protected synchronized void storeBundle(NodePropBundle bundle) throws ItemStateException {
         try {
             ByteArrayOutputStream out =

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleFsPersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleFsPersistenceManager.java?rev=1004184&r1=1004183&r2=1004184&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleFsPersistenceManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleFsPersistenceManager.java Mon Oct  4 10:12:54 2010
@@ -230,8 +230,7 @@ public class BundleFsPersistenceManager 
     /**
      * {@inheritDoc}
      */
-    protected synchronized NodePropBundle loadBundle(NodeId id)
-            throws ItemStateException {
+    protected NodePropBundle loadBundle(NodeId id) throws ItemStateException {
         try {
             String path = buildNodeFilePath(null, id).toString();
             if (!itemFs.exists(path)) {
@@ -254,20 +253,6 @@ public class BundleFsPersistenceManager 
     }
 
     /**
-     * {@inheritDoc}
-     */
-    protected synchronized boolean existsBundle(NodeId id) throws ItemStateException {
-        try {
-            StringBuffer buf = buildNodeFilePath(null, id);
-            return itemFs.exists(buf.toString());
-        } catch (Exception e) {
-            String msg = "failed to check existence of bundle: " + id;
-            BundleFsPersistenceManager.log.error(msg, e);
-            throw new ItemStateException(msg, e);
-        }
-    }
-
-    /**
      * Creates the file path for the given node id that is
      * suitable for storing node states in a filesystem.
      *

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/PostgreSQLPersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/PostgreSQLPersistenceManager.java?rev=1004184&r1=1004183&r2=1004184&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/PostgreSQLPersistenceManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/PostgreSQLPersistenceManager.java Mon Oct  4 10:12:54 2010
@@ -91,10 +91,9 @@ public class PostgreSQLPersistenceManage
      * 
      * {@inheritDoc}
      */
-    protected synchronized NodePropBundle loadBundle(NodeId id)
-            throws ItemStateException {
+    protected NodePropBundle loadBundle(NodeId id) throws ItemStateException {
         ResultSet rs = null;
-        try {        	
+        try {
             rs = conHelper.exec(bundleSelectSQL, getKey(id), false, 0);
             if (rs.next()) {
                 InputStream input = rs.getBinaryStream(1);