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 2022/10/28 13:12:50 UTC

[jackrabbit-oak] branch trunk updated: OAK-9967 : handled case to send lease update time in case of exception

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

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


The following commit(s) were added to refs/heads/trunk by this push:
     new dab01418d5 OAK-9967 : handled case to send lease update time in case of exception
     new 2de9104ee9 Merge pull request #739 from rishabhdaim/OAK-9967
dab01418d5 is described below

commit dab01418d50d5f3a7d9e40d3eb197317b9d640bf
Author: Rishabh Kumar <di...@adobe.com>
AuthorDate: Thu Oct 27 13:38:47 2022 +0530

    OAK-9967 : handled case to send lease update time in case of exception
---
 .../oak/plugins/document/DocumentNodeStore.java    | 16 ++++--
 .../plugins/document/DocumentNodeStoreTest.java    | 62 ++++++++++++++++++++++
 2 files changed, 74 insertions(+), 4 deletions(-)

diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
index 636a75c485..46f055d40a 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
@@ -2357,11 +2357,19 @@ public final class DocumentNodeStore
      */
     boolean renewClusterIdLease() {
         Stopwatch sw = Stopwatch.createStarted();
-        boolean renewed = clusterNodeInfo.renewLease();
-        if (renewed) {
-            nodeStoreStatsCollector.doneLeaseUpdate(sw.elapsed(MICROSECONDS));
+        boolean renewedOrException = true;
+        try {
+            renewedOrException = clusterNodeInfo.renewLease();
+        } finally {
+            // we need to collect the stats if,
+            // 1. Either lease had been renewed
+            // 2. or there is exception while renewing the lease i.e lease renewal failed/timed out
+            // In case lease is not renewed (can happen if it had not expired), we don't collect the stats
+            if (renewedOrException) {
+                nodeStoreStatsCollector.doneLeaseUpdate(sw.elapsed(MICROSECONDS));
+            }
         }
-        return renewed;
+        return renewedOrException;
     }
 
     /**
diff --git a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
index 7851ff1eef..0984961f53 100644
--- a/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
+++ b/oak-store-document/src/test/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreTest.java
@@ -20,6 +20,7 @@ import static com.google.common.collect.ImmutableList.of;
 import static java.util.Collections.emptyList;
 import static java.util.Collections.synchronizedList;
 import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.apache.commons.lang3.reflect.FieldUtils.writeField;
 import static org.apache.jackrabbit.oak.api.CommitFailedException.CONSTRAINT;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.CLUSTER_NODES;
 import static org.apache.jackrabbit.oak.plugins.document.Collection.JOURNAL;
@@ -49,7 +50,11 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import java.lang.management.ManagementFactory;
@@ -413,6 +418,63 @@ public class DocumentNodeStoreTest {
     }
     // END -- OAK-9909
 
+    // OAK-9967
+    @Test
+    public void renewClusterIdLeaseTrue() throws IllegalAccessException {
+        DocumentStore documentStore = new MemoryDocumentStore();
+
+        DocumentNodeStoreStatsCollector statsCollector = mock(DocumentNodeStoreStatsCollector.class);
+        doNothing().when(statsCollector).doneLeaseUpdate(anyLong());
+
+        ClusterNodeInfo clusterNodeInfo = mock(ClusterNodeInfo.class);
+        when(clusterNodeInfo.renewLease()).thenReturn(true);
+
+        DocumentNodeStore nodeStore = builderProvider.newBuilder().setNodeStoreStatsCollector(statsCollector)
+                .setDocumentStore(documentStore).getNodeStore();
+        writeField(nodeStore, "clusterNodeInfo", clusterNodeInfo, true);
+
+        assertTrue(nodeStore.renewClusterIdLease());
+        verify(statsCollector, times(1)).doneLeaseUpdate(anyLong());
+    }
+
+    @Test(expected = DocumentStoreException.class)
+    public void renewClusterIdLeaseException() throws IllegalAccessException {
+        DocumentStore documentStore = new MemoryDocumentStore();
+
+        DocumentNodeStoreStatsCollector statsCollector = mock(DocumentNodeStoreStatsCollector.class);
+        doNothing().when(statsCollector).doneLeaseUpdate(anyLong());
+
+        ClusterNodeInfo clusterNodeInfo = mock(ClusterNodeInfo.class);
+        when(clusterNodeInfo.renewLease()).thenThrow(DocumentStoreException.class);
+
+        DocumentNodeStore nodeStore = builderProvider.newBuilder().setNodeStoreStatsCollector(statsCollector)
+                .setDocumentStore(documentStore).getNodeStore();
+        writeField(nodeStore, "clusterNodeInfo", clusterNodeInfo, true);
+
+        nodeStore.renewClusterIdLease(); // should throw an exception
+        verify(statsCollector, times(1)).doneLeaseUpdate(anyLong());
+        fail("Shouldn't reach here");
+    }
+
+    @Test
+    public void renewClusterIdLeaseFalse() throws IllegalAccessException {
+        DocumentStore documentStore = new MemoryDocumentStore();
+
+        DocumentNodeStoreStatsCollector statsCollector = mock(DocumentNodeStoreStatsCollector.class);
+        doNothing().when(statsCollector).doneLeaseUpdate(anyLong());
+
+        ClusterNodeInfo clusterNodeInfo = mock(ClusterNodeInfo.class);
+        when(clusterNodeInfo.renewLease()).thenReturn(false);
+
+        DocumentNodeStore nodeStore = builderProvider.newBuilder().setNodeStoreStatsCollector(statsCollector)
+                .setDocumentStore(documentStore).getNodeStore();
+        writeField(nodeStore, "clusterNodeInfo", clusterNodeInfo, true);
+
+        assertFalse(nodeStore.renewClusterIdLease());
+        verify(statsCollector, times(0)).doneLeaseUpdate(anyLong());
+    }
+    // END -- OAK-9967
+
     // OAK-1662
     @Test
     public void getNewestRevision() throws Exception {