You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/05/09 20:56:13 UTC

[GitHub] [accumulo] DomGarguilo opened a new pull request, #2685: Convert deadTServers and badTServers tables to DataTables

DomGarguilo opened a new pull request, #2685:
URL: https://github.com/apache/accumulo/pull/2685

   This PR:
   * Converts `deadtservers` and `badtservers` to DataTables
   * Add a title and caption to the online tservers table
   * Conform to eslint formatting
   
   Here is a screenshot of the current `/tservers` page:
   ![image](https://user-images.githubusercontent.com/47725857/167496984-c4136625-00eb-4c82-8b1f-ba1dae8a0002.png)
   


-- 
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


[GitHub] [accumulo] DomGarguilo commented on pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#issuecomment-1122744887

   > when the caption is displayed, it will flash out then back in on each autorefresh.
   
   Fixed this in [78e613b](https://github.com/apache/accumulo/pull/2685/commits/78e613bf4dd4048e001c85230e25ad7fc375a61e).


-- 
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


[GitHub] [accumulo] milleruntime commented on a diff in pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#discussion_r869088939


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/templates/tservers.ftl:
##########
@@ -25,9 +25,9 @@
       </div>
       <div class="row">
         <div class="col-xs-12">
-          <table id="badtservers" class="table table-bordered table-striped table-condensed" style="display: none;">

Review Comment:
   So how are you hiding the table now? I didn't see a hide anywhere. I think with the other datatables I had to hide the div around the table.



-- 
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


[GitHub] [accumulo] milleruntime commented on a diff in pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#discussion_r869087332


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/tservers.js:
##########
@@ -183,29 +112,155 @@ function refreshDeadTServersTable() {
  * @param {string} server Dead TServer to clear
  */
 function clearDeadTServers(server) {
-  clearDeadServers(server);
-  refreshTServers();
-  refreshNavBar();
+    clearDeadServers(server);
+    refreshTServers();
+    refreshNavBar();
 }
 
 /**
- * Generates the tserver table
+ * Creates initial tables
  */
-function refreshTServersTable() {
-  if (tserversTable) tserversTable.ajax.reload(null, false); // user paging is not reset on reload
-}
+$(document).ready(function () {
 
-/**
- * Refreshes the list of recovering tservers used to highlight rows
- */
-function refreshRecoveryList() {
-  $('#recovery-caption').hide(); // Hide the caption about highlighted rows on each refresh
-  getRecoveryList().then(function () {
-    recoveryList = []
-    var data = sessionStorage.recoveryList === undefined ?
-      [] : JSON.parse(sessionStorage.recoveryList);
-    data.recoveryList.forEach(entry => {
-      recoveryList.push(entry.server);
+    refreshRecoveryList();
+
+    // Create a table for tserver list
+    tserversTable = $('#tservers').DataTable({
+        "ajax": {
+            "url": '/rest/tservers',
+            "dataSrc": "servers"
+        },
+        "stateSave": true,
+        "columnDefs": [
+            {
+                "targets": "big-num",
+                "render": function (data, type) {
+                    if (type === 'display') {
+                        data = bigNumberForQuantity(data);
+                    }
+                    return data;
+                }
+            },
+            {
+                "targets": "duration",
+                "render": function (data, type) {
+                    if (type === 'display') {
+                        data = timeDuration(data);
+                    }
+                    return data;
+                }
+            },
+            {
+                "targets": "percent",
+                "render": function (data, type) {
+                    if (type === 'display') {
+                        data = Math.round(data * 100) + '%';
+                    }
+                    return data;
+                }
+            }
+        ],
+        "columns": [
+            {
+                "data": "hostname",
+                "type": "html",
+                "render": function (data, type, row) {
+                    if (type === 'display') {
+                        data = '<a href="/tservers?s=' + row.id + '">' + row.hostname + '</a>';
+                    }
+                    return data;
+                }
+            },
+            { "data": "tablets" },
+            { "data": "lastContact" },
+            { "data": "responseTime" },
+            { "data": "entries" },
+            { "data": "ingest" },
+            { "data": "query" },
+            { "data": "holdtime" },
+            { "data": "scansCombo" },
+            { "data": "minorCombo" },
+            { "data": "majorCombo" },
+            { "data": "indexCacheHitRate" },
+            { "data": "dataCacheHitRate" },
+            { "data": "osload" }
+        ],
+        "rowCallback": function (row, data, index) {
+            // reset background of each row
+            $(row).css('background-color', '');
+
+            // return if the current row's tserver is not recovering
+            if (!recoveryList.includes(data.hostname)) {
+                return;
+            }
+
+            // only show the caption if we know there are rows in the tservers table
+            $('#recovery-caption').show();

Review Comment:
   You could put this in an else above.



-- 
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


[GitHub] [accumulo] DomGarguilo commented on a diff in pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#discussion_r870367542


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/tservers.js:
##########
@@ -16,165 +16,115 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+/* JSLint global definitions */
+/*global
+    $, document, sessionStorage, getTServers, clearDeadServers, refreshNavBar,
+    getRecoveryList, bigNumberForQuantity, timeDuration, dateFormat
+*/
 "use strict";
 
-var tserversTable;
+var tserversTable, deadTServersTable, badTServersTable;
 var recoveryList = [];
 
 /**
- * Creates tservers initial table
+ * Checks if the given server is in the global recoveryList variable
+ * 
+ * @param {JSON} server json server object
+ * @returns true if the server is in the recoveryList, else false
  */
-$(document).ready(function() {
-
-  refreshRecoveryList();
-    
-    // Create a table for tserver list
-    tserversTable = $('#tservers').DataTable({
-      "ajax": {
-        "url": '/rest/tservers',
-        "dataSrc": "servers"
-      },
-      "stateSave": true,
-      "columnDefs": [
-          { "targets": "big-num",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = bigNumberForQuantity(data);
-              return data;
-            }
-          },
-          { "targets": "duration",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = timeDuration(data);
-              return data;
-            }
-          },
-          { "targets": "percent",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = Math.round(data * 100) + '%';
-              return data;
-            }
-          }
-        ],
-      "columns": [
-        { "data": "hostname",
-          "type": "html",
-          "render": function ( data, type, row, meta ) {
-            if(type === 'display') data = '<a href="/tservers?s=' + row.id + '">' + row.hostname + '</a>';
-            return data;
-          }
-        },
-        { "data": "tablets" },
-        { "data": "lastContact" },
-        { "data": "responseTime" },
-        { "data": "entries" },
-        { "data": "ingest" },
-        { "data": "query" },
-        { "data": "holdtime" },
-        { "data": "scansCombo" },
-        { "data": "minorCombo" },
-        { "data": "majorCombo" },
-        { "data": "indexCacheHitRate" },
-        { "data": "dataCacheHitRate" },
-        { "data": "osload" }
-      ],
-      "rowCallback": function (row, data, index) {
-        // reset background of each row
-        $(row).css('background-color', '');
-
-        // return if the current row's tserver is not recovering
-        if (!recoveryList.includes(data.hostname))
-          return;
-
-        // only show the caption if we know there are rows in the tservers table
-        $('#recovery-caption').show();
-
-        // highlight current row
-        console.log('Highlighting row index:' + index + ' tserver:' + data.hostname);
-        $(row).css('background-color', 'gold');
-      }
-    });
-    refreshTServers();
-});
+function serverIsInRecoveryList(server) {
+    return recoveryList.includes(server.hostname);
+}
 
 /**
- * Makes the REST calls, generates the tables with the new information
+ * Refreshes the list of recovering tservers and shows/hides the recovery caption
  */
-function refreshTServers() {
-  getTServers().then(function() {
-    refreshBadTServersTable();
-    refreshDeadTServersTable();
-    refreshRecoveryList();
-    refreshTServersTable();
-  });
+function refreshRecoveryList() {
+    getRecoveryList().then(function () {
+        // get list of recovering servers
+        recoveryList = [];
+        var data = sessionStorage.recoveryList === undefined ?
+                    [] : JSON.parse(sessionStorage.recoveryList);
+        data.recoveryList.forEach(function (entry) {
+            recoveryList.push(entry.server);
+        });
+
+        // get list of online tservers
+        data = sessionStorage.tservers === undefined ?
+                    [] : JSON.parse(sessionStorage.tservers).servers;
+
+        // show the recovery caption if its in the list of recovering servers
+        if (data.some(serverIsInRecoveryList)) {

Review Comment:
   Thanks. I'll go ahead and mess with the variable names then merge this in.



-- 
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


[GitHub] [accumulo] DomGarguilo commented on pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#issuecomment-1122513558

   > Looks good. Just wondering if you will get flashing of the new tables with moving the hiding from the html to the JS.
   
   No, it looks like the hidden tables don't flash on screen with autorefresh on. I made sure the highlighted rows for recovering tservers still works (which it does) but noticed that when the caption is displayed, it will flash out then back in on each autorefresh. Not sure if that is something worth looking into further or not.


-- 
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


[GitHub] [accumulo] milleruntime commented on a diff in pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#discussion_r869097338


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/tservers.js:
##########
@@ -16,165 +16,94 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+/* JSLint global definitions */
+/*global
+    $, document, sessionStorage, getTServers, clearDeadServers, refreshNavBar,
+    getRecoveryList, bigNumberForQuantity, timeDuration, dateFormat
+*/
 "use strict";
 
-var tserversTable;
+var tserversTable, deadTServersTable, badTServersTable;
 var recoveryList = [];
 
 /**
- * Creates tservers initial table
+ * Refreshes the list of recovering tservers used to highlight rows
  */
-$(document).ready(function() {
-
-  refreshRecoveryList();
-    
-    // Create a table for tserver list
-    tserversTable = $('#tservers').DataTable({
-      "ajax": {
-        "url": '/rest/tservers',
-        "dataSrc": "servers"
-      },
-      "stateSave": true,
-      "columnDefs": [
-          { "targets": "big-num",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = bigNumberForQuantity(data);
-              return data;
-            }
-          },
-          { "targets": "duration",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = timeDuration(data);
-              return data;
-            }
-          },
-          { "targets": "percent",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = Math.round(data * 100) + '%';
-              return data;
-            }
-          }
-        ],
-      "columns": [
-        { "data": "hostname",
-          "type": "html",
-          "render": function ( data, type, row, meta ) {
-            if(type === 'display') data = '<a href="/tservers?s=' + row.id + '">' + row.hostname + '</a>';
-            return data;
-          }
-        },
-        { "data": "tablets" },
-        { "data": "lastContact" },
-        { "data": "responseTime" },
-        { "data": "entries" },
-        { "data": "ingest" },
-        { "data": "query" },
-        { "data": "holdtime" },
-        { "data": "scansCombo" },
-        { "data": "minorCombo" },
-        { "data": "majorCombo" },
-        { "data": "indexCacheHitRate" },
-        { "data": "dataCacheHitRate" },
-        { "data": "osload" }
-      ],
-      "rowCallback": function (row, data, index) {
-        // reset background of each row
-        $(row).css('background-color', '');
-
-        // return if the current row's tserver is not recovering
-        if (!recoveryList.includes(data.hostname))
-          return;
-
-        // only show the caption if we know there are rows in the tservers table
-        $('#recovery-caption').show();
-
-        // highlight current row
-        console.log('Highlighting row index:' + index + ' tserver:' + data.hostname);
-        $(row).css('background-color', 'gold');
-      }
+function refreshRecoveryList() {
+    $('#recovery-caption').hide(); // Hide the caption about highlighted rows on each refresh
+    getRecoveryList().then(function () {
+        recoveryList = [];
+        var data = sessionStorage.recoveryList === undefined ?
+                    [] : JSON.parse(sessionStorage.recoveryList);
+        data.recoveryList.forEach(function (entry) {
+            recoveryList.push(entry.server);
+        });
     });
-    refreshTServers();
-});
+}
 
 /**
- * Makes the REST calls, generates the tables with the new information
+ * Performs an ajax reload for the given Datatable
+ *
+ * @param {DataTable} table DataTable to perform an ajax reload on
  */
-function refreshTServers() {
-  getTServers().then(function() {
-    refreshBadTServersTable();
-    refreshDeadTServersTable();
-    refreshRecoveryList();
-    refreshTServersTable();
-  });
+function ajaxReloadTable(table) {
+    if (table) {
+        table.ajax.reload(null, false); // user paging is not reset on reload
+    }
 }
 
 /**
- * Used to redraw the page
+ * Refreshes data in the tserver table
  */
-function refresh() {
-  refreshTServers();
+function refreshTServersTable() {
+    refreshRecoveryList();
+    ajaxReloadTable(tserversTable);
 }
 
 /**
- * Generates the tservers rows
+ * Refreshes data in the deadtservers table
  */
-function refreshBadTServersTable() {
-  $('#badtservers').hide(); // Hide table on each refresh
-
-  var data = sessionStorage.tservers === undefined ?
-    [] : JSON.parse(sessionStorage.tservers);
-
-  clearTableBody('badtservers');
-
-  // return if the table is empty
-  if (data.length === 0 || data.badServers.length === 0)
-    return;
-
-  $('#badtservers').show();
-  $.each(data.badServers, function (key, val) {
-    var items = [];
-    items.push(createFirstCell(val.id, val.id));
-    items.push(createRightCell(val.status, val.status));
+function refreshDeadTServersTable() {
+    ajaxReloadTable(deadTServersTable);
 
-    $('<tr/>', {
-      html: items.join('')
-    }).appendTo('#badtservers tbody');
-  });
+    // Only show the table if there are non-empty rows
+    if ($('#deadtservers tbody .dataTables_empty').length) {
+        $('#deadtservers_wrapper').hide();
+    } else {
+        $('#deadtservers_wrapper').show();
+    }
 }
 
-
 /**
- * Generates the deadtservers rows
+ * Refreshes data in the badtservers table
  */
-function refreshDeadTServersTable() {
-  $('#deadtservers').hide(); // Hide table on each refresh
-
-  var data = sessionStorage.tservers === undefined ?
-    [] : JSON.parse(sessionStorage.tservers);
-
-  clearTableBody('deadtservers');
-
-  // return if the table is empty
-  if (data.length === 0 || data.deadServers.length === 0)
-    return;
-
-  $('#deadtservers').show();
-  $.each(data.deadServers, function (key, val) {
-    var items = [];
-    items.push(createFirstCell(val.server, val.server));
+function refreshBadTServersTable() {
+    ajaxReloadTable(badTServersTable);
 
-    var date = new Date(val.lastStatus);
-    date = date.toLocaleString().split(' ').join('&nbsp;');
-    items.push(createRightCell(val.lastStatus, date));
-    items.push(createRightCell(val.status, val.status));
-    items.push(createRightCell('', '<a href="javascript:clearDeadTServers(\'' +
-      val.server + '\');">clear</a>'));
+    // Only show the table if there are non-empty rows
+    if ($('#badtservers tbody .dataTables_empty').length) {

Review Comment:
   Nice! Sorry I missed this, ignore my other comment.



-- 
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


[GitHub] [accumulo] milleruntime commented on a diff in pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#discussion_r870244762


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/tservers.js:
##########
@@ -16,165 +16,115 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+/* JSLint global definitions */
+/*global
+    $, document, sessionStorage, getTServers, clearDeadServers, refreshNavBar,
+    getRecoveryList, bigNumberForQuantity, timeDuration, dateFormat
+*/
 "use strict";
 
-var tserversTable;
+var tserversTable, deadTServersTable, badTServersTable;
 var recoveryList = [];
 
 /**
- * Creates tservers initial table
+ * Checks if the given server is in the global recoveryList variable
+ * 
+ * @param {JSON} server json server object
+ * @returns true if the server is in the recoveryList, else false
  */
-$(document).ready(function() {
-
-  refreshRecoveryList();
-    
-    // Create a table for tserver list
-    tserversTable = $('#tservers').DataTable({
-      "ajax": {
-        "url": '/rest/tservers',
-        "dataSrc": "servers"
-      },
-      "stateSave": true,
-      "columnDefs": [
-          { "targets": "big-num",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = bigNumberForQuantity(data);
-              return data;
-            }
-          },
-          { "targets": "duration",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = timeDuration(data);
-              return data;
-            }
-          },
-          { "targets": "percent",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = Math.round(data * 100) + '%';
-              return data;
-            }
-          }
-        ],
-      "columns": [
-        { "data": "hostname",
-          "type": "html",
-          "render": function ( data, type, row, meta ) {
-            if(type === 'display') data = '<a href="/tservers?s=' + row.id + '">' + row.hostname + '</a>';
-            return data;
-          }
-        },
-        { "data": "tablets" },
-        { "data": "lastContact" },
-        { "data": "responseTime" },
-        { "data": "entries" },
-        { "data": "ingest" },
-        { "data": "query" },
-        { "data": "holdtime" },
-        { "data": "scansCombo" },
-        { "data": "minorCombo" },
-        { "data": "majorCombo" },
-        { "data": "indexCacheHitRate" },
-        { "data": "dataCacheHitRate" },
-        { "data": "osload" }
-      ],
-      "rowCallback": function (row, data, index) {
-        // reset background of each row
-        $(row).css('background-color', '');
-
-        // return if the current row's tserver is not recovering
-        if (!recoveryList.includes(data.hostname))
-          return;
-
-        // only show the caption if we know there are rows in the tservers table
-        $('#recovery-caption').show();
-
-        // highlight current row
-        console.log('Highlighting row index:' + index + ' tserver:' + data.hostname);
-        $(row).css('background-color', 'gold');
-      }
-    });
-    refreshTServers();
-});
+function serverIsInRecoveryList(server) {
+    return recoveryList.includes(server.hostname);
+}
 
 /**
- * Makes the REST calls, generates the tables with the new information
+ * Refreshes the list of recovering tservers and shows/hides the recovery caption
  */
-function refreshTServers() {
-  getTServers().then(function() {
-    refreshBadTServersTable();
-    refreshDeadTServersTable();
-    refreshRecoveryList();
-    refreshTServersTable();
-  });
+function refreshRecoveryList() {
+    getRecoveryList().then(function () {
+        // get list of recovering servers
+        recoveryList = [];
+        var data = sessionStorage.recoveryList === undefined ?
+                    [] : JSON.parse(sessionStorage.recoveryList);
+        data.recoveryList.forEach(function (entry) {
+            recoveryList.push(entry.server);
+        });
+
+        // get list of online tservers
+        data = sessionStorage.tservers === undefined ?
+                    [] : JSON.parse(sessionStorage.tservers).servers;
+
+        // show the recovery caption if its in the list of recovering servers
+        if (data.some(serverIsInRecoveryList)) {

Review Comment:
   Good job, this is very clear and simple. The only other improvement we could do is rename the "data" variable. There are a lot of instances of "data" being used but that is all over the monitor. LGTM



-- 
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


[GitHub] [accumulo] DomGarguilo commented on a diff in pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on code in PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#discussion_r869356499


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/tservers.js:
##########
@@ -183,29 +112,155 @@ function refreshDeadTServersTable() {
  * @param {string} server Dead TServer to clear
  */
 function clearDeadTServers(server) {
-  clearDeadServers(server);
-  refreshTServers();
-  refreshNavBar();
+    clearDeadServers(server);
+    refreshTServers();
+    refreshNavBar();
 }
 
 /**
- * Generates the tserver table
+ * Creates initial tables
  */
-function refreshTServersTable() {
-  if (tserversTable) tserversTable.ajax.reload(null, false); // user paging is not reset on reload
-}
+$(document).ready(function () {
 
-/**
- * Refreshes the list of recovering tservers used to highlight rows
- */
-function refreshRecoveryList() {
-  $('#recovery-caption').hide(); // Hide the caption about highlighted rows on each refresh
-  getRecoveryList().then(function () {
-    recoveryList = []
-    var data = sessionStorage.recoveryList === undefined ?
-      [] : JSON.parse(sessionStorage.recoveryList);
-    data.recoveryList.forEach(entry => {
-      recoveryList.push(entry.server);
+    refreshRecoveryList();
+
+    // Create a table for tserver list
+    tserversTable = $('#tservers').DataTable({
+        "ajax": {
+            "url": '/rest/tservers',
+            "dataSrc": "servers"
+        },
+        "stateSave": true,
+        "columnDefs": [
+            {
+                "targets": "big-num",
+                "render": function (data, type) {
+                    if (type === 'display') {
+                        data = bigNumberForQuantity(data);
+                    }
+                    return data;
+                }
+            },
+            {
+                "targets": "duration",
+                "render": function (data, type) {
+                    if (type === 'display') {
+                        data = timeDuration(data);
+                    }
+                    return data;
+                }
+            },
+            {
+                "targets": "percent",
+                "render": function (data, type) {
+                    if (type === 'display') {
+                        data = Math.round(data * 100) + '%';
+                    }
+                    return data;
+                }
+            }
+        ],
+        "columns": [
+            {
+                "data": "hostname",
+                "type": "html",
+                "render": function (data, type, row) {
+                    if (type === 'display') {
+                        data = '<a href="/tservers?s=' + row.id + '">' + row.hostname + '</a>';
+                    }
+                    return data;
+                }
+            },
+            { "data": "tablets" },
+            { "data": "lastContact" },
+            { "data": "responseTime" },
+            { "data": "entries" },
+            { "data": "ingest" },
+            { "data": "query" },
+            { "data": "holdtime" },
+            { "data": "scansCombo" },
+            { "data": "minorCombo" },
+            { "data": "majorCombo" },
+            { "data": "indexCacheHitRate" },
+            { "data": "dataCacheHitRate" },
+            { "data": "osload" }
+        ],
+        "rowCallback": function (row, data, index) {
+            // reset background of each row
+            $(row).css('background-color', '');
+
+            // return if the current row's tserver is not recovering
+            if (!recoveryList.includes(data.hostname)) {
+                return;
+            }
+
+            // only show the caption if we know there are rows in the tservers table
+            $('#recovery-caption').show();

Review Comment:
   Looks like jslint doesn't complain about the changes made in [086031e](https://github.com/apache/accumulo/pull/2685/commits/086031ecff58f4c089efb9eaa257218723a1553a)



-- 
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


[GitHub] [accumulo] milleruntime commented on a diff in pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#discussion_r869087999


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/tservers.js:
##########
@@ -183,29 +112,155 @@ function refreshDeadTServersTable() {
  * @param {string} server Dead TServer to clear
  */
 function clearDeadTServers(server) {
-  clearDeadServers(server);
-  refreshTServers();
-  refreshNavBar();
+    clearDeadServers(server);
+    refreshTServers();
+    refreshNavBar();
 }
 
 /**
- * Generates the tserver table
+ * Creates initial tables
  */
-function refreshTServersTable() {
-  if (tserversTable) tserversTable.ajax.reload(null, false); // user paging is not reset on reload
-}
+$(document).ready(function () {
 
-/**
- * Refreshes the list of recovering tservers used to highlight rows
- */
-function refreshRecoveryList() {
-  $('#recovery-caption').hide(); // Hide the caption about highlighted rows on each refresh
-  getRecoveryList().then(function () {
-    recoveryList = []
-    var data = sessionStorage.recoveryList === undefined ?
-      [] : JSON.parse(sessionStorage.recoveryList);
-    data.recoveryList.forEach(entry => {
-      recoveryList.push(entry.server);
+    refreshRecoveryList();
+
+    // Create a table for tserver list
+    tserversTable = $('#tservers').DataTable({
+        "ajax": {
+            "url": '/rest/tservers',
+            "dataSrc": "servers"
+        },
+        "stateSave": true,
+        "columnDefs": [
+            {
+                "targets": "big-num",
+                "render": function (data, type) {
+                    if (type === 'display') {
+                        data = bigNumberForQuantity(data);
+                    }
+                    return data;
+                }
+            },
+            {
+                "targets": "duration",
+                "render": function (data, type) {
+                    if (type === 'display') {
+                        data = timeDuration(data);
+                    }
+                    return data;
+                }
+            },
+            {
+                "targets": "percent",
+                "render": function (data, type) {
+                    if (type === 'display') {
+                        data = Math.round(data * 100) + '%';
+                    }
+                    return data;
+                }
+            }
+        ],
+        "columns": [
+            {
+                "data": "hostname",
+                "type": "html",
+                "render": function (data, type, row) {
+                    if (type === 'display') {
+                        data = '<a href="/tservers?s=' + row.id + '">' + row.hostname + '</a>';
+                    }
+                    return data;
+                }
+            },
+            { "data": "tablets" },
+            { "data": "lastContact" },
+            { "data": "responseTime" },
+            { "data": "entries" },
+            { "data": "ingest" },
+            { "data": "query" },
+            { "data": "holdtime" },
+            { "data": "scansCombo" },
+            { "data": "minorCombo" },
+            { "data": "majorCombo" },
+            { "data": "indexCacheHitRate" },
+            { "data": "dataCacheHitRate" },
+            { "data": "osload" }
+        ],
+        "rowCallback": function (row, data, index) {
+            // reset background of each row
+            $(row).css('background-color', '');
+
+            // return if the current row's tserver is not recovering
+            if (!recoveryList.includes(data.hostname)) {
+                return;
+            }
+
+            // only show the caption if we know there are rows in the tservers table
+            $('#recovery-caption').show();

Review Comment:
   Oh wait nvm JSlint probably told you not to.



-- 
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


[GitHub] [accumulo] milleruntime commented on a diff in pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
milleruntime commented on code in PR #2685:
URL: https://github.com/apache/accumulo/pull/2685#discussion_r870234677


##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/tservers.js:
##########
@@ -16,165 +16,115 @@
  * specific language governing permissions and limitations
  * under the License.
  */
+/* JSLint global definitions */
+/*global
+    $, document, sessionStorage, getTServers, clearDeadServers, refreshNavBar,
+    getRecoveryList, bigNumberForQuantity, timeDuration, dateFormat
+*/
 "use strict";
 
-var tserversTable;
+var tserversTable, deadTServersTable, badTServersTable;
 var recoveryList = [];
 
 /**
- * Creates tservers initial table
+ * Checks if the given server is in the global recoveryList variable
+ * 
+ * @param {JSON} server json server object
+ * @returns true if the server is in the recoveryList, else false
  */
-$(document).ready(function() {
-
-  refreshRecoveryList();
-    
-    // Create a table for tserver list
-    tserversTable = $('#tservers').DataTable({
-      "ajax": {
-        "url": '/rest/tservers',
-        "dataSrc": "servers"
-      },
-      "stateSave": true,
-      "columnDefs": [
-          { "targets": "big-num",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = bigNumberForQuantity(data);
-              return data;
-            }
-          },
-          { "targets": "duration",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = timeDuration(data);
-              return data;
-            }
-          },
-          { "targets": "percent",
-            "render": function ( data, type, row ) {
-              if(type === 'display') data = Math.round(data * 100) + '%';
-              return data;
-            }
-          }
-        ],
-      "columns": [
-        { "data": "hostname",
-          "type": "html",
-          "render": function ( data, type, row, meta ) {
-            if(type === 'display') data = '<a href="/tservers?s=' + row.id + '">' + row.hostname + '</a>';
-            return data;
-          }
-        },
-        { "data": "tablets" },
-        { "data": "lastContact" },
-        { "data": "responseTime" },
-        { "data": "entries" },
-        { "data": "ingest" },
-        { "data": "query" },
-        { "data": "holdtime" },
-        { "data": "scansCombo" },
-        { "data": "minorCombo" },
-        { "data": "majorCombo" },
-        { "data": "indexCacheHitRate" },
-        { "data": "dataCacheHitRate" },
-        { "data": "osload" }
-      ],
-      "rowCallback": function (row, data, index) {
-        // reset background of each row
-        $(row).css('background-color', '');
-
-        // return if the current row's tserver is not recovering
-        if (!recoveryList.includes(data.hostname))
-          return;
-
-        // only show the caption if we know there are rows in the tservers table
-        $('#recovery-caption').show();
-
-        // highlight current row
-        console.log('Highlighting row index:' + index + ' tserver:' + data.hostname);
-        $(row).css('background-color', 'gold');
-      }
-    });
-    refreshTServers();
-});
+function serverIsInRecoveryList(server) {
+    return recoveryList.includes(server.hostname);

Review Comment:
   Cool!



-- 
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


[GitHub] [accumulo] DomGarguilo merged pull request #2685: Convert deadTServers and badTServers tables to DataTables

Posted by GitBox <gi...@apache.org>.
DomGarguilo merged PR #2685:
URL: https://github.com/apache/accumulo/pull/2685


-- 
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