From fe22a33b23fe7a36b7febc8068deeaedcdd793d2 Mon Sep 17 00:00:00 2001
From: Dave Marion <dlmarion@apache.org>
Date: Tue, 7 Jan 2025 15:18:14 -0500
Subject: [PATCH] Better handling for runtime exception in Tablet.commit 
 (#5213)

Wrapped call to getTabletMemory().mutate() in try/finally
so that a RuntimeException does not short circuit the
code in the method that decrements the writes. Added
logging in TabletClientHandler to note which client
was causing the failure.

Closes #5208


Co-authored-by: Keith Turner <kturner@apache.org>
---
 .../accumulo/tserver/TabletClientHandler.java |  7 +++++
 .../accumulo/tserver/tablet/Tablet.java       | 27 +++++++++----------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
index 4086a8a963d..087c733e6f9 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletClientHandler.java
@@ -469,6 +469,7 @@ private void flush(UpdateSession us) {
         }
       } catch (Exception e) {
         TraceUtil.setException(span2, e, true);
+        log.error("Error logging mutations sent from {}", TServerUtils.clientAddress.get(), e);
         throw e;
       } finally {
         span2.end();
@@ -496,6 +497,10 @@ private void flush(UpdateSession us) {
         us.commitTimes.addStat(t2 - t1);
 
         updateAvgCommitTime(t2 - t1, sendables.size());
+      } catch (Exception e) {
+        TraceUtil.setException(span3, e, true);
+        log.error("Error committing mutations sent from {}", TServerUtils.clientAddress.get(), e);
+        throw e;
       } finally {
         span3.end();
       }
@@ -675,6 +680,7 @@ public void update(TInfo tinfo, TCredentials credentials, TKeyExtent tkeyExtent,
           session.commit(mutations);
         } catch (Exception e) {
           TraceUtil.setException(span3, e, true);
+          log.error("Error committing mutations sent from {}", TServerUtils.clientAddress.get(), e);
           throw e;
         } finally {
           span3.end();
@@ -831,6 +837,7 @@ private void writeConditionalMutations(Map<KeyExtent,List<ServerConditionalMutat
       updateAvgCommitTime(t2 - t1, sendables.size());
     } catch (Exception e) {
       TraceUtil.setException(span3, e, true);
+      log.error("Error committing mutations sent from {}", TServerUtils.clientAddress.get(), e);
       throw e;
     } finally {
       span3.end();
diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
index 70c200bb7fb..acacfb73229 100644
--- a/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
+++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/Tablet.java
@@ -877,22 +877,21 @@ public void commit(CommitSession commitSession, List<Mutation> mutations) {
       totalBytes += mutation.numBytes();
     }
 
-    getTabletMemory().mutate(commitSession, mutations, totalCount);
-
-    synchronized (this) {
-      if (isCloseComplete()) {
-        throw new IllegalStateException(
-            "Tablet " + extent + " closed with outstanding messages to the logger");
+    try {
+      getTabletMemory().mutate(commitSession, mutations, totalCount);
+      synchronized (this) {
+        getTabletMemory().updateMemoryUsageStats();
+        if (isCloseComplete()) {
+          throw new IllegalStateException(
+              "Tablet " + extent + " closed with outstanding messages to the logger");
+        }
+        numEntries += totalCount;
+        numEntriesInMemory += totalCount;
+        ingestCount += totalCount;
+        ingestBytes += totalBytes;
       }
-      // decrement here in case an exception is thrown below
+    } finally {
       decrementWritesInProgress(commitSession);
-
-      getTabletMemory().updateMemoryUsageStats();
-
-      numEntries += totalCount;
-      numEntriesInMemory += totalCount;
-      ingestCount += totalCount;
-      ingestBytes += totalBytes;
     }
   }