Author: mreutegg
Date: Wed Oct 8 15:59:30 2014
New Revision: 1630156
URL: http://svn.apache.org/r1630156
Log:
OAK-2144: Intermittent Node not found at given revision with DocumentNodeStore
Detect when _lastRevs are ambiguous and use readRevision instead.
Enable tests.
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1630156&r1=1630155&r2=1630156&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
Wed Oct 8 15:59:30 2014
@@ -823,7 +823,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;
}
}
@@ -1201,6 +1205,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.
*
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java?rev=1630156&r1=1630155&r2=1630156&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ClusterRevisionComparisonTest.java
Wed Oct 8 15:59:30 2014
@@ -36,7 +36,6 @@ 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;
@@ -52,7 +51,6 @@ public class ClusterRevisionComparisonTe
Revision.setClock(clock);
}
- @Ignore("OAK-2144") //FIX ME OAK-2144
@Test
public void revisionComparisonMultipleClusterNode() throws Exception{
DocumentNodeStore c1 = createNS(1);
@@ -109,7 +107,6 @@ public class ClusterRevisionComparisonTe
c1ns1.compareAgainstBaseState(c1ns2, new ClusterTest.TrackingDiff());
}
- @Ignore("OAK-2144")
@Test
public void revisionComparisonTwoClusterNodes() throws Exception {
DocumentNodeStore c1 = createNS(1);
@@ -139,8 +136,8 @@ public class ClusterRevisionComparisonTe
// 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)c1ns1.getChildNode("a")).getLastRevision());
- c1.invalidateNodeCache("/a/c2" ,
((DocumentNodeState)c1ns1.getChildNode("a")).getLastRevision());
+ 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);
@@ -148,6 +145,19 @@ public class ClusterRevisionComparisonTe
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
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java?rev=1630156&r1=1630155&r2=1630156&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentTest.java
Wed Oct 8 15:59:30 2014
@@ -16,10 +16,17 @@
*/
package org.apache.jackrabbit.oak.plugins.document;
+import java.util.Comparator;
+
import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
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,4 +47,41 @@ public class NodeDocumentTest {
UpdateUtils.applyChanges(doc, op, StableRevisionComparator.INSTANCE);
doc.split(DummyRevisionContext.INSTANCE);
}
+
+ @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));
+ }
}
