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 2015/07/21 11:28:11 UTC
svn commit: r1692076 - 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: Tue Jul 21 09:28:10 2015
New Revision: 1692076
URL: http://svn.apache.org/r1692076
Log:
OAK-3104: Version garbage collector doesn't collect a rolled back document if it was never deleted
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
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/DocumentNodeStoreTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java?rev=1692076&r1=1692075&r2=1692076&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java Tue Jul 21 09:28:10 2015
@@ -448,10 +448,14 @@ public class Commit {
DocumentStore store = nodeStore.getDocumentStore();
for (UpdateOp op : changed) {
UpdateOp reverse = op.getReverseOperation();
+ if (op.isNew()) {
+ NodeDocument.setDeletedOnce(reverse);
+ }
store.findAndUpdate(NODES, reverse);
}
for (UpdateOp op : newDocuments) {
UpdateOp reverse = op.getReverseOperation();
+ NodeDocument.setDeletedOnce(reverse);
store.findAndUpdate(NODES, reverse);
}
UpdateOp removeCollision = new UpdateOp(commitRoot.getId(), false);
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=1692076&r1=1692075&r2=1692076&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 Tue Jul 21 09:28:10 2015
@@ -1372,10 +1372,13 @@ public final class NodeDocument extends
if(deleted) {
//DELETED_ONCE would be set upon every delete.
//possibly we can avoid that
- checkNotNull(op).set(DELETED_ONCE, Boolean.TRUE);
+ setDeletedOnce(op);
}
- checkNotNull(op).setMapEntry(DELETED, checkNotNull(revision),
- String.valueOf(deleted));
+ checkNotNull(op).setMapEntry(DELETED, checkNotNull(revision), String.valueOf(deleted));
+ }
+
+ public static void setDeletedOnce(@Nonnull UpdateOp op) {
+ checkNotNull(op).set(DELETED_ONCE, Boolean.TRUE);
}
public static void removeDeleted(@Nonnull UpdateOp op,
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1692076&r1=1692075&r2=1692076&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java Tue Jul 21 09:28:10 2015
@@ -242,15 +242,27 @@ public class DocumentNodeStoreTest {
final DocumentMK mk = new DocumentMK.Builder()
.setDocumentStore(docStore).setAsyncDelay(0).open();
final DocumentNodeStore store = mk.getNodeStore();
- final String head = mk.commit("/", "+\"foo\":{}+\"bar\":{}", null, null);
+
+ NodeBuilder builder = store.getRoot().builder();
+ builder.child("deletedNode");
+ builder.child("updateNode").setProperty("foo", "bar");
+ merge(store, builder);
+
+ builder = store.getRoot().builder();
+ builder.child("deletedNode").remove();
+ merge(store, builder);
+
+ final Revision head = store.getHeadRevision();
+
Thread writer = new Thread(new Runnable() {
@Override
public void run() {
try {
Revision r = store.newRevision();
- Commit c = new Commit(store, r, Revision.fromString(head), null);
- c.addNode(new DocumentNodeState(store, "/foo/node", r));
- c.addNode(new DocumentNodeState(store, "/bar/node", r));
+ Commit c = new Commit(store, r, head, null);
+ c.addNode(new DocumentNodeState(store, "/newConflictingNode", r));
+ c.addNode(new DocumentNodeState(store, "/deletedNode", r));
+ c.updateProperty("/updateNode", "foo", "baz");
c.apply();
} catch (DocumentStoreException e) {
exceptions.add(e);
@@ -265,21 +277,35 @@ public class DocumentNodeStoreTest {
created.acquireUninterruptibly();
// commit will succeed and add collision marker to writer commit
Revision r = store.newRevision();
- Commit c = new Commit(store, r, Revision.fromString(head), null);
- c.addNode(new DocumentNodeState(store, "/foo/node", r));
- c.addNode(new DocumentNodeState(store, "/bar/node", r));
+ Commit c = new Commit(store, r, head, null);
+ c.addNode(new DocumentNodeState(store, "/newConflictingNode", r));
+ c.addNode(new DocumentNodeState(store, "/newNonConflictingNode", r));
c.apply();
// allow writer to continue
s.release();
writer.join();
assertEquals("expected exception", 1, exceptions.size());
- String id = Utils.getIdFromPath("/foo/node");
+ String id = Utils.getIdFromPath("/newConflictingNode");
NodeDocument doc = docStore.find(NODES, id);
assertNotNull("document with id " + id + " does not exist", doc);
- id = Utils.getIdFromPath("/bar/node");
+ assertTrue("document with id " + id + " should get _deletedOnce marked due to rollback",
+ doc.wasDeletedOnce());
+
+ id = Utils.getIdFromPath("/newNonConflictingNode");
doc = docStore.find(NODES, id);
- assertNotNull("document with id " + id + " does not exist", doc);
+ assertNull("document with id " + id + " must not have _deletedOnce",
+ doc.get(NodeDocument.DELETED_ONCE));
+
+ id = Utils.getIdFromPath("/deletedNode");
+ doc = docStore.find(NODES, id);
+ assertTrue("document with id " + id + " should get _deletedOnce marked due to rollback",
+ doc.wasDeletedOnce());
+
+ id = Utils.getIdFromPath("/updateNode");
+ doc = docStore.find(NODES, id);
+ assertNull("document with id " + id + " must not have _deletedOnce despite rollback",
+ doc.get(NodeDocument.DELETED_ONCE));
mk.dispose();
}