(accumulo) branch main updated: avoids acquiring recovery lock when tablet has no wals (#4429)

[kt...@apache.org]

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

kturner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/main by this push:
     new b912506d43 avoids acquiring recovery lock when tablet has no wals (#4429)
b912506d43 is described below

commit b912506d43fbac9477aa3efb9ea81d39ce4d1a5c
Author: Keith Turner <kt...@apache.org>
AuthorDate: Fri Apr 5 13:01:07 2024 -0400

    avoids acquiring recovery lock when tablet has no wals (#4429)
    
    Tablet servers have a lock for recovery that has the purpose of
    preventing concurrent recovery of tablets using too much memory.
    This lock is acquired for tablets that have no walogs and will therefore
    do no recovery.  This commit changes the code to only obtain the lock
    when there are walogs.
    
    Noticed this while reviewing #4404 and while researching found
    [ACCUMULO-1543](https://issues.apache.org/jira/browse/ACCUMULO-1543)
    which is an existing issue describing this change.
---
 .../org/apache/accumulo/tserver/AssignmentHandler.java   |  6 +-----
 .../java/org/apache/accumulo/tserver/TabletServer.java   | 16 ++++++++--------
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
index 8b4f117a21..552d9f40a9 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/AssignmentHandler.java
@@ -156,9 +156,7 @@ class AssignmentHandler implements Runnable {
     Tablet tablet = null;
     boolean successful = false;
 
-    try {
-      server.acquireRecoveryMemory(extent);
-
+    try (var recoveryMemory = server.acquireRecoveryMemory(tabletMetadata)) {
       TabletResourceManager trm = server.resourceManager.createTabletResourceManager(extent,
           server.getTableConfiguration(extent));
       TabletData data = new TabletData(tabletMetadata);
@@ -205,8 +203,6 @@ class AssignmentHandler implements Runnable {
       TableId tableId = extent.tableId();
       ProblemReports.getInstance(server.getContext()).report(new ProblemReport(tableId, TABLET_LOAD,
           extent.getUUID().toString(), server.getClientAddressString(), e));
-    } finally {
-      server.releaseRecoveryMemory(extent);
     }
 
     if (successful) {
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
index b9c9a39359..678b1294c5 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java
@@ -91,6 +91,7 @@ import org.apache.accumulo.core.manager.thrift.TableInfo;
 import org.apache.accumulo.core.manager.thrift.TabletServerStatus;
 import org.apache.accumulo.core.metadata.AccumuloTable;
 import org.apache.accumulo.core.metadata.TServerInstance;
+import org.apache.accumulo.core.metadata.schema.TabletMetadata;
 import org.apache.accumulo.core.metadata.schema.TabletsMetadata;
 import org.apache.accumulo.core.metrics.MetricsUtil;
 import org.apache.accumulo.core.rpc.ThriftUtil;
@@ -516,15 +517,14 @@ public class TabletServer extends AbstractServer implements TabletHostingServer
     managerMessages.addLast(m);
   }
 
-  void acquireRecoveryMemory(KeyExtent extent) {
-    if (!extent.isMeta()) {
-      recoveryLock.lock();
-    }
-  }
+  private static final AutoCloseable NOOP_CLOSEABLE = () -> {};
 
-  void releaseRecoveryMemory(KeyExtent extent) {
-    if (!extent.isMeta()) {
-      recoveryLock.unlock();
+  AutoCloseable acquireRecoveryMemory(TabletMetadata tabletMetadata) {
+    if (tabletMetadata.getExtent().isMeta() || tabletMetadata.getLogs().isEmpty()) {
+      return NOOP_CLOSEABLE;
+    } else {
+      recoveryLock.lock();
+      return recoveryLock::unlock;
     }
   }