Improve fetching scans and compactions in Monitor (apache#2294)

* Drop fetchScans and fetchCompactions threads and just let the page refresh get the data as needed. Created constant to prevent fetching more than once a minute. Created time constant for age off * Replace active scans table with datatables and add fetched column * Add fetched column to Active compactions table * Refactored fetch methods to catch thrift error and log. * Made entry point methods getCompactions and getScans synchronized to allow removal of synchronized code blocks.
milleruntime · Oct 1, 2021 · dcca5d1 · dcca5d1
1 parent 62bfc66
commit dcca5d1
Show file tree

Hide file tree

Showing 9 changed files with 108 additions and 110 deletions.
diff --git a/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java b/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java
@@ -474,28 +474,6 @@ public void run() {
       }
     }).start();
 
-    Threads.createThread("Scan scanner", () -> {
-      while (true) {
-        try {
-          fetchScans();
-        } catch (Exception e) {
-          log.warn("{}", e.getMessage(), e);
-        }
-        sleepUninterruptibly(5, TimeUnit.SECONDS);
-      }
-    }).start();
-
-    Threads.createThread("Compaction fetcher", () -> {
-      while (true) {
-        try {
-          fetchCompactions();
-        } catch (Exception e) {
-          log.warn("{}", e.getMessage(), e);
-        }
-        sleepUninterruptibly(5, TimeUnit.SECONDS);
-      }
-    }).start();
-
     monitorInitialized.set(true);
   }
 
@@ -561,6 +539,7 @@ public static class ScanStats {
         oldest = Math.max(oldest, scan.age);
       }
       this.oldestScan = oldest < 0 ? null : oldest;
+      // use clock time for date friendly display
       this.fetched = System.currentTimeMillis();
     }
   }
@@ -577,63 +556,79 @@ public static class CompactionStats {
         oldest = Math.max(oldest, a.age);
       }
       this.oldest = oldest < 0 ? null : oldest;
+      // use clock time for date friendly display
       this.fetched = System.currentTimeMillis();
     }
   }
 
   private final Map<HostAndPort,ScanStats> allScans = new HashMap<>();
   private final Map<HostAndPort,CompactionStats> allCompactions = new HashMap<>();
   private final RecentLogs recentLogs = new RecentLogs();
+  private long scansFetchedNanos = 0L;
+  private long compactsFetchedNanos = 0L;
+  private final long fetchTimeNanos = TimeUnit.MINUTES.toNanos(1);
+  private final long ageOffEntriesMillis = TimeUnit.MINUTES.toMillis(15);
 
-  public Map<HostAndPort,ScanStats> getScans() {
-    synchronized (allScans) {
-      return new HashMap<>(allScans);
+  /**
+   * Fetch the active scans but only if fetchTimeNanos has elapsed.
+   */
+  public synchronized Map<HostAndPort,ScanStats> getScans() {
+    if (System.nanoTime() - scansFetchedNanos > fetchTimeNanos) {
+      log.info("User initiated fetch of Active Scans");
+      fetchScans();
     }
+    return Map.copyOf(allScans);
   }
 
-  public Map<HostAndPort,CompactionStats> getCompactions() {
-    synchronized (allCompactions) {
-      return new HashMap<>(allCompactions);
+  /**
+   * Fetch the active compactions but only if fetchTimeNanos has elapsed.
+   */
+  public synchronized Map<HostAndPort,CompactionStats> getCompactions() {
+    if (System.nanoTime() - compactsFetchedNanos > fetchTimeNanos) {
+      log.info("User initiated fetch of Active Compactions");
+      fetchCompactions();
     }
+    return Map.copyOf(allCompactions);
   }
 
-  private void fetchScans() throws Exception {
+  private void fetchScans() {
     ServerContext context = getContext();
     for (String server : context.instanceOperations().getTabletServers()) {
       final HostAndPort parsedServer = HostAndPort.fromString(server);
-      Client tserver = ThriftUtil.getTServerClient(parsedServer, context);
+      Client tserver = null;
       try {
+        tserver = ThriftUtil.getTServerClient(parsedServer, context);
         List<ActiveScan> scans = tserver.getActiveScans(null, context.rpcCreds());
-        synchronized (allScans) {
-          allScans.put(parsedServer, new ScanStats(scans));
-        }
+        allScans.put(parsedServer, new ScanStats(scans));
+        scansFetchedNanos = System.nanoTime();
       } catch (Exception ex) {
-        log.debug("Failed to get active scans from {}", server, ex);
+        log.error("Failed to get active scans from {}", server, ex);
       } finally {
         ThriftUtil.returnClient(tserver);
       }
     }
     // Age off old scan information
     Iterator<Entry<HostAndPort,ScanStats>> entryIter = allScans.entrySet().iterator();
+    // clock time used for fetched for date friendly display
     long now = System.currentTimeMillis();
     while (entryIter.hasNext()) {
       Entry<HostAndPort,ScanStats> entry = entryIter.next();
-      if (now - entry.getValue().fetched > 5 * 60 * 1000) {
+      if (now - entry.getValue().fetched > ageOffEntriesMillis) {
         entryIter.remove();
       }
     }
   }
 
-  private void fetchCompactions() throws Exception {
+  private void fetchCompactions() {
     ServerContext context = getContext();
     for (String server : context.instanceOperations().getTabletServers()) {
       final HostAndPort parsedServer = HostAndPort.fromString(server);
-      Client tserver = ThriftUtil.getTServerClient(parsedServer, context);
+      Client tserver = null;
       try {
+        tserver = ThriftUtil.getTServerClient(parsedServer, context);
         var compacts = tserver.getActiveCompactions(null, context.rpcCreds());
-        synchronized (allCompactions) {
-          allCompactions.put(parsedServer, new CompactionStats(compacts));
-        }
+        allCompactions.put(parsedServer, new CompactionStats(compacts));
+        compactsFetchedNanos = System.nanoTime();
       } catch (Exception ex) {
         log.debug("Failed to get active compactions from {}", server, ex);
       } finally {
@@ -642,10 +637,11 @@ private void fetchCompactions() throws Exception {
     }
     // Age off old compaction information
     var entryIter = allCompactions.entrySet().iterator();
+    // clock time used for fetched for date friendly display
     long now = System.currentTimeMillis();
     while (entryIter.hasNext()) {
       var entry = entryIter.next();
-      if (now - entry.getValue().fetched > 5 * 60 * 1000) {
+      if (now - entry.getValue().fetched > ageOffEntriesMillis) {
         entryIter.remove();
       }
     }

diff --git a/...er/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/CompactionInfo.java b/...er/monitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/CompactionInfo.java
@@ -19,6 +19,7 @@
 package org.apache.accumulo.monitor.rest.compactions;
 
 import org.apache.accumulo.core.master.thrift.TabletServerStatus;
+import org.apache.accumulo.monitor.Monitor;
 
 /**
  * Generates a compaction info JSON object
@@ -30,24 +31,19 @@ public class CompactionInfo {
   // Variable names become JSON keys
   public String server;
 
+  public long fetched;
   public long count;
   public Long oldest;
 
   public CompactionInfo() {}
 
   /**
    * Stores new compaction information
-   *
-   * @param tserverInfo
-   *          status of the tserver
-   * @param count
-   *          number of compactions
-   * @param oldest
-   *          time of oldest compaction
    */
-  public CompactionInfo(TabletServerStatus tserverInfo, long count, Long oldest) {
+  public CompactionInfo(TabletServerStatus tserverInfo, Monitor.CompactionStats stats) {
     this.server = tserverInfo.getName();
-    this.count = count;
-    this.oldest = oldest;
+    this.fetched = stats.fetched;
+    this.count = stats.count;
+    this.oldest = stats.oldest;
   }
 }
diff --git a/...nitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/CompactionsResource.java b/...nitor/src/main/java/org/apache/accumulo/monitor/rest/compactions/CompactionsResource.java
@@ -61,7 +61,7 @@ public Compactions getActiveCompactions() {
     for (TabletServerStatus tserverInfo : mmi.getTServerInfo()) {
       var stats = entry.get(HostAndPort.fromString(tserverInfo.name));
       if (stats != null) {
-        compactions.addCompaction(new CompactionInfo(tserverInfo, stats.count, stats.oldest));
+        compactions.addCompaction(new CompactionInfo(tserverInfo, stats));
       }
     }
     return compactions;

diff --git a/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/scans/ScanInformation.java b/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/scans/ScanInformation.java
@@ -19,6 +19,7 @@
 package org.apache.accumulo.monitor.rest.scans;
 
 import org.apache.accumulo.core.master.thrift.TabletServerStatus;
+import org.apache.accumulo.monitor.Monitor;
 
 /**
  * Generates a scan JSON object
@@ -30,24 +31,19 @@ public class ScanInformation {
   // Variable names become JSON keys
   public String server;
 
+  public long fetched;
   public long scanCount;
   public Long oldestScan;
 
   public ScanInformation() {}
 
   /**
    * Stores new scan information
-   *
-   * @param tserverInfo
-   *          status of the tserver
-   * @param scanCount
-   *          number of scans
-   * @param oldestScan
-   *          time of oldest scan
    */
-  public ScanInformation(TabletServerStatus tserverInfo, long scanCount, Long oldestScan) {
+  public ScanInformation(TabletServerStatus tserverInfo, Monitor.ScanStats stats) {
     this.server = tserverInfo.getName();
-    this.scanCount = scanCount;
-    this.oldestScan = oldestScan;
+    this.fetched = stats.fetched;
+    this.scanCount = stats.scanCount;
+    this.oldestScan = stats.oldestScan;
   }
 }
diff --git a/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/scans/ScansResource.java b/server/monitor/src/main/java/org/apache/accumulo/monitor/rest/scans/ScansResource.java
@@ -50,7 +50,7 @@ public class ScansResource {
    * @return Scan JSON object
    */
   @GET
-  public Scans getActiveScans() {
+  public Scans getActiveScans() throws Exception {
     Scans scans = new Scans();
     ManagerMonitorInfo mmi = monitor.getMmi();
     if (mmi == null) {
@@ -63,7 +63,7 @@ public Scans getActiveScans() {
     for (TabletServerStatus tserverInfo : mmi.getTServerInfo()) {
       ScanStats stats = entry.get(HostAndPort.fromString(tserverInfo.name));
       if (stats != null) {
-        scans.addScan(new ScanInformation(tserverInfo, stats.scanCount, stats.oldestScan));
+        scans.addScan(new ScanInformation(tserverInfo, stats));
       }
     }
     return scans;

diff --git a/server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/compactions.js b/server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/compactions.js
@@ -36,7 +36,13 @@
                if(type === 'display') data = timeDuration(data);
                return data;
              }
-           }
+           },
+           { "targets": "date",
+               "render": function ( data, type, row ) {
+                 if(type === 'display') data = dateFormat(data);
+                 return data;
+               }
+             }
          ],
        "columns": [
          { "data": "server",
@@ -49,7 +55,8 @@
            }
          },
          { "data": "count" },
-         { "data": "oldest" }
+         { "data": "oldest" },
+         { "data": "fetched" },
        ]
      });
  });

diff --git a/server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/scans.js b/server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/scans.js
@@ -21,55 +21,56 @@
  * Creates scans initial table
  */
 $(document).ready(function() {
-  refreshScans();
+// Create a table for scans list
+     scansList = $('#scansList').DataTable({
+       "ajax": {
+         "url": '/rest/scans',
+         "dataSrc": "scans"
+       },
+       "stateSave": true,
+       "dom": 't<"align-left"l>p',
+       "columnDefs": [
+           { "targets": "duration",
+             "render": function ( data, type, row ) {
+               if(type === 'display') data = timeDuration(data);
+               return data;
+             }
+           },
+           { "targets": "date",
+               "render": function ( data, type, row ) {
+                 if(type === 'display') data = dateFormat(data);
+                 return data;
+               }
+             }
+         ],
+       "columns": [
+         { "data": "server",
+           "type": "html",
+           "render": function ( data, type, row, meta ) {
+             if(type === 'display') {
+                data = '<a href="/tservers?s=' + row.server + '">' + row.server + '</a>';
+             }
+             return data;
+           }
+         },
+         { "data": "scanCount" },
+         { "data": "oldestScan" },
+         { "data": "fetched" },
+       ]
+     });
 });
 
-/**
- * Makes the REST calls, generates the tables with the new information
- */
-function refreshScans() {
-  getScans().then(function() {
-    refreshScansTable();
-  });
-}
 
 /**
  * Used to redraw the page
  */
 function refresh() {
-  refreshScans();
+  refreshScansTable();
 }
 
 /**
  * Generates the scans table
  */
 function refreshScansTable() {
-  clearTableBody('scanStatus');
-
-  var data = sessionStorage.scans === undefined ?
-      [] : JSON.parse(sessionStorage.scans);
-
-  if (data.length === 0 || data.scans.length === 0) {
-    var items = createEmptyRow(3, 'Empty');
-
-    $('<tr/>', {
-      html: items
-    }).appendTo('#scanStatus tbody');
-  } else {
-    $.each(data.scans, function(key, val) {
-      var items = [];
-
-      items.push(createFirstCell(val.server,
-          '<a href="/tservers?s=' + val.server + '">' + val.server +
-          '</a>'));
-
-      items.push(createRightCell(val.scanCount, val.scanCount));
-
-      items.push(createRightCell(val.oldestScan, timeDuration(val.oldestScan)));
-
-      $('<tr/>', {
-        html: items.join('')
-      }).appendTo('#scanStatus tbody');
-    });
-  }
+   if(scansList) scansList.ajax.reload(null, false ); // user paging is not reset on reload
 }
diff --git a/server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/compactions.ftl b/server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/compactions.ftl
@@ -31,6 +31,7 @@
                 <th class="firstcell">Server&nbsp;</th>
                 <th title="Number of compactions presently running">#&nbsp;</th>
                 <th class="duration" title="The age of the oldest compaction on this server.">Oldest&nbsp;Age&nbsp;</th>
+                <th class="date" title="Last time data was fetched. Server fetches on page refresh, at most every minute.">Fetched</th>
               </tr>
             </thead>
             <tbody></tbody>

diff --git a/server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/scans.ftl b/server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/scans.ftl
@@ -25,13 +25,14 @@
       </div>
       <div class="row">
         <div class="col-xs-12">
-          <table id="scanStatus" class="table table-bordered table-striped table-condensed">
+          <table id="scansList" class="table table-bordered table-striped table-condensed">
             <thead>
-              <tr>
-                <th class="firstcell">Server&nbsp;</th>
-                <th title="Number of scans presently running">#&nbsp;</th>
-                <th title="The age of the oldest scan on this server.">Oldest&nbsp;Age&nbsp;</th>
-              </tr>
+             <tr>
+               <th class="firstcell">Server&nbsp;</th>
+               <th title="Number of scans presently running">#&nbsp;</th>
+               <th class="duration" title="The age of the oldest scan on this server.">Oldest&nbsp;Age&nbsp;</th>
+               <th class="date" title="Last time data was fetched. Server fetches on page refresh, at most every minute.">Fetched</th>
+             </tr>
             </thead>
             <tbody></tbody>
           </table>