This is an automated email from the ASF dual-hosted git repository. joscorbe pushed a commit to branch OAK-11720-tests in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 15b0ca92d66849bef6a5159d0322fe5af73feb02 Author: Jose Cordero <[email protected]> AuthorDate: Tue Jun 24 02:33:18 2025 +0200 OAK-11720: Introduce tests for exclusive merge lock. --- .../document/DocumentNodeStoreBranchTest.java | 125 +++++++++++++++++++++ 1 file changed, 125 insertions(+) diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java index ca0f9e41cb..a678a5f3bb 100644 --- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java +++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreBranchTest.java @@ -16,17 +16,33 @@ */ package org.apache.jackrabbit.oak.plugins.document; +import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.json.JsopDiff; +import org.apache.jackrabbit.oak.spi.commit.CommitHook; +import org.apache.jackrabbit.oak.spi.commit.CommitInfo; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.junit.Rule; import org.junit.Test; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.locks.ReentrantReadWriteLock; + +import static org.apache.jackrabbit.oak.api.CommitFailedException.MERGE; import static org.apache.jackrabbit.oak.plugins.document.TestUtils.merge; import static org.apache.jackrabbit.oak.plugins.document.TestUtils.persistToBranch; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; public class DocumentNodeStoreBranchTest { @@ -120,4 +136,113 @@ public class DocumentNodeStoreBranchTest { } builder.getNodeState().compareAgainstBaseState(root, new JsopDiff()); } + + @Test // OAK-11720 + public void mergeRetriesWithExclusiveLock() throws Exception { + // avoidMergeLock = false -> should retry with exclusive lock + boolean AVOID_MERGE_LOCK = false; + DocumentMK.Builder mkBuilder = builderProvider.newBuilder(); + DocumentNodeStoreStatsCollector statsCollector = mock(DocumentNodeStoreStatsCollector.class); + mkBuilder.setNodeStoreStatsCollector(statsCollector); + DocumentNodeStore store = mkBuilder.getNodeStore(); + // Max back-off time for retries. + // It will retry with a waiting time of 50ms, 100ms, 200ms and 400ms (4 attempts in total). + store.setMaxBackOffMillis(500); + + // Best way to simulate a merge failure is to use a CommitHook that throws + // an exception on the first 4 attempts and succeeds on the 5th attempt. + AtomicInteger hookInvocations = new AtomicInteger(); + CommitHook hook = (before, after, info) -> { + int count = hookInvocations.incrementAndGet(); + if (count <= 4) { // Force a merge failure for the first 4 attempts + throw new CommitFailedException(MERGE, 1000 + count, "simulated failure"); + } else { + // on the 5th attempt will succeed + return after; + } + }; + + // create a test node to be merged + NodeBuilder builder = store.getRoot().builder(); + builder.child("testNode").setProperty("testProperty", "testValue"); + + DocumentNodeStoreBranch branch = new DocumentNodeStoreBranch(store, store.getRoot(), + new ReentrantReadWriteLock(), AVOID_MERGE_LOCK // avoidMergeLock set to false - must retry with exclusive lock + ); + branch.setRoot(builder.getNodeState()); + + // Initially the test node must not exist + assertFalse(store.getRoot().hasChildNode("testNode")); + NodeState result = branch.merge(hook, CommitInfo.EMPTY); + assertNotNull(result); + // Check the CommitHook was invoked 5 times (4 failures + 1 success) + assertEquals("CommitHook must be invoked 5 times", 5, hookInvocations.get()); + // The test node must now exist after the successful merge + assertTrue("Node must be present after successful merge", store.getRoot().hasChildNode("testNode")); + assertTrue("Property must be set after successful merge", store.getRoot().getChildNode("testNode").hasProperty("testProperty")); + + // Verify that first 4 attempts failed with exclusive == false + verify(statsCollector).failedMerge(anyInt(), anyLong(), anyLong(), eq(false)); + // Verify that the last attempt succeeded with exclusive == true + verify(statsCollector).doneMerge(anyInt(), anyInt(), anyLong(), anyLong(), eq(true)); + // Verify that no attempt without exclusive lock failed + verify(statsCollector, never()).doneMerge(anyInt(), anyInt(), anyLong(), anyLong(), eq(false)); + } + + @Test // OAK-11720 + public void mergeRetriesWithoutExclusiveLock() { + // avoidMergeLock = true -> should not retry with exclusive lock and fail immediately + boolean AVOID_MERGE_LOCK = true; + DocumentMK.Builder mkBuilder = builderProvider.newBuilder(); + DocumentNodeStoreStatsCollector statsCollector = mock(DocumentNodeStoreStatsCollector.class); + mkBuilder.setNodeStoreStatsCollector(statsCollector); + DocumentNodeStore store = mkBuilder.getNodeStore(); + // Max back-off time for retries. + // It will retry with a waiting time of 50ms, 100ms, 200ms and 400ms (4 attempts in total) + store.setMaxBackOffMillis(500); + + AtomicInteger hookInvocations = new AtomicInteger(); + CommitHook hook = (before, after, info) -> { + int count = hookInvocations.incrementAndGet(); + if (count <= 4) { // Force a merge failure for the first 4 attempts + throw new CommitFailedException(MERGE, 1000 + count, "simulated failure"); + } else { + // on the 5th attempt will succeed + return after; + } + }; + + // create a test node to be merged + NodeBuilder builder = store.getRoot().builder(); + builder.child("testNode").setProperty("testProperty", "testValue"); + + DocumentNodeStoreBranch branch = new DocumentNodeStoreBranch(store, store.getRoot(), + new ReentrantReadWriteLock(), AVOID_MERGE_LOCK // avoidMergeLock set to true - must fail after retries without exclusive lock + ); + branch.setRoot(builder.getNodeState()); + + // Initially the test node must not exist + assertFalse(store.getRoot().hasChildNode("testNode")); + try { + branch.merge(hook, CommitInfo.EMPTY); + fail("Merge must fail with CommitFailedException after all the attempts without exclusive lock"); + } catch (CommitFailedException e) { + assertEquals(MERGE, e.getType()); + assertEquals(1004, e.getCode()); + } + + // Check the CommitHook was invoked 4 times (4 failures) + assertEquals("CommitHook must be invoked 4 times", 4, hookInvocations.get()); + // The test node must NOT exist after the successful merge + assertFalse("Node must be present after successful merge", store.getRoot().hasChildNode("testNode")); + + // Verify that first 4 attempts failed with exclusive == false + verify(statsCollector).failedMerge(anyInt(), anyLong(), anyLong(), eq(false)); + // Verify that no attempt failed with exclusive == true + verify(statsCollector, never()).failedMerge(anyInt(), anyLong(), anyLong(), eq(true)); + // Verify that no merge attempt happened with exclusive == true + verify(statsCollector, never()).failedMerge(anyInt(), anyLong(), anyLong(), eq(true)); + // Verify that the merge never succeeded (with any value of exclusive lock) + verify(statsCollector, never()).doneMerge(anyInt(), anyInt(), anyLong(), anyLong(), anyBoolean()); + } }
