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/08 17:59:30 UTC
svn commit: r1630156 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/
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));
+ }
}
Re: svn commit: r1630156 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/ test/java/org/apache/jackrabbit/oak/plugins/document/
Posted by Chetan Mehrotra <ch...@gmail.com>.
Thats simple and precise solution to a nasty problem. :)
Chetan Mehrotra
On Wed, Oct 8, 2014 at 9:29 PM, <mr...@apache.org> wrote:
> 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));
> + }
> }
>
>