jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ste...@apache.org
Subject svn commit: r1036117 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state: LocalItemStateManager.java SharedItemStateManager.java
Date Wed, 17 Nov 2010 17:14:15 GMT
Author: stefan
Date: Wed Nov 17 17:14:14 2010
New Revision: 1036117

URL: http://svn.apache.org/viewvc?rev=1036117&view=rev
Log:
Revert: JCR-2699: Improve read/write concurrency  It's better to allow the occasional cache
overwrite than to block concurrent threads. The locking in SessionState and SISM already prevent
concurrent read/write operations that could possibly lead to inconsistencies.

=> the log warnings "overwriting cached entry" do indicate that, at any given time, 
there might be multiple ItemState instances in the 'shared' layer representing the same 
item. there's a risk of data loss/inconsistency since all but one instance are stale copies.

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java?rev=1036117&r1=1036116&r2=1036117&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
Wed Nov 17 17:14:14 2010
@@ -163,17 +163,19 @@ public class LocalItemStateManager
             return state;
         }
 
-        // check cache
-        state = cache.retrieve(id);
-        if (state == null) {
-            // regular behaviour
-            if (id.denotesNode()) {
-                state = getNodeState((NodeId) id);
-            } else {
-                state = getPropertyState((PropertyId) id);
+        // check cache. synchronized to ensure an entry is not created twice.
+        synchronized (this) {
+            state = cache.retrieve(id);
+            if (state == null) {
+                // regular behaviour
+                if (id.denotesNode()) {
+                    state = getNodeState((NodeId) id);
+                } else {
+                    state = getPropertyState((PropertyId) id);
+                }
             }
+            return state;
         }
-        return state;
     }
 
     /**

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java?rev=1036117&r1=1036116&r2=1036117&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
Wed Nov 17 17:14:14 2010
@@ -1703,8 +1703,21 @@ public class SharedItemStateManager
             // not found in cache, load from persistent storage
             state = loadItemState(id);
             state.setStatus(ItemState.STATUS_EXISTING);
-            state.setContainer(this);
-            cache.cache(state);
+            synchronized (this) {
+                // Use a double check to ensure that the cache entry is
+                // not created twice. We don't synchronize the entire
+                // method to allow the first cache retrieval to proceed
+                // even when another thread is loading a new item state.
+                ItemState cachedState = cache.retrieve(id);
+                if (cachedState == null) {
+                    // put it in cache
+                    cache.cache(state);
+                    // set parent container
+                    state.setContainer(this);
+                } else {
+                    state = cachedState;
+                }
+            }
         }
         return state;
     }



Mime
View raw message