Author: mreutegg
Date: Tue Jan 15 10:21:09 2019
New Revision: 1851334
URL: http://svn.apache.org/viewvc?rev=1851334&view=rev
Log:
OAK-7976: Non-blocking commit rollback
Added:
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Rollback.java
(with props)
Modified:
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitRootUpdateTest.java
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RollbackTest.java
Modified:
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java?rev=1851334&r1=1851333&r2=1851334&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
(original)
+++
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
Tue Jan 15 10:21:09 2019
@@ -64,7 +64,7 @@ public class Commit {
private final HashMap<String, UpdateOp> operations = new
LinkedHashMap<String, UpdateOp>();
private final Set<Revision> collisions = new LinkedHashSet<Revision>();
private Branch b;
- private boolean rollbackFailed;
+ private Rollback rollback = Rollback.NONE;
/**
* List of all node paths which have been modified in this commit. In
addition to the nodes
@@ -170,11 +170,21 @@ public class Commit {
}
/**
- * @return {@code true} if this commit did not succeed and the rollback
- * was unable to revert all changes; otherwise {@code false}.
+ * Performs a rollback of this commit if necessary.
+ *
+ * @return {@code false} if a rollback was necessary and the rollback did
+ * not complete successfully, {@code true} otherwise.
*/
- boolean rollbackFailed() {
- return rollbackFailed;
+ boolean rollback() {
+ boolean success = false;
+ try {
+ rollback.perform(this.nodeStore.getDocumentStore());
+ success = true;
+ } catch (Throwable t) {
+ // catch any exception caused by the rollback and log it
+ LOG.warn("Rollback failed", t);
+ }
+ return success;
}
/**
@@ -195,6 +205,7 @@ public class Commit {
success = true;
} finally {
if (!success) {
+ rollback();
Branch branch = getBranch();
if (branch != null) {
branch.removeCommit(rev.asBranchRevision());
@@ -263,10 +274,9 @@ public class Commit {
*/
private void applyToDocumentStore(RevisionVector baseBranchRevision)
throws ConflictException, DocumentStoreException {
- // initially set the rollbackFailed flag to true
- // the flag will be set to false at the end of the method
- // when the commit succeeds
- rollbackFailed = true;
+ // initially set the rollback to always fail until we have changes
+ // in an oplog list and a commit root.
+ rollback = Rollback.FAILED;
// the value in _revisions.<revision> property of the commit root node
// regular commits use "c", which makes the commit visible to
@@ -299,6 +309,8 @@ public class Commit {
}
}
+ rollback = new Rollback(revision, opLog,
Utils.getIdFromPath(commitRootPath));
+
for (String p : bundledNodes.keySet()){
markChanged(p);
}
@@ -386,14 +398,6 @@ public class Commit {
if (success) {
LOG.error("Exception occurred after commit. Rollback will be
suppressed.", e);
} else {
- try {
- rollback(opLog, commitRoot);
- rollbackFailed = false;
- } catch (Throwable ex) {
- // catch any exception caused by the rollback, log it
- // and throw the original exception
- LOG.warn("Rollback failed", ex);
- }
if (e instanceof ConflictException) {
throw e;
} else {
@@ -402,7 +406,7 @@ public class Commit {
}
} finally {
if (success) {
- rollbackFailed = false;
+ rollback = Rollback.NONE;
}
}
}
@@ -474,23 +478,6 @@ public class Commit {
}
}
- private void rollback(List<UpdateOp> changed,
- UpdateOp commitRoot) {
- DocumentStore store = nodeStore.getDocumentStore();
- List<UpdateOp> reverseOps = new ArrayList<>();
- for (UpdateOp op : changed) {
- UpdateOp reverse = op.getReverseOperation();
- if (op.isNew()) {
- NodeDocument.setDeletedOnce(reverse);
- }
- // do not create document if it doesn't exist
- reverse.setNew(false);
- reverseOps.add(reverse);
- }
- store.createOrUpdate(NODES, reverseOps);
- removeCollisionMarker(commitRoot.getId());
- }
-
/**
* Try to create or update the node. If there was a conflict, this method
* throws a {@link ConflictException}, even though the change is still
applied.
Modified:
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1851334&r1=1851333&r2=1851334&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
(original)
+++
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
Tue Jan 15 10:21:09 2019
@@ -499,8 +499,8 @@ public final class DocumentNodeStore
* is called and removed when the commit either:
* <ul>
* <li>Succeeds with {@link #done(Commit, boolean, CommitInfo)}</li>
- * <li>Fails with {@link #canceled(Commit)} and the commit does *not*
- * have the {@link Commit#rollbackFailed()} flag set.</li>
+ * <li>Fails with {@link #canceled(Commit)} and the commit was
+ * successfully rolled back.</li>
* </ul>
* The {@link NodeDocumentSweeper} periodically goes through this set and
* reverts changes done by commits in the set that are older than the
@@ -616,6 +616,7 @@ public final class DocumentNodeStore
try {
commit.applyToDocumentStore();
} catch (ConflictException e) {
+ commit.rollback();
throw new IllegalStateException("Conflict while creating root
document", e);
}
unsavedLastRevisions.put("/", commitRev);
@@ -956,15 +957,17 @@ public final class DocumentNodeStore
void canceled(Commit c) {
if (commitQueue.contains(c.getRevision())) {
try {
- if (!c.rollbackFailed() || c.isEmpty()) {
+ commitQueue.canceled(c.getRevision());
+ if (c.rollback()) {
+ // rollback was successful
inDoubtTrunkCommits.remove(c.getRevision());
}
- commitQueue.canceled(c.getRevision());
} finally {
backgroundOperationLock.readLock().unlock();
}
} else {
try {
+ c.rollback();
Branch b = branches.getBranch(c.getBaseRevision());
if (b != null) {
b.removeCommit(c.getRevision().asBranchRevision());
Added:
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Rollback.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Rollback.java?rev=1851334&view=auto
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Rollback.java
(added)
+++
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Rollback.java
Tue Jan 15 10:21:09 2019
@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.oak.plugins.document;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+import org.jetbrains.annotations.NotNull;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static org.apache.jackrabbit.oak.plugins.document.Collection.NODES;
+
+/**
+ * Performs a rollback of a commit based on a list of changes. Changes are
+ * rolled back by applying the reverse operation to the document store.
+ */
+class Rollback {
+
+ static final Rollback FAILED = new Rollback(Revision.newRevision(0),
+ Collections.emptyList(), "") {
+
+ @Override
+ void perform(@NotNull DocumentStore store) throws
DocumentStoreException {
+ throw new DocumentStoreException("rollback failed");
+ }
+ };
+
+ static final Rollback NONE = new Rollback(Revision.newRevision(0),
+ Collections.emptyList(), "") {
+
+ @Override
+ void perform(@NotNull DocumentStore store) throws
DocumentStoreException {
+ }
+ };
+
+ private final Revision revision;
+
+ private final List<UpdateOp> changed;
+
+ private final String commitRootId;
+
+ /**
+ * Creates a new rollback for the given commit revision.
+ *
+ * @param revision the commit revision.
+ * @param changed the changes to revert.
+ * @param commitRootId the id of the commit root document.
+ */
+ Rollback(@NotNull Revision revision,
+ @NotNull List<UpdateOp> changed,
+ @NotNull String commitRootId) {
+ this.revision = revision;
+ this.changed = checkNotNull(changed);
+ this.commitRootId = checkNotNull(commitRootId);
+ }
+
+ /**
+ * Performs the rollback. If this operation fails with a
+ * {@link DocumentStoreException}, then only some of the rollback may have
+ * been performed.
+ *
+ * @param store the store where to apply the rollback.
+ * @throws DocumentStoreException if any of the operations fails.
+ */
+ void perform(@NotNull DocumentStore store) throws DocumentStoreException {
+ checkNotNull(store);
+ List<UpdateOp> reverseOps = new ArrayList<>();
+ for (UpdateOp op : changed) {
+ UpdateOp reverse = op.getReverseOperation();
+ if (op.isNew()) {
+ NodeDocument.setDeletedOnce(reverse);
+ }
+ // do not create document if it doesn't exist
+ reverse.setNew(false);
+ reverseOps.add(reverse);
+ }
+ store.createOrUpdate(NODES, reverseOps);
+ removeCollisionMarker(store, commitRootId);
+ }
+
+ private void removeCollisionMarker(DocumentStore store, String id) {
+ UpdateOp removeCollision = new UpdateOp(id, false);
+ NodeDocument.removeCollision(removeCollision, revision);
+ store.findAndUpdate(NODES, removeCollision);
+ }
+}
Propchange:
jackrabbit/oak/trunk/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/Rollback.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified:
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitRootUpdateTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitRootUpdateTest.java?rev=1851334&r1=1851333&r2=1851334&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitRootUpdateTest.java
(original)
+++
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/CommitRootUpdateTest.java
Tue Jan 15 10:21:09 2019
@@ -30,6 +30,7 @@ import org.apache.jackrabbit.oak.spi.sta
import org.junit.Rule;
import org.junit.Test;
+import static
org.apache.jackrabbit.oak.plugins.document.TestUtils.isFinalCommitRootUpdate;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -55,20 +56,6 @@ public class CommitRootUpdateTest {
}
return doc;
}
-
- private boolean isFinalCommitRootUpdate(UpdateOp update) {
- boolean finalUpdate = true;
- for (Map.Entry<Key, Operation> op :
update.getChanges().entrySet()) {
- String name = op.getKey().getName();
- if (NodeDocument.isRevisionsEntry(name)
- || NodeDocument.MODIFIED_IN_SECS.equals(name)) {
- continue;
- }
- finalUpdate = false;
- break;
- }
- return finalUpdate;
- }
};
DocumentNodeStore ns = builderProvider.newBuilder()
Modified:
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java?rev=1851334&r1=1851333&r2=1851334&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
(original)
+++
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
Tue Jan 15 10:21:09 2019
@@ -294,13 +294,15 @@ public class DocumentNodeStoreTest {
Thread writer = new Thread(new Runnable() {
@Override
public void run() {
+ Commit c = new CommitBuilder(store, store.newRevision(), head)
+ .addNode("/newConflictingNode")
+ .addNode("/deletedNode")
+ .updateProperty("/updateNode", "foo", "baz")
+ .build();
try {
- new CommitBuilder(store, store.newRevision(), head)
- .addNode("/newConflictingNode")
- .addNode("/deletedNode")
- .updateProperty("/updateNode", "foo", "baz")
- .build().apply();
+ c.apply();
} catch (Exception e) {
+ c.rollback();
exceptions.add(e);
}
}
Modified:
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RollbackTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RollbackTest.java?rev=1851334&r1=1851333&r2=1851334&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RollbackTest.java
(original)
+++
jackrabbit/oak/trunk/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RollbackTest.java
Tue Jan 15 10:21:09 2019
@@ -36,7 +36,6 @@ import org.apache.jackrabbit.oak.stats.C
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Rule;
import org.junit.Test;
@@ -74,7 +73,6 @@ public class RollbackTest {
Revision.resetClockToDefault();
}
- @Ignore("OAK-7976")
@Test
public void nonBlocking() throws Exception {
TestStore store = new TestStore();
@@ -121,6 +119,16 @@ public class RollbackTest {
greaterThanOrEqualTo(1000L));
}
+ @Test(expected = DocumentStoreException.class)
+ public void rollbackFailed() {
+ Rollback.FAILED.perform(new MemoryDocumentStore());
+ }
+
+ @Test
+ public void rollbackNone() {
+ Rollback.NONE.perform(new MemoryDocumentStore());
+ }
+
private class TestStore extends MemoryDocumentStore {
final AtomicBoolean failCommitOnce = new AtomicBoolean();
