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/09/22 17:30:13 UTC
svn commit: r1704655 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/mongo/
Author: mreutegg
Date: Tue Sep 22 15:30:08 2015
New Revision: 1704655
URL: http://svn.apache.org/viewvc?rev=1704655&view=rev
Log:
OAK-3433: Commit does not detect conflict when background read happens after rebase
Add yet another test, enable existing test and implement fix
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/LastRevRecoveryAgent.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.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=1704655&r1=1704654&r2=1704655&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 Tue Sep 22 15:30:08 2015
@@ -2084,9 +2084,9 @@ public final class DocumentNodeStore
BackgroundWriteStats backgroundWrite() {
return unsavedLastRevisions.persist(this, new UnsavedModifications.Snapshot() {
@Override
- public void acquiring() {
+ public void acquiring(Revision mostRecent) {
if (store.create(JOURNAL,
- singletonList(changes.asUpdateOp(getHeadRevision())))) {
+ singletonList(changes.asUpdateOp(mostRecent)))) {
changes = JOURNAL.newDocument(getDocumentStore());
}
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java?rev=1704655&r1=1704654&r2=1704655&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java Tue Sep 22 15:30:08 2015
@@ -235,7 +235,7 @@ public class LastRevRecoveryAgent {
unsaved.persist(nodeStore, new UnsavedModifications.Snapshot() {
@Override
- public void acquiring() {
+ public void acquiring(Revision mostRecent) {
if (lastRootRev == null) {
// this should never happen - when unsaved has no changes
// that is reflected in the 'map' to be empty - in that
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java?rev=1704655&r1=1704654&r2=1704655&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java Tue Sep 22 15:30:08 2015
@@ -159,7 +159,7 @@ class UnsavedModifications {
time = clock.getTime();
Map<String, Revision> pending;
try {
- snapshot.acquiring();
+ snapshot.acquiring(getMostRecentRevision());
pending = Maps.newTreeMap(PathComparator.INSTANCE);
pending.putAll(map);
} finally {
@@ -234,14 +234,26 @@ class UnsavedModifications {
return map.toString();
}
+ private Revision getMostRecentRevision() {
+ // use revision of root document
+ Revision rev = map.get("/");
+ // otherwise find most recent
+ if (rev == null) {
+ for (Revision r : map.values()) {
+ rev = Utils.max(rev, r);
+ }
+ }
+ return rev;
+ }
+
public interface Snapshot {
Snapshot IGNORE = new Snapshot() {
@Override
- public void acquiring() {
+ public void acquiring(Revision mostRecent) {
}
};
- void acquiring();
+ void acquiring(Revision mostRecent);
}
}
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java?rev=1704655&r1=1704654&r2=1704655&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java Tue Sep 22 15:30:08 2015
@@ -444,6 +444,31 @@ public class JournalTest extends Abstrac
}
}
+ // OAK-3433
+ @Test
+ public void journalEntryKey() throws Exception {
+ DocumentNodeStore ns1 = createMK(1, 0).getNodeStore();
+ DocumentNodeStore ns2 = createMK(2, 0).getNodeStore();
+
+ NodeBuilder b1 = ns1.getRoot().builder();
+ b1.child("foo");
+ ns1.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ ns1.runBackgroundOperations();
+
+ NodeBuilder b2 = ns2.getRoot().builder();
+ b2.child("bar");
+ ns2.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
+ Revision h2 = ns2.getHeadRevision();
+
+ ns2.runBackgroundReadOperations();
+
+ ns2.runBackgroundOperations();
+
+ String id = JournalEntry.asId(h2);
+ assertTrue("Background update did not create a journal entry with id " + id,
+ ns1.getDocumentStore().find(Collection.JOURNAL, id) != null);
+ }
+
private DocumentMK createMK(int clusterId, int asyncDelay) {
if (ds == null) {
ds = new MemoryDocumentStore();
Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java?rev=1704655&r1=1704654&r2=1704655&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java Tue Sep 22 15:30:08 2015
@@ -32,10 +32,8 @@ 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 static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
@@ -61,7 +59,6 @@ public class ClusterConflictTest extends
}
// OAK-3433
- @Ignore
@Test
public void mergeRetryWhileBackgroundRead() throws Exception {
DocumentNodeStore ns1 = mk.getNodeStore();
@@ -100,7 +97,7 @@ public class ClusterConflictTest extends
// because /a/b/c is seen as changed in the future
// without the fix for OAK-3433:
// the second merge attempt succeeds because now the
- // /a/b/c change is visible and happened before the commit
+ // /a/b/c change revision is visible and happened before the commit
// revision but before the base revision
b2 = ns2.getRoot().builder();
b2.child("z").setProperty("q", "v");
@@ -115,13 +112,18 @@ public class ClusterConflictTest extends
runBackgroundRead(ns2);
NodeBuilder builder = after.builder();
- builder.child("a").child("b").child("c").child("bar");
+ if (builder.getChildNode("a").getChildNode("b").hasChildNode("c")) {
+ builder.child("a").child("b").child("c").child("bar");
+ } else {
+ throw new CommitFailedException(
+ CommitFailedException.OAK, 0,
+ "/a/b/c does not exist anymore");
+ }
return builder.getNodeState();
}
}, CommitInfo.EMPTY);
fail("Merge must fail with CommitFailedException");
} catch (CommitFailedException e) {
- e.printStackTrace();
// expected
}
}
Re: svn commit: r1704655 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/ test/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/mongo/
Posted by Chetan Mehrotra <ch...@gmail.com>.
Hi Marcel,
A short description of what was the fix would be very helpful for
future reference!
Chetan Mehrotra
On Tue, Sep 22, 2015 at 9:00 PM, <mr...@apache.org> wrote:
> Author: mreutegg
> Date: Tue Sep 22 15:30:08 2015
> New Revision: 1704655
>
> URL: http://svn.apache.org/viewvc?rev=1704655&view=rev
> Log:
> OAK-3433: Commit does not detect conflict when background read happens after rebase
>
> Add yet another test, enable existing test and implement fix
>
> 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/LastRevRecoveryAgent.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java
> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java
> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.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=1704655&r1=1704654&r2=1704655&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 Tue Sep 22 15:30:08 2015
> @@ -2084,9 +2084,9 @@ public final class DocumentNodeStore
> BackgroundWriteStats backgroundWrite() {
> return unsavedLastRevisions.persist(this, new UnsavedModifications.Snapshot() {
> @Override
> - public void acquiring() {
> + public void acquiring(Revision mostRecent) {
> if (store.create(JOURNAL,
> - singletonList(changes.asUpdateOp(getHeadRevision())))) {
> + singletonList(changes.asUpdateOp(mostRecent)))) {
> changes = JOURNAL.newDocument(getDocumentStore());
> }
> }
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java?rev=1704655&r1=1704654&r2=1704655&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java Tue Sep 22 15:30:08 2015
> @@ -235,7 +235,7 @@ public class LastRevRecoveryAgent {
> unsaved.persist(nodeStore, new UnsavedModifications.Snapshot() {
>
> @Override
> - public void acquiring() {
> + public void acquiring(Revision mostRecent) {
> if (lastRootRev == null) {
> // this should never happen - when unsaved has no changes
> // that is reflected in the 'map' to be empty - in that
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java?rev=1704655&r1=1704654&r2=1704655&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java Tue Sep 22 15:30:08 2015
> @@ -159,7 +159,7 @@ class UnsavedModifications {
> time = clock.getTime();
> Map<String, Revision> pending;
> try {
> - snapshot.acquiring();
> + snapshot.acquiring(getMostRecentRevision());
> pending = Maps.newTreeMap(PathComparator.INSTANCE);
> pending.putAll(map);
> } finally {
> @@ -234,14 +234,26 @@ class UnsavedModifications {
> return map.toString();
> }
>
> + private Revision getMostRecentRevision() {
> + // use revision of root document
> + Revision rev = map.get("/");
> + // otherwise find most recent
> + if (rev == null) {
> + for (Revision r : map.values()) {
> + rev = Utils.max(rev, r);
> + }
> + }
> + return rev;
> + }
> +
> public interface Snapshot {
>
> Snapshot IGNORE = new Snapshot() {
> @Override
> - public void acquiring() {
> + public void acquiring(Revision mostRecent) {
> }
> };
>
> - void acquiring();
> + void acquiring(Revision mostRecent);
> }
> }
>
> Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java?rev=1704655&r1=1704654&r2=1704655&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java Tue Sep 22 15:30:08 2015
> @@ -444,6 +444,31 @@ public class JournalTest extends Abstrac
> }
> }
>
> + // OAK-3433
> + @Test
> + public void journalEntryKey() throws Exception {
> + DocumentNodeStore ns1 = createMK(1, 0).getNodeStore();
> + DocumentNodeStore ns2 = createMK(2, 0).getNodeStore();
> +
> + NodeBuilder b1 = ns1.getRoot().builder();
> + b1.child("foo");
> + ns1.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
> + ns1.runBackgroundOperations();
> +
> + NodeBuilder b2 = ns2.getRoot().builder();
> + b2.child("bar");
> + ns2.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
> + Revision h2 = ns2.getHeadRevision();
> +
> + ns2.runBackgroundReadOperations();
> +
> + ns2.runBackgroundOperations();
> +
> + String id = JournalEntry.asId(h2);
> + assertTrue("Background update did not create a journal entry with id " + id,
> + ns1.getDocumentStore().find(Collection.JOURNAL, id) != null);
> + }
> +
> private DocumentMK createMK(int clusterId, int asyncDelay) {
> if (ds == null) {
> ds = new MemoryDocumentStore();
>
> Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java?rev=1704655&r1=1704654&r2=1704655&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java Tue Sep 22 15:30:08 2015
> @@ -32,10 +32,8 @@ 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 static org.junit.Assert.assertFalse;
> import static org.junit.Assert.assertTrue;
> import static org.junit.Assert.fail;
>
> @@ -61,7 +59,6 @@ public class ClusterConflictTest extends
> }
>
> // OAK-3433
> - @Ignore
> @Test
> public void mergeRetryWhileBackgroundRead() throws Exception {
> DocumentNodeStore ns1 = mk.getNodeStore();
> @@ -100,7 +97,7 @@ public class ClusterConflictTest extends
> // because /a/b/c is seen as changed in the future
> // without the fix for OAK-3433:
> // the second merge attempt succeeds because now the
> - // /a/b/c change is visible and happened before the commit
> + // /a/b/c change revision is visible and happened before the commit
> // revision but before the base revision
> b2 = ns2.getRoot().builder();
> b2.child("z").setProperty("q", "v");
> @@ -115,13 +112,18 @@ public class ClusterConflictTest extends
> runBackgroundRead(ns2);
>
> NodeBuilder builder = after.builder();
> - builder.child("a").child("b").child("c").child("bar");
> + if (builder.getChildNode("a").getChildNode("b").hasChildNode("c")) {
> + builder.child("a").child("b").child("c").child("bar");
> + } else {
> + throw new CommitFailedException(
> + CommitFailedException.OAK, 0,
> + "/a/b/c does not exist anymore");
> + }
> return builder.getNodeState();
> }
> }, CommitInfo.EMPTY);
> fail("Merge must fail with CommitFailedException");
> } catch (CommitFailedException e) {
> - e.printStackTrace();
> // expected
> }
> }
>
>
Re: svn commit: r1704655 - in /jackrabbit/oak/trunk/oak-core/src:
main/java/org/apache/jackrabbit/oak/plugins/document/ test/java/org/apache/jackrabbit/oak/plugins/document/
test/java/org/apache/jackrabbit/oak/plugins/document/mongo/
Posted by Chetan Mehrotra <ch...@gmail.com>.
Hi Marcel,
A short description of what was the fix would be very helpful for
future reference!
Chetan Mehrotra
On Tue, Sep 22, 2015 at 9:00 PM, <mr...@apache.org> wrote:
> Author: mreutegg
> Date: Tue Sep 22 15:30:08 2015
> New Revision: 1704655
>
> URL: http://svn.apache.org/viewvc?rev=1704655&view=rev
> Log:
> OAK-3433: Commit does not detect conflict when background read happens after rebase
>
> Add yet another test, enable existing test and implement fix
>
> 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/LastRevRecoveryAgent.java
> jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java
> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java
> jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.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=1704655&r1=1704654&r2=1704655&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 Tue Sep 22 15:30:08 2015
> @@ -2084,9 +2084,9 @@ public final class DocumentNodeStore
> BackgroundWriteStats backgroundWrite() {
> return unsavedLastRevisions.persist(this, new UnsavedModifications.Snapshot() {
> @Override
> - public void acquiring() {
> + public void acquiring(Revision mostRecent) {
> if (store.create(JOURNAL,
> - singletonList(changes.asUpdateOp(getHeadRevision())))) {
> + singletonList(changes.asUpdateOp(mostRecent)))) {
> changes = JOURNAL.newDocument(getDocumentStore());
> }
> }
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java?rev=1704655&r1=1704654&r2=1704655&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java Tue Sep 22 15:30:08 2015
> @@ -235,7 +235,7 @@ public class LastRevRecoveryAgent {
> unsaved.persist(nodeStore, new UnsavedModifications.Snapshot() {
>
> @Override
> - public void acquiring() {
> + public void acquiring(Revision mostRecent) {
> if (lastRootRev == null) {
> // this should never happen - when unsaved has no changes
> // that is reflected in the 'map' to be empty - in that
>
> Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java?rev=1704655&r1=1704654&r2=1704655&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/UnsavedModifications.java Tue Sep 22 15:30:08 2015
> @@ -159,7 +159,7 @@ class UnsavedModifications {
> time = clock.getTime();
> Map<String, Revision> pending;
> try {
> - snapshot.acquiring();
> + snapshot.acquiring(getMostRecentRevision());
> pending = Maps.newTreeMap(PathComparator.INSTANCE);
> pending.putAll(map);
> } finally {
> @@ -234,14 +234,26 @@ class UnsavedModifications {
> return map.toString();
> }
>
> + private Revision getMostRecentRevision() {
> + // use revision of root document
> + Revision rev = map.get("/");
> + // otherwise find most recent
> + if (rev == null) {
> + for (Revision r : map.values()) {
> + rev = Utils.max(rev, r);
> + }
> + }
> + return rev;
> + }
> +
> public interface Snapshot {
>
> Snapshot IGNORE = new Snapshot() {
> @Override
> - public void acquiring() {
> + public void acquiring(Revision mostRecent) {
> }
> };
>
> - void acquiring();
> + void acquiring(Revision mostRecent);
> }
> }
>
> Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java?rev=1704655&r1=1704654&r2=1704655&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalTest.java Tue Sep 22 15:30:08 2015
> @@ -444,6 +444,31 @@ public class JournalTest extends Abstrac
> }
> }
>
> + // OAK-3433
> + @Test
> + public void journalEntryKey() throws Exception {
> + DocumentNodeStore ns1 = createMK(1, 0).getNodeStore();
> + DocumentNodeStore ns2 = createMK(2, 0).getNodeStore();
> +
> + NodeBuilder b1 = ns1.getRoot().builder();
> + b1.child("foo");
> + ns1.merge(b1, EmptyHook.INSTANCE, CommitInfo.EMPTY);
> + ns1.runBackgroundOperations();
> +
> + NodeBuilder b2 = ns2.getRoot().builder();
> + b2.child("bar");
> + ns2.merge(b2, EmptyHook.INSTANCE, CommitInfo.EMPTY);
> + Revision h2 = ns2.getHeadRevision();
> +
> + ns2.runBackgroundReadOperations();
> +
> + ns2.runBackgroundOperations();
> +
> + String id = JournalEntry.asId(h2);
> + assertTrue("Background update did not create a journal entry with id " + id,
> + ns1.getDocumentStore().find(Collection.JOURNAL, id) != null);
> + }
> +
> private DocumentMK createMK(int clusterId, int asyncDelay) {
> if (ds == null) {
> ds = new MemoryDocumentStore();
>
> Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java
> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java?rev=1704655&r1=1704654&r2=1704655&view=diff
> ==============================================================================
> --- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java (original)
> +++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/mongo/ClusterConflictTest.java Tue Sep 22 15:30:08 2015
> @@ -32,10 +32,8 @@ 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 static org.junit.Assert.assertFalse;
> import static org.junit.Assert.assertTrue;
> import static org.junit.Assert.fail;
>
> @@ -61,7 +59,6 @@ public class ClusterConflictTest extends
> }
>
> // OAK-3433
> - @Ignore
> @Test
> public void mergeRetryWhileBackgroundRead() throws Exception {
> DocumentNodeStore ns1 = mk.getNodeStore();
> @@ -100,7 +97,7 @@ public class ClusterConflictTest extends
> // because /a/b/c is seen as changed in the future
> // without the fix for OAK-3433:
> // the second merge attempt succeeds because now the
> - // /a/b/c change is visible and happened before the commit
> + // /a/b/c change revision is visible and happened before the commit
> // revision but before the base revision
> b2 = ns2.getRoot().builder();
> b2.child("z").setProperty("q", "v");
> @@ -115,13 +112,18 @@ public class ClusterConflictTest extends
> runBackgroundRead(ns2);
>
> NodeBuilder builder = after.builder();
> - builder.child("a").child("b").child("c").child("bar");
> + if (builder.getChildNode("a").getChildNode("b").hasChildNode("c")) {
> + builder.child("a").child("b").child("c").child("bar");
> + } else {
> + throw new CommitFailedException(
> + CommitFailedException.OAK, 0,
> + "/a/b/c does not exist anymore");
> + }
> return builder.getNodeState();
> }
> }, CommitInfo.EMPTY);
> fail("Merge must fail with CommitFailedException");
> } catch (CommitFailedException e) {
> - e.printStackTrace();
> // expected
> }
> }
>
>