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 re...@apache.org on 2023/01/04 10:18:30 UTC

[jackrabbit-oak] 03/03: OAK-9908: Recovery may revert committed changes

This is an automated email from the ASF dual-hosted git repository.

reschke pushed a commit to branch 1.22
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git

commit 8804afb6e176ee2ee595b3097ee5d582672d370f
Author: Marcel Reutegger <ma...@gmail.com>
AuthorDate: Thu Aug 25 11:35:10 2022 +0200

    OAK-9908: Recovery may revert committed changes
    
    More tests as suggested by Stefan and Jose
---
 .../plugins/document/NodeDocumentSweeperIT.java    | 102 +++++++++++++++++++--
 1 file changed, 94 insertions(+), 8 deletions(-)

diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeperIT.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeperIT.java
index 1b6ba62b09..42e397992e 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeperIT.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeperIT.java
@@ -18,7 +18,6 @@ package org.apache.jackrabbit.oak.plugins.document;
 
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.TimeUnit;
 
 import org.apache.jackrabbit.oak.plugins.document.mongo.MongoDocumentStore;
 import org.apache.jackrabbit.oak.plugins.document.mongo.MongoMissingLastRevSeeker;
@@ -31,13 +30,19 @@ import org.jetbrains.annotations.NotNull;
 import org.junit.Before;
 import org.junit.Test;
 
+import static java.util.concurrent.TimeUnit.MINUTES;
+import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.apache.jackrabbit.oak.plugins.document.TestUtils.disposeQuietly;
 import static org.apache.jackrabbit.oak.plugins.document.TestUtils.merge;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 public class NodeDocumentSweeperIT extends AbstractTwoNodeTest {
 
-    private static final Path TEST_PATH = Path.fromString("/foo/bar/baz");
+    private static final Path BASE_PATH = Path.fromString("/foo/bar/baz");
+
+    private static final Path TEST_PATH = new Path(BASE_PATH, "test");
 
     private FailingDocumentStore fds1;
 
@@ -92,7 +97,36 @@ public class NodeDocumentSweeperIT extends AbstractTwoNodeTest {
     }
 
     @Test
-    public void recoveryWithSweep() throws Exception {
+    public void recoveryWithSweepNodeAdded() throws Exception {
+        // create some test data
+        NodeBuilder builder = ds2.getRoot().builder();
+        getOrCreate(builder, BASE_PATH);
+        merge(ds2, builder);
+        ds2.runBackgroundOperations();
+        ds1.runBackgroundOperations();
+        // now these nodes are visible on ds1
+        assertExists(ds1, BASE_PATH);
+        // wait a bit
+        clock.waitUntil(clock.getTime() + SECONDS.toMillis(10));
+        // add a child
+        builder = ds1.getRoot().builder();
+        getOrCreate(builder, TEST_PATH);
+        merge(ds1, builder);
+        // simulate a crash
+        fds1.fail().after(0).eternally();
+        disposeQuietly(ds1);
+        // wait and run recovery for ds1
+        clock.waitUntil(clock.getTime() + MINUTES.toMillis(3));
+        ds2.renewClusterIdLease();
+
+        assertTrue(agent2.isRecoveryNeeded());
+        agent2.recover(1);
+        ds2.runBackgroundOperations();
+        assertExists(ds2, TEST_PATH);
+    }
+
+    @Test
+    public void recoveryWithSweepNodeDeleted() throws Exception {
         // create some test data
         NodeBuilder builder = ds2.getRoot().builder();
         getOrCreate(builder, TEST_PATH);
@@ -102,22 +136,66 @@ public class NodeDocumentSweeperIT extends AbstractTwoNodeTest {
         // now these nodes are visible on ds1
         assertExists(ds1, TEST_PATH);
         // wait a bit
-        clock.waitUntil(clock.getTime() + TimeUnit.SECONDS.toMillis(10));
-        // add a child
+        clock.waitUntil(clock.getTime() + SECONDS.toMillis(10));
+        // remove the child
+        builder = ds1.getRoot().builder();
+        getOrCreate(builder, TEST_PATH).remove();
+        // modify something on the remaining parent to move the commit root there
+        getOrCreate(builder, BASE_PATH).setProperty("p", "v");
+        merge(ds1, builder);
+        // simulate a crash
+        fds1.fail().after(0).eternally();
+        disposeQuietly(ds1);
+        // wait and run recovery for ds1
+        clock.waitUntil(clock.getTime() + MINUTES.toMillis(3));
+        ds2.renewClusterIdLease();
+
+        assertTrue(agent2.isRecoveryNeeded());
+        agent2.recover(1);
+        ds2.runBackgroundOperations();
+        assertNotExists(ds2, TEST_PATH);
+    }
+
+    @Test
+    public void recoveryWithSweepNodeChanged() throws Exception {
+        // create some test data
+        NodeBuilder builder = ds2.getRoot().builder();
+        getOrCreate(builder, TEST_PATH);
+        merge(ds2, builder);
+        ds2.runBackgroundOperations();
+        ds1.runBackgroundOperations();
+        // now these nodes are visible on ds1
+        assertExists(ds1, TEST_PATH);
+        // wait a bit
+        clock.waitUntil(clock.getTime() + SECONDS.toMillis(10));
+        // set a property
         builder = ds1.getRoot().builder();
-        getOrCreate(builder, TEST_PATH).child("test");
+        getOrCreate(builder, TEST_PATH).setProperty("p", "v");
+        // modify something on the remaining parent to move the commit root there
+        getOrCreate(builder, BASE_PATH).setProperty("p", "v");
         merge(ds1, builder);
         // simulate a crash
         fds1.fail().after(0).eternally();
         disposeQuietly(ds1);
         // wait and run recovery for ds1
-        clock.waitUntil(clock.getTime() + TimeUnit.MINUTES.toMillis(3));
+        clock.waitUntil(clock.getTime() + MINUTES.toMillis(3));
         ds2.renewClusterIdLease();
 
         assertTrue(agent2.isRecoveryNeeded());
         agent2.recover(1);
         ds2.runBackgroundOperations();
-        assertExists(ds2, new Path(TEST_PATH, "test"));
+        assertPropertyExists(ds2, new Path(TEST_PATH, "p"));
+    }
+
+    private void assertPropertyExists(NodeStore ns, Path path) {
+        NodeState state = ns.getRoot();
+        Path parent = path.getParent();
+        assertNotNull(parent);
+        for (String name : parent.elements()) {
+            state = state.getChildNode(name);
+            assertTrue(state.exists());
+        }
+        assertTrue(state.hasProperty(path.getName()));
     }
 
     private void assertExists(NodeStore ns, Path path) {
@@ -128,6 +206,14 @@ public class NodeDocumentSweeperIT extends AbstractTwoNodeTest {
         }
     }
 
+    private void assertNotExists(NodeStore ns, Path path) {
+        NodeState state = ns.getRoot();
+        for (String name : path.elements()) {
+            state = state.getChildNode(name);
+        }
+        assertFalse(state.exists());
+    }
+
     private NodeBuilder getOrCreate(NodeBuilder builder, Path path) {
         for (String name : path.elements()) {
             builder = builder.child(name);