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(); }