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 re...@apache.org on 2014/10/24 15:45:39 UTC

svn commit: r1634055 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/document/rdb/RDBDocumentStore.java test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java

Author: reschke
Date: Fri Oct 24 13:45:39 2014
New Revision: 1634055

URL: http://svn.apache.org/r1634055
Log:
OAK-1941 - in queries, avoid re-parsing content from the DB when it's already in the cache

Modified:
    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/BasicDocumentStoreTest.java

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=1634055&r1=1634054&r2=1634055&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 Oct 24 13:45:39 2014
@@ -202,14 +202,12 @@ public class RDBDocumentStore implements
 
     @Override
     public <T extends Document> List<T> query(Collection<T> collection, String fromKey, String toKey, int limit) {
-        // TODO cache
         return query(collection, fromKey, toKey, null, 0, limit);
     }
 
     @Override
     public <T extends Document> List<T> query(Collection<T> collection, String fromKey, String toKey, String indexedProperty,
             long startValue, int limit) {
-        // TODO cache
         return internalQuery(collection, fromKey, toKey, indexedProperty, startValue, limit);
     }
 
@@ -727,10 +725,8 @@ public class RDBDocumentStore implements
             connection = getConnection();
             List<RDBRow> dbresult = dbQuery(connection, tableName, fromKey, toKey, indexedProperty, startValue, limit);
             for (RDBRow r : dbresult) {
-                T doc = SR.fromRow(collection, r);
-                doc.seal();
+                T doc = runThroughCache(collection, r);
                 result.add(doc);
-                addToCacheIfNotNewer(collection, doc);
             }
         } catch (Exception ex) {
             LOG.error("SQL exception on query", ex);
@@ -920,6 +916,8 @@ public class RDBDocumentStore implements
     private static boolean NOGZIP = Boolean.getBoolean("org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.NOGZIP");
     // Number of documents to insert at once for batch create
     private static int CHUNKSIZE = Integer.getInteger("org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.CHUNKSIZE", 64);
+    // Whether to use cache for query results
+    private static boolean NOQUERYFROMCACHE = Boolean.getBoolean("org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore.NOQUERYFROMCACHE");
 
     private static byte[] asBytes(String data) {
         byte[] bytes;
@@ -1306,32 +1304,59 @@ public class RDBDocumentStore implements
         }
     }
 
-    private <T extends Document> void addToCacheIfNotNewer(Collection<T> collection, T doc) {
-        if (collection == Collection.NODES) {
-            String id = doc.getId();
-            Lock lock = getAndLock(id);
-            CacheValue cacheKey = new StringValue(id);
-            try {
-                // do not overwrite document in cache if the
-                // existing one in the cache is newer
-                NodeDocument cached = nodesCache.getIfPresent(cacheKey);
-                if (cached != null && cached != NodeDocument.NULL) {
-                    // check mod count
-                    Number cachedModCount = cached.getModCount();
-                    Number modCount = doc.getModCount();
-                    if (cachedModCount == null || modCount == null) {
-                        throw new IllegalStateException("Missing " + Document.MOD_COUNT);
-                    }
-                    if (modCount.longValue() > cachedModCount.longValue()) {
-                        nodesCache.put(cacheKey, (NodeDocument) doc);
-                    }
+    private <T extends Document> T runThroughCache(Collection<T> collection, RDBRow row) {
+
+        if (collection != Collection.NODES) {
+            // not in the cache anyway
+            return SR.fromRow(collection, row);
+        }
+
+        String id = row.getId();
+        CacheValue cacheKey = new StringValue(id);
+        NodeDocument inCache = nodesCache.getIfPresent(cacheKey);
+        Number modCount = row.getModcount();
+
+        if (! NOQUERYFROMCACHE) {
+            // do not overwrite document in cache if the
+            // existing one in the cache is newer
+            if (inCache != null && inCache != NodeDocument.NULL) {
+                // check mod count
+                Number cachedModCount = inCache.getModCount();
+                if (cachedModCount == null) {
+                    throw new IllegalStateException("Missing " + Document.MOD_COUNT);
+                }
+                if (modCount.longValue() <= cachedModCount.longValue()) {
+                    // we can use the cached document
+                    return (T) inCache;
+                }
+            }
+        }
+
+        NodeDocument fresh = (NodeDocument) SR.fromRow(collection, row);
+        fresh.seal();
+
+        Lock lock = getAndLock(id);
+        try {
+            inCache = nodesCache.getIfPresent(cacheKey);
+            if (inCache != null && inCache != NodeDocument.NULL) {
+                // check mod count
+                Number cachedModCount = inCache.getModCount();
+                if (cachedModCount == null) {
+                    throw new IllegalStateException("Missing " + Document.MOD_COUNT);
+                }
+                if (modCount.longValue() > cachedModCount.longValue()) {
+                    nodesCache.put(cacheKey, fresh);
                 } else {
-                    nodesCache.put(cacheKey, (NodeDocument) doc);
+                    fresh = inCache;
                 }
-            } finally {
-                lock.unlock();
             }
+            else {
+                nodesCache.put(cacheKey, fresh);
+            }
+        } finally {
+            lock.unlock();
         }
+        return (T) fresh;
     }
 
     private Connection getConnection() throws SQLException {

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java?rev=1634055&r1=1634054&r2=1634055&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/BasicDocumentStoreTest.java Fri Oct 24 13:45:39 2014
@@ -403,7 +403,16 @@ public class BasicDocumentStoreTest exte
 
     @Test
     public void testPerfCollectionPaging() {
-        String cid = this.getClass().getName() + ".testPerfCollectionPaging";
+        testPerfCollectionPaging(this.getClass().getName() + ".testPerfCollectionPaging", false);
+    }
+
+    @Test
+    public void testPerfCollectionPagingUnCached() {
+        testPerfCollectionPaging(this.getClass().getName() + ".testPerfCollectionPagingUnCached", true);
+    }
+
+    private void testPerfCollectionPaging(String name, boolean invalidateCache) {
+        String cid = name;
         int nodecount = 20000;
         int initialFetchCount = 100;
         int maxFetchCount = 1600;
@@ -426,6 +435,7 @@ public class BasicDocumentStoreTest exte
 
         boolean success = super.ds.create(Collection.NODES, ups);
         assertTrue(success);
+        super.ds.invalidateCache();
 
         long end = System.currentTimeMillis() + duration;
         String sid = cid;
@@ -449,10 +459,13 @@ public class BasicDocumentStoreTest exte
                 }
             }
             cnt += 1;
+            if (invalidateCache) {
+                super.ds.invalidateCache();
+            }
         }
 
-        LOG.info("collection lookups " + super.dsname + " was "
-                + cnt + " in " + duration + "ms (" + (cnt / (duration / 1000f)) + "/s)");
+        LOG.info("collection lookups " + (invalidateCache ? "(uncached) " : "") + super.dsname + " was " + cnt + " in " + duration
+                + "ms (" + (cnt / (duration / 1000f)) + "/s)");
     }
 
     @Test