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:27 UTC
[jackrabbit-oak] branch 1.22 updated (7ee940da89 -> 8804afb6e1)
This is an automated email from the ASF dual-hosted git repository.
reschke pushed a change to branch 1.22
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
from 7ee940da89 OAK-9809: oak-run - update Jetty version for Servlet 3.1 API compliance
new aee3c2026f OAK-9908: Recovery may revert committed changes
new a71e0a6bbb OAK-9908: Recovery may revert committed changes
new 8804afb6e1 OAK-9908: Recovery may revert committed changes
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../oak/plugins/document/LastRevRecoveryAgent.java | 8 +
.../plugins/document/NodeDocumentSweeperIT.java | 224 +++++++++++++++++++++
2 files changed, 232 insertions(+)
create mode 100644 oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeperIT.java
[jackrabbit-oak] 01/03: OAK-9908: Recovery may revert committed changes
Posted by re...@apache.org.
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 aee3c2026f4d1b445b7203238c99c9548d7e96ef
Author: Marcel Reutegger <ma...@gmail.com>
AuthorDate: Mon Aug 22 14:54:52 2022 +0200
OAK-9908: Recovery may revert committed changes
Add ignored test
---
.../plugins/document/NodeDocumentSweeperIT.java | 140 +++++++++++++++++++++
1 file changed, 140 insertions(+)
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
new file mode 100644
index 0000000000..8048f19ea1
--- /dev/null
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/NodeDocumentSweeperIT.java
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+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;
+import org.apache.jackrabbit.oak.plugins.document.rdb.RDBDocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.rdb.RDBMissingLastRevSeeker;
+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.jetbrains.annotations.NotNull;
+import org.junit.Before;
+import org.junit.Ignore;
+import org.junit.Test;
+
+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.assertTrue;
+
+public class NodeDocumentSweeperIT extends AbstractTwoNodeTest {
+
+ private static final Path TEST_PATH = Path.fromString("/foo/bar/baz");
+
+ private FailingDocumentStore fds1;
+
+ private LastRevRecoveryAgent agent2;
+
+ public NodeDocumentSweeperIT(DocumentStoreFixture fixture) {
+ super(fixture);
+ }
+
+ @Override
+ protected DocumentStore customize(DocumentStore store) {
+ if (fds1 == null) {
+ fds1 = new FailingDocumentStore(store);
+ return fds1;
+ } else {
+ return store;
+ }
+ }
+
+ @Before
+ public void prepareAgent() {
+ // first setup seeker according to underlying document store implementation
+ MissingLastRevSeeker seeker;
+ if (store2 instanceof MongoDocumentStore) {
+ seeker = new MongoMissingLastRevSeeker((MongoDocumentStore) store2, clock);
+ } else if (store2 instanceof RDBDocumentStore) {
+ seeker = new RDBMissingLastRevSeeker((RDBDocumentStore) store2, clock) {
+ @Override
+ public @NotNull Iterable<NodeDocument> getCandidates(long startTime) {
+ List<NodeDocument> docs = new ArrayList<>();
+ super.getCandidates(startTime).forEach(docs::add);
+ docs.sort((o1, o2) -> NodeDocumentIdComparator.INSTANCE.compare(o1.getId(), o2.getId()));
+ return docs;
+ }
+ };
+ } else {
+ // use default implementation
+ seeker = new MissingLastRevSeeker(store2, clock);
+ }
+ // then customize seeker to return documents in a defined order
+ // return docs sorted by decreasing depth
+ MissingLastRevSeeker testSeeker = new MissingLastRevSeeker(store2, clock) {
+ @Override
+ public @NotNull Iterable<NodeDocument> getCandidates(long startTime) {
+ List<NodeDocument> docs = new ArrayList<>();
+ seeker.getCandidates(startTime).forEach(docs::add);
+ docs.sort((o1, o2) -> NodeDocumentIdComparator.INSTANCE.compare(o1.getId(), o2.getId()));
+ return docs;
+ }
+ };
+ agent2 = new LastRevRecoveryAgent(ds2.getDocumentStore(), ds2, testSeeker, v -> {});
+ }
+
+ @Ignore("OAK-9908")
+ @Test
+ public void recoveryWithSweep() 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() + TimeUnit.SECONDS.toMillis(10));
+ // add a child
+ builder = ds1.getRoot().builder();
+ getOrCreate(builder, TEST_PATH).child("test");
+ 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));
+ ds2.renewClusterIdLease();
+
+ assertTrue(agent2.isRecoveryNeeded());
+ agent2.recover(1);
+ ds2.runBackgroundOperations();
+ assertExists(ds2, new Path(TEST_PATH, "test"));
+ }
+
+ private void assertExists(NodeStore ns, Path path) {
+ NodeState state = ns.getRoot();
+ for (String name : path.elements()) {
+ state = state.getChildNode(name);
+ assertTrue(state.exists());
+ }
+ }
+
+ private NodeBuilder getOrCreate(NodeBuilder builder, Path path) {
+ for (String name : path.elements()) {
+ builder = builder.child(name);
+ }
+ return builder;
+ }
+
+}
[jackrabbit-oak] 02/03: OAK-9908: Recovery may revert committed changes
Posted by re...@apache.org.
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 a71e0a6bbb640819e52edded3f31f50cfd08ffb1
Author: Marcel Reutegger <ma...@gmail.com>
AuthorDate: Mon Aug 22 15:58:07 2022 +0200
OAK-9908: Recovery may revert committed changes
Ensure suspect documents are either up-to-date in the cache or invalidated
Enable test
---
.../jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java | 8 ++++++++
.../jackrabbit/oak/plugins/document/NodeDocumentSweeperIT.java | 2 --
2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
index e1f920f5e8..ada28007a9 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
@@ -19,6 +19,7 @@
package org.apache.jackrabbit.oak.plugins.document;
import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.transform;
import static com.google.common.collect.Lists.newArrayList;
import static com.google.common.collect.Maps.filterKeys;
import static java.util.Collections.singletonList;
@@ -44,6 +45,7 @@ import com.google.common.collect.Sets;
import org.apache.jackrabbit.oak.commons.TimeDurationFormatter;
import org.apache.jackrabbit.oak.plugins.document.bundlor.DocumentBundlor;
+import org.apache.jackrabbit.oak.plugins.document.cache.CacheInvalidationStats;
import org.apache.jackrabbit.oak.plugins.document.util.MapFactory;
import org.apache.jackrabbit.oak.plugins.document.util.Utils;
import org.apache.jackrabbit.oak.stats.Clock;
@@ -260,6 +262,12 @@ public class LastRevRecoveryAgent {
revisionContext.getClock(), clusterId,
revisionContext::getCommitValue);
final NodeDocumentSweeper sweeper = new NodeDocumentSweeper(context, true);
+ // make sure recovery does not run on stale cache
+ // invalidate all suspects (OAK-9908)
+ log.info("Starting cache invalidation before sweep...");
+ CacheInvalidationStats stats = store.invalidateCache(
+ transform(suspects, Document::getId));
+ log.info("Invalidation stats: {}", stats);
sweeper.sweep(suspects, new NodeDocumentSweepListener() {
@Override
public void sweepUpdate(Map<Path, UpdateOp> updates)
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 8048f19ea1..1b6ba62b09 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
@@ -29,7 +29,6 @@ import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
import org.jetbrains.annotations.NotNull;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import static org.apache.jackrabbit.oak.plugins.document.TestUtils.disposeQuietly;
@@ -92,7 +91,6 @@ public class NodeDocumentSweeperIT extends AbstractTwoNodeTest {
agent2 = new LastRevRecoveryAgent(ds2.getDocumentStore(), ds2, testSeeker, v -> {});
}
- @Ignore("OAK-9908")
@Test
public void recoveryWithSweep() throws Exception {
// create some test data
[jackrabbit-oak] 03/03: OAK-9908: Recovery may revert committed changes
Posted by re...@apache.org.
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);