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

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


The following commit(s) were added to refs/heads/2.1 by this push:
     new 801db455a5 Remove unneeded handling of exceptions not thrown (#3556)
801db455a5 is described below

commit 801db455a5e4542fd9b4296351dcc3bb779940d4
Author: Christopher Tubbs <[email protected]>
AuthorDate: Fri Jun 30 16:08:50 2023 -0400

    Remove unneeded handling of exceptions not thrown (#3556)
    
    * Use more specific AutoCloseable type for metrics closer, since it
      doesn't thrown exceptions, and there's no point in forcing the caller
      to handle them because the super-class has `throws Exception` that
      this one doesn't use
    * Narrow the catch block on compaction failures to handle only
      RuntimeExceptions, since it can't throw any checked exceptions
---
 .../compactions/InternalCompactionExecutor.java    | 11 +++----
 .../metrics/CompactionExecutorsMetrics.java        | 34 ++++++++++++----------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/compactions/InternalCompactionExecutor.java
 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/compactions/InternalCompactionExecutor.java
index 7f3d90af39..b7cdac5d4c 100644
--- 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/compactions/InternalCompactionExecutor.java
+++ 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/compactions/InternalCompactionExecutor.java
@@ -41,6 +41,7 @@ import org.apache.accumulo.core.util.ratelimit.RateLimiter;
 import org.apache.accumulo.core.util.threads.ThreadPools;
 import org.apache.accumulo.tserver.compactions.SubmittedJob.Status;
 import org.apache.accumulo.tserver.metrics.CompactionExecutorsMetrics;
+import 
org.apache.accumulo.tserver.metrics.CompactionExecutorsMetrics.CeMetrics;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -64,7 +65,7 @@ public class InternalCompactionExecutor implements 
CompactionExecutor {
   // not have constant time size operations.
   private final Set<InternalJob> queuedJob = Collections.synchronizedSet(new 
HashSet<>());
 
-  private final AutoCloseable metricCloser;
+  private final CeMetrics metricCloser;
 
   private final RateLimiter readLimiter;
   private final RateLimiter writeLimiter;
@@ -96,7 +97,7 @@ public class InternalCompactionExecutor implements 
CompactionExecutor {
           compactable.compact(csid, getJob(), readLimiter, writeLimiter, 
queuedTime);
           completionCallback.accept(compactable);
         }
-      } catch (Exception e) {
+      } catch (RuntimeException e) {
         log.warn("Compaction failed for {} on {}", compactable.getExtent(), 
getJob(), e);
         status.compareAndSet(Status.RUNNING, Status.FAILED);
       } finally {
@@ -208,11 +209,7 @@ public class InternalCompactionExecutor implements 
CompactionExecutor {
   public void stop() {
     threadPool.shutdownNow();
     log.debug("Stopped compaction executor {}", ceid);
-    try {
-      metricCloser.close();
-    } catch (Exception e) {
-      log.warn("Failed to close metrics {}", ceid, e);
-    }
+    metricCloser.close();
   }
 
   @Override
diff --git 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/CompactionExecutorsMetrics.java
 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/CompactionExecutorsMetrics.java
index 159528f1b2..dfb3f87e25 100644
--- 
a/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/CompactionExecutorsMetrics.java
+++ 
b/server/tserver/src/main/java/org/apache/accumulo/tserver/metrics/CompactionExecutorsMetrics.java
@@ -49,12 +49,21 @@ public class CompactionExecutorsMetrics implements 
MetricsProducer {
   private final Map<CompactionExecutorId,ExMetrics> exCeMetricsMap = new 
HashMap<>();
   private MeterRegistry registry = null;
 
-  private static class CeMetrics {
-    AtomicInteger queued;
-    AtomicInteger running;
-
-    IntSupplier runningSupplier;
-    IntSupplier queuedSupplier;
+  // public so it can be closed by outside callers
+  public static class CeMetrics implements AutoCloseable {
+    private AtomicInteger queued;
+    private AtomicInteger running;
+
+    private IntSupplier runningSupplier;
+    private IntSupplier queuedSupplier;
+
+    @Override
+    public void close() {
+      runningSupplier = () -> 0;
+      queuedSupplier = () -> 0;
+      running.set(0);
+      queued.set(0);
+    }
   }
 
   private static class ExMetrics {
@@ -75,8 +84,8 @@ public class CompactionExecutorsMetrics implements 
MetricsProducer {
         minimumRefreshDelay, minimumRefreshDelay, TimeUnit.MILLISECONDS));
   }
 
-  public synchronized AutoCloseable addExecutor(CompactionExecutorId ceid,
-      IntSupplier runningSupplier, IntSupplier queuedSupplier) {
+  public synchronized CeMetrics addExecutor(CompactionExecutorId ceid, 
IntSupplier runningSupplier,
+      IntSupplier queuedSupplier) {
 
     synchronized (ceMetricsMap) {
 
@@ -96,14 +105,7 @@ public class CompactionExecutorsMetrics implements 
MetricsProducer {
 
       ceMetricsList = List.copyOf(ceMetricsMap.values());
 
-      return () -> {
-
-        cem.runningSupplier = () -> 0;
-        cem.queuedSupplier = () -> 0;
-
-        cem.running.set(0);
-        cem.queued.set(0);
-      };
+      return cem;
     }
 
   }

Reply via email to