svn commit: r1750075 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/cache/ main/java/org/apache/jackrabbit/oak/plugins/document/rdb/ test/java/org/apache/jackrabbit/oak/plugins/document/

[reschke]

Author: reschke
Date: Fri Jun 24 10:50:35 2016
New Revision: 1750075

URL: http://svn.apache.org/viewvc?rev=1750075&view=rev
Log:
OAK-4497: RDBDocumentStore: potential race condition between update and 
invalidate can cause stale cache entries

Modified:
    
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/cache/NodeDocumentCache.java
    
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java
    
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentQueryAndInvalidateIT.java

Modified: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/cache/NodeDocumentCache.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/cache/NodeDocumentCache.java?rev=1750075&r1=1750074&r2=1750075&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/cache/NodeDocumentCache.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/cache/NodeDocumentCache.java
 Fri Jun 24 10:50:35 2016
@@ -96,9 +96,21 @@ public class NodeDocumentCache implement
                 nodeDocumentsCache.invalidate(new StringValue(key));
             }
 
-            for (CacheChangesTracker tracker : changeTrackers) {
-                tracker.invalidateDocument(key);
-            }
+            internalMarkChanged(key);
+        } finally {
+            lock.unlock();
+        }
+    }
+
+    /**
+     * Mark that the document with the given key is being changed.
+     *
+     * @param key to mark
+     */
+    public void markChanged(@Nonnull String key) {
+        Lock lock = locks.acquire(key);
+        try {
+            internalMarkChanged(key);
         } finally {
             lock.unlock();
         }
@@ -447,6 +459,17 @@ public class NodeDocumentCache implement
     //----------------------------< internal 
>----------------------------------
 
     /**
+     * Marks the document as potentially changed.
+     * 
+     * @param key the document to be marked
+     */
+    private void internalMarkChanged(String key) {
+        for (CacheChangesTracker tracker : changeTrackers) {
+            tracker.invalidateDocument(key);
+        }
+    }
+
+    /**
      * Puts a document into the cache without acquiring a lock.
      *
      * @param doc the document to put into the cache.

Modified: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java?rev=1750075&r1=1750074&r2=1750075&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java
 Fri Jun 24 10:50:35 2016
@@ -524,6 +524,7 @@ public class RDBDocumentStore implements
             if (remove) {
                 nodesCache.invalidate(id);
             } else {
+                nodesCache.markChanged(id);
                 NodeDocument entry = nodesCache.getIfPresent(id);
                 if (entry != null) {
                     entry.markUpToDate(0);
@@ -1416,8 +1417,17 @@ public class RDBDocumentStore implements
                     // already in the cache
                     doc = convertFromDBObject(collection, row);
                 } else {
-                    // otherwise mark it as fresh
-                    ((NodeDocument) doc).markUpToDate(now);
+                    // we got a document from the cache, thus collection is 
NODES
+                    // and a tracker is present
+                    Lock lock = locks.acquire(row.getId());
+                    try {
+                        if (!tracker.mightBeenAffected(row.getId())) {
+                            // otherwise mark it as fresh
+                            ((NodeDocument) doc).markUpToDate(now);
+                        }
+                    } finally {
+                        lock.unlock();
+                    }
                 }
                 result.add(doc);
             }

Modified: 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentQueryAndInvalidateIT.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentQueryAndInvalidateIT.java?rev=1750075&r1=1750074&r2=1750075&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentQueryAndInvalidateIT.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentQueryAndInvalidateIT.java
 Fri Jun 24 10:50:35 2016
@@ -29,13 +29,11 @@ import static org.apache.jackrabbit.oak.
 import static 
org.apache.jackrabbit.oak.plugins.document.util.Utils.getKeyUpperLimit;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
-import static org.junit.Assume.assumeFalse;
 
 public class ConcurrentQueryAndInvalidateIT extends 
AbstractMultiDocumentStoreTest {
 
     public ConcurrentQueryAndInvalidateIT(DocumentStoreFixture dsf) {
         super(dsf);
-        assumeFalse(dsf instanceof DocumentStoreFixture.RDBFixture);
     }
 
     protected static final int NUM_NODES = 50;