You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ch...@apache.org on 2013/11/15 07:49:57 UTC

svn commit: r1542183 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/mongomk/ test/java/org/apache/jackrabbit/oak/plugins/mongomk/

Author: chetanm
Date: Fri Nov 15 06:49:56 2013
New Revision: 1542183

URL: http://svn.apache.org/r1542183
Log:
OAK-1156 - Improve the document cache invalidation logic to selectivly invalidate doc

Changes as per review comments
1. Using only modCount for checking if two nodes are same and removing check for lastRev
   as modCount gets changed for changes in lastRev also
2. Using AtomicLong for lastCheck

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidator.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidationIT.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidator.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidator.java?rev=1542183&r1=1542182&r2=1542183&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidator.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidator.java Fri Nov 15 06:49:56 2013
@@ -164,13 +164,13 @@ abstract class CacheInvalidator {
 
             //Fetch only the lastRev map and id
             final BasicDBObject keys = new BasicDBObject(NodeDocument.ID, 1);
-            keys.put(NodeDocument.LAST_REV, 1);
             keys.put(NodeDocument.MOD_COUNT, 1);
 
             while (pitr.hasNext()) {
                 final TreeNode tn = pitr.next();
 
                 //Root node would already have been processed
+                //Allows us to save on the extra query for /
                 if(tn.isRoot()){
                     tn.markUptodate(startTime);
                     continue;
@@ -201,13 +201,14 @@ abstract class CacheInvalidator {
 
                         result.cacheEntriesProcessedCount++;
 
-                        //Note that this is a partial document
-                        NodeDocument latestDoc = documentStore.convertFromDBObject(Collection.NODES, obj);
+                        Number latestModCount = (Number) obj.get(NodeDocument.MOD_COUNT);
+                        String id = (String) obj.get(NodeDocument.ID);
 
-                        final TreeNode tn2 = sameLevelNodes.get(latestDoc.getId());
+                        final TreeNode tn2 = sameLevelNodes.get(id);
                         NodeDocument cachedDoc = tn2.getDocument();
                         if (cachedDoc != null) {
-                            if (noChangeInRevision(latestDoc, cachedDoc)) {
+                            boolean noChangeInModCount = Objects.equal(latestModCount, cachedDoc.getModCount());
+                            if (noChangeInModCount) {
                                 result.uptodateCount++;
                                 tn2.markUptodate(startTime);
                             } else {
@@ -241,15 +242,6 @@ abstract class CacheInvalidator {
             return result;
         }
 
-        private boolean noChangeInRevision(NodeDocument latestDoc, NodeDocument cachedDoc) {
-            if (Objects.equal(latestDoc.getModCount(), cachedDoc.getModCount())) {
-                return true;
-            }
-            //TODO This is a brute force check. Check if we need to compare for rev which are
-            // visible to current cluster node only etc
-            return Objects.equal(latestDoc.getLastRev(), cachedDoc.getLastRev());
-        }
-
         private TreeNode constructTreeFromPaths(Set<String> ids) {
             TreeNode root = new TreeNode("");
             for (String id : ids) {

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java?rev=1542183&r1=1542182&r2=1542183&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/mongomk/NodeDocument.java Fri Nov 15 06:49:56 2013
@@ -29,6 +29,7 @@ import java.util.NavigableMap;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.TreeMap;
+import java.util.concurrent.atomic.AtomicLong;
 
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
@@ -155,7 +156,7 @@ public class NodeDocument extends Docume
     /**
      * Time at which this object was check for cache consistency
      */
-    private long lastCheckTime = System.currentTimeMillis();
+    private final AtomicLong lastCheckTime = new AtomicLong(System.currentTimeMillis());
 
     private final long time = System.currentTimeMillis();
 
@@ -202,7 +203,7 @@ public class NodeDocument extends Docume
      * @param checkTime time at which the check was performed
      */
     public void markUptodate(long checkTime){
-        this.lastCheckTime = checkTime;
+        this.lastCheckTime.set(checkTime);
     }
 
     /**
@@ -211,14 +212,14 @@ public class NodeDocument extends Docume
      * @param lastCheckTime time at which current cycle started
      */
     public boolean isUptodate(long lastCheckTime){
-        return lastCheckTime <= this.lastCheckTime;
+        return lastCheckTime <= this.lastCheckTime.get();
     }
 
     /**
      * Returns the last time when this object was checked for consistency
      */
     public long getLastCheckTime() {
-        return lastCheckTime;
+        return lastCheckTime.get();
     }
 
     /**

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidationIT.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidationIT.java?rev=1542183&r1=1542182&r2=1542183&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidationIT.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/mongomk/CacheInvalidationIT.java Fri Nov 15 06:49:56 2013
@@ -113,8 +113,8 @@ public class CacheInvalidationIT extends
         //itself
         assertEquals(2, result.invalidationCount);
 
-        //All excluding /a and /a/d would be updated
-        assertEquals(totalPaths - 2, result.uptodateCount);
+        //All excluding /a and /a/d would be updated. Also we exclude / from processing
+        assertEquals(totalPaths - 3, result.uptodateCount);
 
         //3 queries would be fired for [/] [/a] [/a/b, /a/c, /a/d]
         assertEquals(2, result.queryCount);