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 2021/07/08 16:04:15 UTC
[jackrabbit-oak] branch 1.22 updated: OAK-9401: Breaking recovery
lock issue
This is an automated email from the ASF dual-hosted git repository.
mreutegg pushed a commit to branch 1.22
in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
The following commit(s) were added to refs/heads/1.22 by this push:
new e5a3fef OAK-9401: Breaking recovery lock issue
new 48b8894 Merge pull request #319 from mreutegg/OAK-9401-1.22
e5a3fef is described below
commit e5a3fef442c3ea6967825d64e0f8edab00d61648
Author: Marcel Reutegger <mr...@apache.org>
AuthorDate: Tue May 11 07:34:55 2021 +0000
OAK-9401: Breaking recovery lock issue
Apply patch provided by José Andrés Cordero Benítez
git-svn-id: https://svn.apache.org/repos/asf/jackrabbit/oak/trunk@1889746 13f79535-47bb-0310-9956-ffa450edef68
---
.../oak/plugins/document/RecoveryLock.java | 13 +++---
.../oak/plugins/document/RecoveryLockTest.java | 51 +++++++++++++++++++++-
2 files changed, 58 insertions(+), 6 deletions(-)
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/RecoveryLock.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/RecoveryLock.java
index 9d5f1a6..4b92d4f 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/RecoveryLock.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/RecoveryLock.java
@@ -177,11 +177,14 @@ class RecoveryLock {
// cannot determine current lock owner
return false;
}
- long now = clock.getTime();
- long leaseEnd = recovering.getLeaseEndTime();
- if (recovering.isActive() && leaseEnd > now) {
- // still active, cannot break lock
- return false;
+ // OAK-9401: leaseEndTime can be null when the cluster is not active
+ if (recovering.isActive()) {
+ long now = clock.getTime();
+ long leaseEnd = recovering.getLeaseEndTime();
+ if (leaseEnd > now) {
+ // still active, cannot break lock
+ return false;
+ }
}
// try to break the lock
try {
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RecoveryLockTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RecoveryLockTest.java
index 2f167bc..255e887 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RecoveryLockTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/RecoveryLockTest.java
@@ -23,6 +23,7 @@ import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.Semaphore;
+import org.apache.jackrabbit.oak.commons.concurrent.ExecutorCloser;
import org.apache.jackrabbit.oak.plugins.document.memory.MemoryDocumentStore;
import org.apache.jackrabbit.oak.stats.Clock;
import org.junit.After;
@@ -40,6 +41,9 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
public class RecoveryLockTest {
@@ -53,7 +57,6 @@ public class RecoveryLockTest {
private RecoveryLock lock2 = new RecoveryLock(store, clock, 2);
private ClusterNodeInfo info1;
- private ClusterNodeInfo info2;
@Before
public void before() throws Exception {
@@ -66,6 +69,7 @@ public class RecoveryLockTest {
@After
public void after() {
ClusterNodeInfo.resetClockToDefault();
+ new ExecutorCloser(executor).close();
}
@Test
@@ -182,6 +186,51 @@ public class RecoveryLockTest {
assertFalse(lock1.acquireRecoveryLock(2));
}
+ @Test
+ // OAK-9401: Reproduce a bug that happens when the cluster is not active having a null leaseEndTime
+ public void breakRecoveryLockOfNotActiveCluster() throws Exception {
+ // Create a mocked version of the DocumentStore, to replace a method later during the test
+ DocumentStore store = spy(new MemoryDocumentStore());
+
+ info1 = ClusterNodeInfo.getInstance(store, RecoveryHandler.NOOP,
+ null, "node1", 1);
+ RecoveryLock recLock = new RecoveryLock(store, clock, 1);
+
+ // expire clusterId 1
+ clock.waitUntil(info1.getLeaseEndTime() + DEFAULT_LEASE_UPDATE_INTERVAL_MILLIS);
+
+ Semaphore recovering = new Semaphore(0);
+ Semaphore recovered = new Semaphore(0);
+ // simulate new startup and get info again
+ executor.submit(() ->
+ ClusterNodeInfo.getInstance(store, clusterId -> {
+ assertTrue(recLock.acquireRecoveryLock(1));
+ recovering.release();
+ recovered.acquireUninterruptibly();
+ recLock.releaseRecoveryLock(true);
+ return true;
+ }, null, "node1", 1));
+ // wait until submitted task is in recovery
+ recovering.acquireUninterruptibly();
+
+ // OAK-9401: Reproduce a bug that happens when the cluster is not active having a null leaseEndTime
+ // create a mocked copy of the ClusterNodeInfoDocument to be able to edit it, as the original is immutable
+ ClusterNodeInfoDocument realClusterInfo = spy(store.find(CLUSTER_NODES, String.valueOf(1)));
+ ClusterNodeInfoDocument mockClusterInfo = spy(CLUSTER_NODES.newDocument(store));
+ realClusterInfo.deepCopy(mockClusterInfo);
+
+ mockClusterInfo.put(ClusterNodeInfo.LEASE_END_KEY, null);
+ doReturn(false).when(mockClusterInfo).isActive();
+ when(store.find(CLUSTER_NODES, String.valueOf(1))).thenCallRealMethod().thenReturn(mockClusterInfo);
+
+ // clusterId 2 should be able to acquire (break) the recovery lock, instead of
+ // throwing "java.lang.NullPointerException: Lease End Time not set"
+ assertTrue(recLock.acquireRecoveryLock(2));
+
+ // let submitted task complete
+ recovered.release();
+ }
+
private ClusterNodeInfoDocument infoDocument(int clusterId) {
ClusterNodeInfoDocument doc = store.find(CLUSTER_NODES, String.valueOf(clusterId));
assertNotNull(doc);