[GitHub] [accumulo] keith-turner commented on a change in pull request #2405: Created additional metrics for long assignments and stuck compactions

[GitBox <gi...@apache.org>]

keith-turner commented on a change in pull request #2405:
URL: https://github.com/apache/accumulo/pull/2405#discussion_r777708779



##########
File path: server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java
##########
@@ -107,8 +108,58 @@
 import com.beust.jcommander.Parameter;
 import com.google.common.base.Preconditions;
 
+import io.micrometer.core.instrument.Gauge;
+import io.micrometer.core.instrument.MeterRegistry;
+
 public class Compactor extends AbstractServer implements CompactorService.Iface {
 
+  private static class CompactionProgress implements MetricsProducer {

Review comment:
       Would it be possible to move CompactionWatcher from the tserver to server-base and reuse it in the compactor instead of introducing similar code in the compactor?

##########
File path: server/tserver/src/main/java/org/apache/accumulo/tserver/tablet/CompactionWatcher.java
##########
@@ -75,35 +80,42 @@ public void run() {
     HashMap<List<Long>,ObservedCompactionInfo> copy = new HashMap<>(observedCompactions);
     copy.keySet().removeAll(newKeys);
 
+    long unstuck = 0;
     for (ObservedCompactionInfo oci : copy.values()) {
+      unstuck++;
       if (oci.loggedWarning) {
         LoggerFactory.getLogger(CompactionWatcher.class).info("Compaction of {} is no longer stuck",
             oci.compactionInfo.getExtent());
       }
     }
+    totalStuck = Math.min(0, (totalStuck - unstuck));
 
     // remove any compaction that completed or made progress
     observedCompactions.keySet().retainAll(newKeys);
 
     long warnTime = config.getTimeInMillis(Property.TSERV_COMPACTION_WARN_TIME);
 
     // check for stuck compactions
+    long stuck = 0;
     for (ObservedCompactionInfo oci : observedCompactions.values()) {
-      if (time - oci.firstSeen > warnTime && !oci.loggedWarning) {
-        Thread compactionThread = oci.compactionInfo.getThread();
-        if (compactionThread != null) {
-          StackTraceElement[] trace = compactionThread.getStackTrace();
-          Exception e = new Exception(
-              "Possible stack trace of compaction stuck on " + oci.compactionInfo.getExtent());
-          e.setStackTrace(trace);
-          LoggerFactory.getLogger(CompactionWatcher.class)
-              .warn("Compaction of " + oci.compactionInfo.getExtent() + " to "
-                  + oci.compactionInfo.getOutputFile() + " has not made progress for at least "
-                  + (time - oci.firstSeen) + "ms", e);
-          oci.loggedWarning = true;
+      if (time - oci.firstSeen > warnTime) {
+        stuck++;
+        if (!oci.loggedWarning) {
+          Thread compactionThread = oci.compactionInfo.getThread();
+          if (compactionThread != null) {
+            StackTraceElement[] trace = compactionThread.getStackTrace();
+            Exception e = new Exception(
+                "Possible stack trace of compaction stuck on " + oci.compactionInfo.getExtent());
+            e.setStackTrace(trace);
+            LoggerFactory.getLogger(CompactionWatcher.class).warn("Compaction of "
+                + oci.compactionInfo.getExtent() + " to " + oci.compactionInfo.getOutputFile()
+                + " has not made progress for at least " + (time - oci.firstSeen) + "ms", e);
+            oci.loggedWarning = true;
+          }
         }
       }
     }
+    totalStuck += stuck;

Review comment:
       Could the following be done?  If so could remove the unstuck calculation and deduction earlier in the code.
   
   ```suggestion
       totalStuck = stuck;
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org