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 mr...@apache.org on 2014/10/09 13:32:38 UTC
svn commit: r1630372 - in /jackrabbit/oak/branches/1.0: ./
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/
Author: mreutegg
Date: Thu Oct 9 11:32:38 2014
New Revision: 1630372
URL: http://svn.apache.org/r1630372
Log:
OAK-2144: Intermittent Node not found at given revision with DocumentNodeStore
Merged revisions 1629688,1629840,1629917,1630156 from trunk
Added:
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java
- copied, changed from r1629688, jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java
Modified:
jackrabbit/oak/branches/1.0/ (props changed)
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterTest.java
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
Propchange: jackrabbit/oak/branches/1.0/
------------------------------------------------------------------------------
Merged /jackrabbit/oak/trunk:r1629688,1629840,1629917,1630156
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1630372&r1=1630371&r2=1630372&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Thu Oct 9 11:32:38 2014
@@ -610,6 +610,10 @@ public final class DocumentNodeStore
docChildrenCache.invalidateAll();
}
+ void invalidateNodeCache(String path, Revision revision){
+ nodeCache.invalidate(new PathRev(path, revision));
+ }
+
public int getPendingWriteCount() {
return unsavedLastRevisions.getPaths().size();
}
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1630372&r1=1630371&r2=1630372&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Thu Oct 9 11:32:38 2014
@@ -808,7 +808,11 @@ public final class NodeDocument extends
// changes is the base of the branch
r = branchBase;
}
- if (isRevisionNewer(nodeStore, r, lastRevision)) {
+ if (revisionAreAmbiguous(nodeStore, r, lastRevision)) {
+ // _lastRev entries from multiple cluster nodes are ambiguous
+ // use readRevision to make sure read is consistent
+ lastRevision = readRevision;
+ } else if (isRevisionNewer(nodeStore, r, lastRevision)) {
lastRevision = r;
}
}
@@ -1342,6 +1346,34 @@ public final class NodeDocument extends
//----------------------------< internal >----------------------------------
/**
+ * Returns {@code true} if the two revisions are ambiguous. That is, they
+ * are from different cluster nodes and the comparison of the two revision
+ * depends on the seen at revision and is different when just comparing the
+ * timestamps of the revisions.
+ *
+ * @param context the revision context.
+ * @param r1 the first revision.
+ * @param r2 the second revision.
+ * @return {@code true} if ambiguous, {@code false} otherwise.
+ */
+ static boolean revisionAreAmbiguous(@Nonnull RevisionContext context,
+ @Nonnull Revision r1,
+ @Nonnull Revision r2) {
+ if (r1.getClusterId() == r2.getClusterId()) {
+ return false;
+ }
+ int c1 = context.getRevisionComparator().compare(r1, r2);
+ int c2 = r1.compareRevisionTimeThenClusterId(r2);
+ if (c1 == 0) {
+ return c2 == 0;
+ } else if (c1 < 0) {
+ return c2 >= 0;
+ } else {
+ return c2 <= 0;
+ }
+ }
+
+ /**
* Returns the commit root document for the given revision. This may either
* be this document or another one.
*
Copied: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java (from r1629688, jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java)
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java?p2=jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java&p1=jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java&r1=1629688&r2=1630372&rev=1630372&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java Thu Oct 9 11:32:38 2014
@@ -19,7 +19,11 @@
package org.apache.jackrabbit.oak.plugins.document;
+import java.util.List;
+
import com.google.common.collect.Iterables;
+import com.google.common.collect.Lists;
+
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
@@ -32,20 +36,21 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.stats.Clock;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
+import static org.junit.Assert.assertTrue;
+
public class ClusterRevisionComparisonTest {
private MemoryDocumentStore ds = new MemoryDocumentStore();
private MemoryBlobStore bs = new MemoryBlobStore();
private Clock clock = new Clock.Virtual();
+ private List<DocumentNodeStore> stores = Lists.newArrayList();
@Before
public void setUp(){
Revision.setClock(clock);
}
- @Ignore("OAK-2144") //FIX ME OAK-2144
@Test
public void revisionComparisonMultipleClusterNode() throws Exception{
DocumentNodeStore c1 = createNS(1);
@@ -71,7 +76,7 @@ public class ClusterRevisionComparisonTe
runBgOps(c2);
//6. Time T4. Read the changes /a/c2 by c2 created at T1.
- // Would be considered seen ar T4 i.e. rT1-C2 -> rT4-C1
+ // Would be considered seen at T4 i.e. rT1-C2 -> rT4-C1
// Now from C1 view rT1-C2 > rT2-C3 even though T1 < T2
//so effectively changes done in future in C3 in absolute time terms
//is considered to be seen in past by C1
@@ -87,10 +92,14 @@ public class ClusterRevisionComparisonTe
c1.invalidateNodeCache("/a/c2" , ((DocumentNodeState)c1ns1.getChildNode("a")).getLastRevision());
c1.invalidateNodeCache("/a/c3" , ((DocumentNodeState)c1ns1.getChildNode("a")).getLastRevision());
- //Revision compartor purge by moving in future
+ //Revision comparator purge by moving in future
clock.waitUntil(clock.getTime() + DocumentNodeStore.REMEMBER_REVISION_ORDER_MILLIS * 2);
runBgOps(c1);
+ NodeState a = c1ns1.getChildNode("a");
+ assertTrue("/a/c2 disappeared", a.hasChildNode("c2"));
+ assertTrue("/a/c3 disappeared", a.hasChildNode("c3"));
+
DocumentNodeState c1ns2 = c1.getRoot();
//Trigger a diff. With OAK-2144 an exception would be thrown as diff traverses
@@ -98,19 +107,78 @@ public class ClusterRevisionComparisonTe
c1ns1.compareAgainstBaseState(c1ns2, new ClusterTest.TrackingDiff());
}
+ @Test
+ public void revisionComparisonTwoClusterNodes() throws Exception {
+ DocumentNodeStore c1 = createNS(1);
+ DocumentNodeStore c2 = createNS(2);
+
+ // 1. Create /a and make it visible to both cluster nodes
+ createNode(c1, "/a");
+ runBgOps(c1, c2);
+
+ // 2. Time T1. Create /a/c2 but do not push the changes yet rT1-C2
+ createNode(c2, "/a/c2");
+
+ // 3. Time T2. Create /a/c1
+ createNode(c1, "/a/c1");
+
+ // 4. Push changes on c2
+ runBgOps(c2);
+
+ // 5. Time T3. Read the changes /a/c2 by c2 created at T1.
+ // Now from C1 view rT1-C2 > rT2-C1 even though T1 < T2
+ runBgOps(c1);
+
+ DocumentNodeState c1ns1 = c1.getRoot();
+ NodeState a = c1ns1.getChildNode("a");
+ assertTrue("/a/c1 missing", a.hasChildNode("c1"));
+ assertTrue("/a/c2 missing", a.hasChildNode("c2"));
+
+ // 6. Purge revision comparator. Also purge entries from nodeCache
+ // such that later reads at rT1-C2 triggers read from underlying DocumentStore
+ c1.invalidateNodeCache("/a/c1" , ((DocumentNodeState)a).getLastRevision());
+ c1.invalidateNodeCache("/a/c2" , ((DocumentNodeState)a).getLastRevision());
+
+ //Revision comparator purge by moving in future
+ clock.waitUntil(clock.getTime() + DocumentNodeStore.REMEMBER_REVISION_ORDER_MILLIS * 2);
+ runBgOps(c1);
+
+ assertTrue("/a/c1 disappeared", a.hasChildNode("c1"));
+ assertTrue("/a/c2 disappeared", a.hasChildNode("c2"));
+
+ // read again starting at root node with a invalidated cache
+ // and purged revision comparator
+ c1ns1 = c1.getRoot();
+ c1.invalidateNodeCache("/", c1ns1.getRevision());
+ c1ns1 = c1.getRoot();
+ c1.invalidateNodeCache("/a", c1ns1.getLastRevision());
+ assertTrue("/a missing", c1ns1.hasChildNode("a"));
+ a = c1ns1.getChildNode("a");
+ c1.invalidateNodeCache("/a/c1", ((DocumentNodeState)a).getLastRevision());
+ c1.invalidateNodeCache("/a/c2", ((DocumentNodeState)a).getLastRevision());
+ assertTrue("/a/c1 disappeared", a.hasChildNode("c1"));
+ assertTrue("/a/c2 disappeared", a.hasChildNode("c2"));
+ }
+
@After
public void tearDown(){
+ for (DocumentNodeStore store : stores) {
+ store.dispose();
+ }
+ stores.clear();
Revision.resetClockToDefault();
}
private DocumentNodeStore createNS(int clusterId){
- return new DocumentMK.Builder()
+ DocumentNodeStore store = new DocumentMK.Builder()
.setDocumentStore(ds)
.setBlobStore(bs)
.setClusterId(clusterId)
.setAsyncDelay(0)
.open()
.getNodeStore();
+ stores.add(store);
+ return store;
}
private NodeState createNode(NodeStore ns, String path) throws CommitFailedException {
Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterTest.java?rev=1630372&r1=1630371&r2=1630372&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterTest.java Thu Oct 9 11:32:38 2014
@@ -416,7 +416,7 @@ public class ClusterTest {
}
}
- private static class TrackingDiff extends DefaultNodeStateDiff {
+ static class TrackingDiff extends DefaultNodeStateDiff {
final String path;
final Set<String> added;
Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java?rev=1630372&r1=1630371&r2=1630372&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java Thu Oct 9 11:32:38 2014
@@ -22,6 +22,11 @@ import org.apache.jackrabbit.oak.plugins
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
import org.junit.Test;
+import static org.apache.jackrabbit.oak.plugins.document.NodeDocument.revisionAreAmbiguous;
+import static org.apache.jackrabbit.oak.plugins.document.Revision.RevisionComparator;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
/**
* Tests for {@link NodeDocument}.
*/
@@ -40,10 +45,12 @@ public class NodeDocumentTest {
NodeDocument.addCollision(op, r);
}
UpdateUtils.applyChanges(doc, op, StableRevisionComparator.INSTANCE);
- doc.split(CONTEXT);
+ doc.split(DummyRevisionContext.INSTANCE);
}
- private static final RevisionContext CONTEXT = new RevisionContext() {
+ private static class DummyRevisionContext implements RevisionContext {
+
+ static final RevisionContext INSTANCE = new DummyRevisionContext();
private final Comparator<Revision> comparator
= StableRevisionComparator.INSTANCE;
@@ -67,5 +74,42 @@ public class NodeDocumentTest {
public int getClusterId() {
return 1;
}
- };
+ }
+
+ @Test
+ public void ambiguousRevisions() {
+ // revisions from same cluster node are not ambiguous
+ RevisionContext context = DummyRevisionContext.INSTANCE;
+ Revision r1 = new Revision(1, 0, 1);
+ Revision r2 = new Revision(2, 0, 1);
+ assertFalse(revisionAreAmbiguous(context, r1, r1));
+ assertFalse(revisionAreAmbiguous(context, r1, r2));
+ assertFalse(revisionAreAmbiguous(context, r2, r1));
+
+ // revisions from different cluster nodes are not ambiguous
+ // if seen with stable revision comparator
+ r1 = new Revision(1, 0, 2);
+ r2 = new Revision(2, 0, 1);
+ assertFalse(revisionAreAmbiguous(context, r1, r1));
+ assertFalse(revisionAreAmbiguous(context, r1, r2));
+ assertFalse(revisionAreAmbiguous(context, r2, r1));
+
+ // now use a revision comparator with seen-at support
+ final RevisionComparator comparator = new RevisionComparator(1);
+ context = new DummyRevisionContext() {
+ @Override
+ public Comparator<Revision> getRevisionComparator() {
+ return comparator;
+ }
+ };
+ r1 = new Revision(1, 0, 2);
+ r2 = new Revision(2, 0, 1);
+ // add revision to comparator in reverse time order
+ comparator.add(r2, new Revision(2, 0, 0));
+ comparator.add(r1, new Revision(3, 0, 0)); // r1 seen after r2
+ assertFalse(revisionAreAmbiguous(context, r1, r1));
+ assertFalse(revisionAreAmbiguous(context, r2, r2));
+ assertTrue(revisionAreAmbiguous(context, r1, r2));
+ assertTrue(revisionAreAmbiguous(context, r2, r1));
+ }
}