Author: mreutegg
Date: Wed Jul 8 10:22:53 2015
New Revision: 1689833
URL: http://svn.apache.org/r1689833
Log:
OAK-3081: SplitOperations may undo committed changes
Implemented fix and enable test
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.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/DocumentNodeStore.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1689833&r1=1689832&r2=1689833&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
Wed Jul 8 10:22:53 2015
@@ -1960,13 +1960,14 @@ public final class DocumentNodeStore
}
private void backgroundSplit() {
+ Revision head = getHeadRevision();
for (Iterator<String> it = splitCandidates.keySet().iterator();
it.hasNext();) {
String id = it.next();
NodeDocument doc = store.find(Collection.NODES, id);
if (doc == null) {
continue;
}
- for (UpdateOp op : doc.split(this)) {
+ for (UpdateOp op : doc.split(this, head)) {
NodeDocument before = store.createOrUpdate(Collection.NODES,
op);
if (before != null) {
if (LOG.isDebugEnabled()) {
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=1689833&r1=1689832&r2=1689833&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 Jul 8 10:22:53 2015
@@ -1040,14 +1040,21 @@ public final class NodeDocument extends
/**
* Returns update operations to split this document. The implementation may
- * decide to not return any operations if no splitting is required.
+ * decide to not return any operations if no splitting is required. A
caller
+ * must explicitly pass a head revision even though it is available through
+ * the {@link RevisionContext}. The given head revision must reflect a head
+ * state before {@code doc} was retrieved from the document store. This is
+ * important in order to maintain consistency. See OAK-3081 for details.
*
* @param context the revision context.
+ * @param head the head revision before this document was retrieved from
+ * the document store.
* @return the split operations.
*/
@Nonnull
- public Iterable<UpdateOp> split(@Nonnull RevisionContext context) {
- return SplitOperations.forDocument(this, context);
+ public Iterable<UpdateOp> split(@Nonnull RevisionContext context,
+ @Nonnull Revision head) {
+ return SplitOperations.forDocument(this, context, head);
}
/**
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java?rev=1689833&r1=1689832&r2=1689833&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/SplitOperations.java
Wed Jul 8 10:22:53 2015
@@ -70,6 +70,7 @@ class SplitOperations {
private final NodeDocument doc;
private final String path;
private final String id;
+ private final Revision headRevision;
private final RevisionContext context;
private Revision high;
private Revision low;
@@ -84,19 +85,27 @@ class SplitOperations {
private UpdateOp main;
private SplitOperations(@Nonnull NodeDocument doc,
- @Nonnull RevisionContext context) {
+ @Nonnull RevisionContext context,
+ @Nonnull Revision headRevision) {
this.doc = checkNotNull(doc);
this.context = checkNotNull(context);
this.path = doc.getPath();
this.id = doc.getId();
+ this.headRevision = checkNotNull(headRevision);
}
/**
* Creates a list of update operations in case the given document requires
- * a split.
+ * a split. A caller must explicitly pass a head revision even though it
+ * is available through the {@link RevisionContext}. The given head
revision
+ * must reflect a head state before {@code doc} was retrieved from the
+ * document store. This is important in order to maintain consistency.
+ * See OAK-3081 for details.
*
* @param doc a main document.
* @param context the revision context.
+ * @param headRevision the head revision before the document was retrieved
+ * from the document store.
* @return list of update operations. An empty list indicates the document
* does not require a split.
* @throws IllegalArgumentException if the given document is a split
@@ -104,12 +113,13 @@ class SplitOperations {
*/
@Nonnull
static List<UpdateOp> forDocument(@Nonnull NodeDocument doc,
- @Nonnull RevisionContext context) {
+ @Nonnull RevisionContext context,
+ @Nonnull Revision headRevision) {
if (doc.isSplitDocument()) {
throw new IllegalArgumentException(
"Not a main document: " + doc.getId());
}
- return new SplitOperations(doc, context).create();
+ return new SplitOperations(doc, context, headRevision).create();
}
@@ -392,9 +402,10 @@ class SplitOperations {
}
private boolean isGarbage(Revision rev) {
- Revision head = context.getHeadRevision();
Comparator<Revision> comp = context.getRevisionComparator();
- if (comp.compare(head, rev) <= 0) {
+ // use headRevision as passed in the constructor instead
+ // of the head revision from the RevisionContext. see OAK-3081
+ if (comp.compare(headRevision, rev) <= 0) {
// this may be an in-progress commit
return false;
}
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java?rev=1689833&r1=1689832&r2=1689833&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentSplitTest.java
Wed Jul 8 10:22:53 2015
@@ -44,7 +44,6 @@ import org.apache.jackrabbit.oak.spi.com
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
-import org.junit.Ignore;
import org.junit.Test;
import com.google.common.collect.Iterables;
@@ -537,7 +536,8 @@ public class DocumentSplitTest extends B
doc = store.find(NODES, id);
assertNotNull(doc);
- List<UpdateOp> splitOps =
Lists.newArrayList(doc.split(mk.getNodeStore()));
+ List<UpdateOp> splitOps = Lists.newArrayList(
+ doc.split(mk.getNodeStore(),
mk.getNodeStore().getHeadRevision()));
assertEquals(2, splitOps.size());
// first update op is for the new intermediate doc
op = splitOps.get(0);
@@ -592,7 +592,8 @@ public class DocumentSplitTest extends B
// must split document and create a previous document starting at
// the second most recent revision
- List<UpdateOp> splitOps =
Lists.newArrayList(doc.split(mk.getNodeStore()));
+ List<UpdateOp> splitOps = Lists.newArrayList(
+ doc.split(mk.getNodeStore(),
mk.getNodeStore().getHeadRevision()));
assertEquals(2, splitOps.size());
String prevId = Utils.getPreviousIdFor("/test", revs.get(revs.size() -
2), 0);
assertEquals(prevId, splitOps.get(0).getId());
@@ -668,7 +669,8 @@ public class DocumentSplitTest extends B
NodeDocument doc = new NodeDocument(mk.getDocumentStore());
doc.put(NodeDocument.ID, Utils.getIdFromPath("/test"));
doc.put(NodeDocument.SD_TYPE, NodeDocument.SplitDocType.DEFAULT.type);
- SplitOperations.forDocument(doc, DummyRevisionContext.INSTANCE);
+ Revision head = mk.getNodeStore().getHeadRevision();
+ SplitOperations.forDocument(doc, DummyRevisionContext.INSTANCE, head);
}
@Test
@@ -739,7 +741,6 @@ public class DocumentSplitTest extends B
}
// OAK-3081
- @Ignore
@Test
public void removeGarbage() throws Exception {
final DocumentStore store = mk.getDocumentStore();
@@ -776,8 +777,9 @@ public class DocumentSplitTest extends B
RevisionContext rc = new TestRevisionContext(ns);
while (t.isAlive()) {
for (String id : ns.getSplitCandidates()) {
+ Revision head = ns.getHeadRevision();
NodeDocument doc = store.find(NODES, id);
- List<UpdateOp> ops = SplitOperations.forDocument(doc, rc);
+ List<UpdateOp> ops = SplitOperations.forDocument(doc, rc,
head);
Set<Revision> removed = Sets.newHashSet();
Set<Revision> added = Sets.newHashSet();
for (UpdateOp op : ops) {
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=1689833&r1=1689832&r2=1689833&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 Jul 8 10:22:53 2015
@@ -45,7 +45,8 @@ public class NodeDocumentTest {
NodeDocument.addCollision(op, r);
}
UpdateUtils.applyChanges(doc, op, StableRevisionComparator.INSTANCE);
- doc.split(DummyRevisionContext.INSTANCE);
+ Revision head = DummyRevisionContext.INSTANCE.getHeadRevision();
+ doc.split(DummyRevisionContext.INSTANCE, head);
}
@Test
