dlmarion commented on code in PR #5894:
URL: https://github.com/apache/accumulo/pull/5894#discussion_r2364517139


##########
server/monitor/src/main/java/org/apache/accumulo/monitor/next/Endpoints.java:
##########
@@ -151,6 +151,18 @@ public MetricResponse getManager() {
     return monitor.getInformationFetcher().getAllMetrics().asMap().get(s);
   }
 
+  @GET

Review Comment:
   The metrics are already returned as part of the `MetricResponse` json from 
the `getManager` call. Just wondering if we really need this, it seems 
duplicative.



##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/manager.js:
##########
@@ -92,146 +65,106 @@ function refreshManagerTables() {
  */
 $(function () {
 
-  // Generates the manager table
-  managerStatusTable = $('#managerStatus').DataTable({
-    "ajax": {
-      "url": contextPath + 'rest/manager',
-      "dataSrc": function (json) {
-        // the data needs to be in an array to work with DataTables
-        var arr = [json];
-        return arr;
-      }
-    },
-    "stateSave": true,
-    "searching": false,
-    "paging": false,
-    "info": false,
-    "columnDefs": [{
-        "targets": "big-num",
-        "render": function (data, type) {
-          if (type === 'display') {
-            data = bigNumberForQuantity(data);
+  getStatus().then(function () {
+    managerStatus = JSON.parse(sessionStorage.status).managerStatus;
+    if (managerStatus !== 'ERROR') {
+      // Generates the manager table
+      managerStatusTable = $('#managerStatusTable').DataTable({
+        "ajax": {
+          "url": contextPath + 'rest-v2/manager',
+          "dataSrc": function (json) {
+            // the data needs to be in an array to work with DataTables
+            var arr = [json];
+            return arr;
           }
-          return data;
-        }
-      },
-      {
-        "targets": "big-num-rounded",
-        "render": function (data, type) {
-          if (type === 'display') {
-            data = bigNumberForQuantity(Math.round(data));
+        },
+        "stateSave": true,
+        "searching": false,
+        "paging": false,
+        "info": false,
+        "columnDefs": [{
+            "targets": "timestamp",
+            "render": function (data, type) {
+              if (type === 'display') {
+                data = dateFormat(data);
+              }
+              return data;
+            }
+          },
+          {
+            "targets": "metrics",
+            "orderable": false,
+            "render": function () {
+              return "<a href=\"rest-v2/manager/metrics\">Metrics</a>";
+            }
           }
-          return data;
-        }
-      },
-      {
-        "targets": "duration",
-        "render": function (data, type) {
-          if (type === 'display') {
-            data = timeDuration(parseInt(data, 10));
+        ],
+        "columns": [{
+            "data": "host"
+          },
+          {
+            "data": "resourceGroup"
+          },
+          {
+            "data": "timestamp"
+          },
+          {
+            "data": "metrics"
+          }
+        ]
+      });
+    }
+
+    // Generates the recovery table
+    recoveryListTable = $('#recoveryList').DataTable({
+      "ajax": {
+        "url": contextPath + 'rest/tservers/recovery',
+        "dataSrc": function (data) {
+          data = data.recoveryList;
+          if (data.length === 0) {
+            console.info('Recovery list is empty, hiding recovery table');
+            $('#recoveryList_wrapper').hide();
+          } else {
+            $('#recoveryList_wrapper').show();
           }
           return data;
         }
-      }
-    ],
-    "columns": [{
-        "data": "manager"
-      },
-      {
-        "data": "onlineTabletServers"
       },
-      {
-        "data": "totalTabletServers"
-      },
-      {
-        "data": "lastGC",
-        "type": "html",
-        "render": function (data, type) {
-          if (type === 'display') {
-            if (data !== 'Waiting') {
-              data = dateFormat(parseInt(data, 10));
+      "columnDefs": [{
+          "targets": "duration",
+          "render": function (data, type) {
+            if (type === 'display') {
+              data = timeDuration(parseInt(data, 10));
             }
-            data = '<a href="gc">' + data + '</a>';
+            return data;
           }
-          return data;
-        }
-      },
-      {
-        "data": "tablets"
-      },
-      {
-        "data": "unassignedTablets"
-      },
-      {
-        "data": "numentries"
-      },
-      {
-        "data": "ingestrate"
-      },
-      {
-        "data": "entriesRead"
-      },
-      {
-        "data": "queryrate"
-      },
-      {
-        "data": "holdTime"
-      },
-      {
-        "data": "osload"
-      },
-    ]
-  });
-
-  // Generates the recovery table
-  recoveryListTable = $('#recoveryList').DataTable({

Review Comment:
   Do we need to add metrics for recovery?



##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/manager.js:
##########
@@ -23,56 +23,29 @@
 */
 "use strict";
 
-var managerStatusTable, recoveryListTable;
+var managerStatusTable, recoveryListTable, managerStatus;
 
 function refreshManagerBanners() {
-  getStatus().then(function () {
-    const managerStatus = JSON.parse(sessionStorage.status).managerStatus;
-
-    // If manager status is error
-    if (managerStatus === 'ERROR') {
-      // show the manager error banner and hide table
-      $('#managerRunningBanner').show();
-      $('#managerStatus_wrapper').hide();
-    } else {
-      // otherwise, hide the error banner and show manager table
-      $('#managerRunningBanner').hide();
-      $('#managerStatus_wrapper').show();
-    }
-  });
-
-  getManager().then(function () {
-    const managerData = JSON.parse(sessionStorage.manager);
-    const managerState = managerData.managerState;
-    const managerGoalState = managerData.managerGoalState;
-
-    const isStateGoalSame = managerState === managerGoalState;
-
-    // if the manager state is normal and the goal state is the same as the 
current state,
-    // or of the manager is not running, hide the state banner and return early
-    if ((managerState === 'NORMAL' && isStateGoalSame) || managerState === 
null) {
-      $('#managerStateBanner').hide();
-      return;
-    }
-
-    // update the manager state banner message and show it
-    let bannerMessage = 'Manager state: ' + managerState;
-    if (!isStateGoalSame) {
-      // only show the goal state if it differs from the manager's current 
state
-      bannerMessage += '. Manager goal state: ' + managerGoalState;
-    }
-    $('#manager-banner-message').text(bannerMessage);
-    $('#managerStateBanner').show();
-  });

Review Comment:
   Should we add Metrics for the Manager.ManagerState enum?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to