OAK-11720: Introduce tests for exclusive merge lock (#2351)

Author: Joscorbe

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 void noopChanges() throws Exception {
         }
         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());
+    }
 }