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/04/24 12:16:48 UTC
svn commit: r1589658 - in /jackrabbit/oak/branches/1.0: ./
oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/
oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/
oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/
Author: mreutegg
Date: Thu Apr 24 10:16:48 2014
New Revision: 1589658
URL: http://svn.apache.org/r1589658
Log:
OAK-1751: DocumentNodeStore may detect conflict too late
Added:
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentPropertyUpdateTest.java
- copied unchanged from r1589442, jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/ConcurrentPropertyUpdateTest.java
Modified:
jackrabbit/oak/branches/1.0/ (props changed)
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java
Propchange: jackrabbit/oak/branches/1.0/
------------------------------------------------------------------------------
Merged /jackrabbit/oak/trunk:r1589442
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java?rev=1589658&r1=1589657&r2=1589658&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Collision.java Thu Apr 24 10:16:48 2014
@@ -81,7 +81,7 @@ class Collision {
document.deepCopy(newDoc);
UpdateUtils.applyChanges(newDoc, ourOp, context.getRevisionComparator());
if (!markCommitRoot(newDoc, ourRev, store)) {
- throw new MicroKernelException("Unable to annotate our revision "
+ throw new IllegalStateException("Unable to annotate our revision "
+ "with collision marker. Our revision: " + ourRev
+ ", document:\n" + newDoc.format());
}
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java?rev=1589658&r1=1589657&r2=1589658&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/Commit.java Thu Apr 24 10:16:48 2014
@@ -275,6 +275,8 @@ public class Commit {
}
}
int commitRootDepth = PathUtils.getDepth(commitRootPath);
+ // check if there are real changes on the commit root
+ boolean commitRootHasChanges = operations.containsKey(commitRootPath);
// create a "root of the commit" if there is none
UpdateOp commitRoot = getUpdateOperationForNode(commitRootPath);
for (String p : operations.keySet()) {
@@ -283,7 +285,10 @@ public class Commit {
NodeDocument.setDeleted(op, revision, false);
}
if (op == commitRoot) {
- // apply at the end
+ if (!op.isNew() && commitRootHasChanges) {
+ // commit root already exists and this is an update
+ changedNodes.add(op);
+ }
} else {
NodeDocument.setCommitRoot(op, revision, commitRootDepth);
if (op.isNew()) {
@@ -327,11 +332,10 @@ public class Commit {
}
}
for (UpdateOp op : changedNodes) {
- // set commit root on changed nodes unless it's the
- // commit root itself
- if (op != commitRoot) {
- NodeDocument.setCommitRoot(op, revision, commitRootDepth);
- }
+ // set commit root on changed nodes. this may even apply
+ // to the commit root. the _commitRoot entry is removed
+ // again when the _revisions entry is set at the end
+ NodeDocument.setCommitRoot(op, revision, commitRootDepth);
opLog.add(op);
createOrUpdateNode(store, op);
}
@@ -340,7 +344,12 @@ public class Commit {
// first to check if there was a conflict, and only then to commit
// the revision, with the revision property set)
if (changedNodes.size() > 0 || !commitRoot.isNew()) {
+ // set revision to committed
NodeDocument.setRevision(commitRoot, revision, commitValue);
+ if (commitRootHasChanges) {
+ // remove previously added commit root
+ NodeDocument.removeCommitRoot(commitRoot, revision);
+ }
opLog.add(commitRoot);
if (baseBranchRevision == null) {
// create a clone of the commitRoot in order
@@ -554,7 +563,7 @@ public class Commit {
// or document did not exist before
return false;
}
- return doc.isConflicting(op, baseRevision, nodeStore);
+ return doc.isConflicting(op, baseRevision, revision, nodeStore);
}
/**
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java?rev=1589658&r1=1589657&r2=1589658&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/NodeDocument.java Thu Apr 24 10:16:48 2014
@@ -814,17 +814,22 @@ public final class NodeDocument extends
*
* @param op the update operation.
* @param baseRevision the base revision for the update operation.
+ * @param commitRevision the commit revision of the update operation.
* @param context the revision context.
* @return <code>true</code> if conflicting, <code>false</code> otherwise.
*/
- public boolean isConflicting(@Nonnull UpdateOp op,
+ boolean isConflicting(@Nonnull UpdateOp op,
@Nonnull Revision baseRevision,
+ @Nonnull Revision commitRevision,
@Nonnull RevisionContext context) {
// did existence of node change after baseRevision?
// only check local deleted map, which contains the most
// recent values
Map<Revision, String> deleted = getLocalDeleted();
for (Map.Entry<Revision, String> entry : deleted.entrySet()) {
+ if (entry.getKey().equals(commitRevision)) {
+ continue;
+ }
if (isRevisionNewer(context, entry.getKey(), baseRevision)) {
return true;
}
@@ -845,6 +850,9 @@ public final class NodeDocument extends
}
// was this property touched after baseRevision?
for (Revision rev : getValueMap(name).keySet()) {
+ if (rev.equals(commitRevision)) {
+ continue;
+ }
if (isRevisionNewer(context, rev, baseRevision)) {
return true;
}
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java?rev=1589658&r1=1589657&r2=1589658&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/AbstractNodeStoreBranch.java Thu Apr 24 10:16:48 2014
@@ -32,6 +32,7 @@ import static org.apache.jackrabbit.oak.
import javax.annotation.Nonnull;
+import org.apache.jackrabbit.mk.api.MicroKernelException;
import org.apache.jackrabbit.oak.api.CommitFailedException;
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.spi.commit.ChangeDispatcher;
@@ -500,9 +501,12 @@ public abstract class AbstractNodeStoreB
dispatcher.contentChanged(newHead, info);
branchState = new Merged(base);
return newHead;
- } catch (Exception e) {
+ } catch (MicroKernelException e) {
throw new CommitFailedException(MERGE, 1,
"Failed to merge changes to the underlying store", e);
+ } catch (Exception e) {
+ throw new CommitFailedException(OAK, 1,
+ "Failed to merge changes to the underlying store", e);
}
} finally {
dispatcher.contentChanged(getRoot(), null);