Skip to content

removes CoordinatorSummaryLogger#6237

Merged
keith-turner merged 3 commits intoapache:mainfrom
keith-turner:remove-coordinator-summary-logger
Mar 25, 2026
Merged

removes CoordinatorSummaryLogger#6237
keith-turner merged 3 commits intoapache:mainfrom
keith-turner:remove-coordinator-summary-logger

Conversation

@keith-turner
Copy link
Contributor

This logger was useful in 2.1. However in 4.0 its redundant with functionality in the monitor and is just extra code to maintain. Removing it also removes a usage of the running cache which may be helpful for #6217

This logger was useful in 2.1.  However in 4.0 its redundant with
functionality in the monitor and is just extra code to maintain.
Removing it also removes a usage of the running cache which may be
helpful for apache#6217
@keith-turner keith-turner added this to the 4.0.0 milestone Mar 23, 2026
@dlmarion
Copy link
Contributor

The CoordinatorSummaryLogger prints number of queued and running compactions per queue and number of running compactions per table. The related queue metrics are:

  • accumulo.compaction.queue.jobs.queued for number of queued compactions, has a queue.id tag for each queue, emitted by the coordinator
  • accumulo.compaction.majc.in_progress gives number of running compactions, but I don't think it has a queue.id tag, emitted by the compactor
  • I don't think there is a metric or tag for table, so there is no way to see number of running compactions per table in the metrics.

@keith-turner
Copy link
Contributor Author

keith-turner commented Mar 24, 2026

I don't think there is a metric or tag for table, so there is no way to see number of running compactions per table in the metrics.

The monitor also scans the metadata table, could it compute those counts from its metadata table scan?

@keith-turner
Copy link
Contributor Author

accumulo.compaction.majc.in_progress gives number of running compactions, but I don't think it has a queue.id tag, emitted by the compactor

It does seem to set the queue.id here

@keith-turner
Copy link
Contributor Author

@dlmarion do you know if the monitor currently displays this information or if there is an issue open for doing so? Wondering if this PR needs a follow on issue before it can be closed.

@keith-turner
Copy link
Contributor Author

Instead of scanning the metadata table, another way the monitor could potentially compute per table info is if the data it gets from each compactor about its running compaction contains the table id.

@keith-turner
Copy link
Contributor Author

It may be easy to compute the per table counts in the monitor. The following is a quick try.

diff --git a/server/monitor/src/main/java/org/apache/accumulo/monitor/next/SystemInformation.java b/server/monitor/src/main/java/org/apache/accumulo/monitor/next/SystemInformation.java
index 27b22d1700..7594e0d1f9 100644
--- a/server/monitor/src/main/java/org/apache/accumulo/monitor/next/SystemInformation.java
+++ b/server/monitor/src/main/java/org/apache/accumulo/monitor/next/SystemInformation.java
@@ -400,6 +400,9 @@ public class SystemInformation {
   protected final Map<String,TimeOrderedRunningCompactionSet> longRunningCompactionsByRg =
       new ConcurrentHashMap<>();
 
+  protected final Map<TableId,AtomicLong> runningCompactionsPerTable = new ConcurrentHashMap<>();
+  protected final Map<String,AtomicLong> runningCompactionsPerGroup = new ConcurrentHashMap<>();
+
   // Table Information
   private final Map<TableId,TableSummary> tables = new ConcurrentHashMap<>();
   private final Map<TableId,List<TabletInformation>> tablets = new ConcurrentHashMap<>();
@@ -538,6 +541,12 @@ public class SystemInformation {
   }
 
   public void processExternalCompaction(TExternalCompaction tec) {
+
+    var tableId = KeyExtent.fromThrift(tec.getJob().extent).tableId();
+    runningCompactionsPerTable.computeIfAbsent(tableId, t -> new AtomicLong()).incrementAndGet();
+    runningCompactionsPerGroup.computeIfAbsent(tec.getGroupName(), t -> new AtomicLong())
+        .incrementAndGet();
+
     this.longRunningCompactionsByRg.computeIfAbsent(tec.getGroupName(),
         k -> new TimeOrderedRunningCompactionSet(rgLongRunningCompactionSize)).add(tec);
   }

@dlmarion
Copy link
Contributor

@keith-turner - I like your approach above to capture the information in the SystemInformation class as it's processing the information for the current compactions. I would suggest making that change in this PR and then creating a follow-on issue (assign to me if you want) to integrate that information into an endpoint for the Monitor display.

@keith-turner
Copy link
Contributor Author

@dlmarion I added the changes to system information and opened #6247

Comment on lines +402 to +404
protected final Map<TableId,LongAdder> runningCompactionsPerTable = new ConcurrentHashMap<>();
protected final Map<String,LongAdder> runningCompactionsPerGroup = new ConcurrentHashMap<>();

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should include these in the clear() method.

@keith-turner keith-turner merged commit b26d59d into apache:main Mar 25, 2026
9 checks passed
@keith-turner keith-turner deleted the remove-coordinator-summary-logger branch March 25, 2026 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants