Author: mreutegg
Date: Wed Jul 15 12:06:14 2015
New Revision: 1691188
URL: http://svn.apache.org/r1691188
Log:
OAK-3103: Stale document in MongoDocumentStore cache
Fix and enable test
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/CacheConsistencyIT.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java?rev=1691188&r1=1691187&r2=1691188&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoDocumentStore.java
Wed Jul 15 12:06:14 2015
@@ -819,8 +819,15 @@ public class MongoDocumentStore implemen
if (oldDoc != null) {
putToCache(collection, oldDoc, updateOp);
oldDoc.seal();
+ } else if (upsert) {
+ if (collection == Collection.NODES) {
+ NodeDocument doc = (NodeDocument)
collection.newDocument(this);
+ UpdateUtils.applyChanges(doc, updateOp, comparator);
+ addToCache(doc);
+ }
} else {
- applyToCache(collection, null, updateOp);
+ // updateOp without conditions and not an upsert
+ // this means the document does not exist
}
return oldDoc;
} catch (Exception e) {
@@ -954,8 +961,7 @@ public class MongoDocumentStore implemen
// make sure concurrently loaded document is
invalidated
nodesCache.invalidate(new
StringValue(entry.getKey()));
} else {
- applyToCache(Collection.NODES,
entry.getValue(),
- updateOp.shallowCopy(entry.getKey()));
+ updateCache(Collection.NODES,
entry.getValue(), updateOp.shallowCopy(entry.getKey()));
}
} finally {
lock.unlock();
@@ -1163,43 +1169,34 @@ public class MongoDocumentStore implemen
*
* @param <T> the document type.
* @param collection the document collection.
- * @param oldDoc the old document or <code>null</code> if the update is for
- * a new document (insert).
+ * @param oldDoc the old document.
* @param updateOp the update operation.
*/
- private <T extends Document> void applyToCache(@Nonnull Collection<T>
collection,
- @Nullable T oldDoc,
- @Nonnull UpdateOp updateOp)
{
+ private <T extends Document> void updateCache(@Nonnull Collection<T>
collection,
+ @Nonnull T oldDoc,
+ @Nonnull UpdateOp updateOp) {
// cache the new document
if (collection == Collection.NODES) {
+ checkNotNull(oldDoc);
+ checkNotNull(updateOp);
+ // we can only update the cache based on the oldDoc if we
+ // still have the oldDoc in the cache, otherwise we may
+ // update the cache with an outdated document
CacheValue key = new StringValue(updateOp.getId());
- NodeDocument newDoc = (NodeDocument) collection.newDocument(this);
- if (oldDoc != null) {
- // we can only update the cache based on the oldDoc if we
- // still have the oldDoc in the cache, otherwise we may
- // update the cache with an outdated document
- NodeDocument cached = nodesCache.getIfPresent(key);
- if (cached == null) {
- // cannot use oldDoc to update cache
- return;
- }
- oldDoc.deepCopy(newDoc);
- }
- UpdateUtils.applyChanges(newDoc, updateOp, comparator);
- newDoc.seal();
-
- NodeDocument cached = addToCache(newDoc);
- if (cached == newDoc) {
- // successful
+ NodeDocument cached = nodesCache.getIfPresent(key);
+ if (cached == null) {
+ // cannot use oldDoc to update cache
return;
}
- if (oldDoc == null) {
- // this is an insert and some other thread was quicker
- // loading it into the cache -> return now
- return;
- }
- // this is an update (oldDoc != null)
+
+ // check if the currently cached document matches oldDoc
if (Objects.equal(cached.getModCount(), oldDoc.getModCount())) {
+ NodeDocument newDoc = (NodeDocument)
collection.newDocument(this);
+ oldDoc.deepCopy(newDoc);
+
+ UpdateUtils.applyChanges(newDoc, updateOp, comparator);
+ newDoc.seal();
+
nodesCache.put(key, newDoc);
} else {
// the cache entry was modified by some other thread in
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/CacheConsistencyIT.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/CacheConsistencyIT.java?rev=1691188&r1=1691187&r2=1691188&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/CacheConsistencyIT.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/CacheConsistencyIT.java
Wed Jul 15 12:06:14 2015
@@ -33,7 +33,6 @@ import org.apache.jackrabbit.oak.plugins
import org.apache.jackrabbit.oak.plugins.document.util.StringValue;
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
@@ -43,7 +42,6 @@ import static org.junit.Assert.assertTru
/**
* Test for OAK-3103
*/
-@Ignore
public class CacheConsistencyIT extends AbstractMongoConnectionTest {
private MongoDocumentStore store;
