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/10/29 12:40:34 UTC

[GitHub] [accumulo] dlmarion opened a new issue, #3057: Annoying pop-up in Monitor

dlmarion opened a new issue, #3057:
URL: https://github.com/apache/accumulo/issues/3057

   During testing of 2.1.0 RC4 I shut down the compaction coordinator. I had the monitor open and was on the `/ec` page to look at the external compaction information. It appears that when the ajax calls to refresh the tables failed,  a pop-up would appear that said:
   
   `DataTables warning: table id=<tableName> - Ajax error. For more information about this error, please see http://datatables.net/tn/7`
   
   Is there a way to suppress this and do something different? When the `manager` is down, there is just a message on the main page saying it's down - no popup.


-- 
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.apache.org

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


[GitHub] [accumulo] ctubbsii commented on issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1297490676

   > One thing that I thought of, that I'm not sure how we want to handle, is if the page is refreshed, the banner indicating "Compaction Coordinator Not Running" shows up. But it doesn't appear that this will ever happen automatically via auto-refresh and will only show up on a full page reload. An alternative to moving the datatables errors to the console could be to just refresh the page (via `location.reload()`) so that the banner will show up and also not create the alerts. Maybe for now, we can just merge the change that logs the errors in the console and then as a follow on try the approach described here if it seems like that would be better.
   
   Definitely don't do the `location.reload()` thing. One of the biggest advantages of the 2.x monitor improvements to use AJAX principles is that you don't need to do a full page reload. You can do asynchronous updates to a page using less bandwidth and less disruption to the user experience scrolling, etc. We achieve this by creating REST endpoints for data we want to load, and use background JavaScript to load it and/or keep it up-to-date.


-- 
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] cshannon commented on issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
cshannon commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1295895112

   I looked at this in detail and there are several issues here of why this happens compared to the Manager page.
   
   1. The main issue is that the status for EC is cached for 1 minute. So when the page first loads if the coordinator was recently shut down then the api call of `/rest/ec` will still return a good status for a minute until the coordinator info is refreshed. Because of this the status page renders the tables and makes the table REST calls for data thinking that there is a compaction coordinator. The rest endpoints for the tables are then called and proceed to fail with a 500 error (Issue 2)
   2. Instead of returning no data or empty response the rest endpoints throw an IllegalStateException if the coordinator is down. This translates into a 500 error and is why the pop ups show up because the Ajax call had a server error. The rest endpoints on the manager page don't seem to throw a 500 error if the manager is down but these do.
   3. The EC page and manager page are a bit different with status checks. The manager page will check the status each time a refresh is done on a table to see if the manager is alive. If the manager is alive then it updates the page to show the window that it's down as described and hide the bottom table. The EC page only checks the status on first render of the page and only refreshed if you refresh the entire page. Status is not checked again if you click on one of the refresh buttons in the Compactors or Running Compactors table. So this would cause an error forever until the page is loaded again (due to the 500 error) even if the EC status updates after a minute.
   
   So based on this I think we need to decide what we want to do. 
   
   One thing that should happen either way is status should always be checked on refreshes of the tables. Besides that we probably need to do one or more of the following to fix this:
   
   1. Don't cache the status or provide a way for an immediate refresh of status to see if the coordinator is alive.
   2. Don't return a 500 error when querying the endpoints for data on external compactions, just return something like an empty list, etc. This will prevent the error and just show an empty table and then eventually the status would catch up and we could show that it's down and hide the tables.
   3. Keep throwing the 500 error if the endpoints are hit and figure out a way for the JavaScript code to catch the error and show an empty table instead of the pop up, or maybe print a message but not an alert. Or on 500 error we just immediately take that to mean that the coordinator is down and show the down message and hide the tables, etc. Basically just some sort of better handling of the 500 error on the rest call until the cached status catches up so that the page knows the coordinator is down.
   
   Number 3 seems like the best solution to me. Just figure out a way to handle the 500 error more gracefully until the server updates the cached status to show it's down. (And of course as I said make sure the refreshes check status first).
   
   @dlmarion and @ctubbsii thoughts?
   
   Also, @DomGarguilo - Can you look at this too and see what you think about the best way to handle the issue? I saw that you worked a lot recently on the monitor including the status stuff for the manager page so you may have a better idea on how to fix it or know of something I'm not aware of as my JavaScript is pretty rusty. I'm wondering if there is something simple in the data tables to just better handle the failed Ajax call but I didn't get a chance to look into it yet. If you wanted to take a shot at fixing it feel free too otherwise I can get back to it probably Friday or next weekend.


-- 
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] ctubbsii commented on issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1297012835

   @cshannon asked:
   > @ctubbsii thoughts?
   
   Seems like this is outcome-driven. Any of the options you suggested could work, as long as it yields the desired behavior. I don't have an opinion about which to pursue.


-- 
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] cshannon commented on issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
cshannon commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1297480315

   I think it makes sense to go ahead and make the easy/quick change to get rid of the annoying pop up as that is a big improvement. Then you can create a follow on issue for the reload/refresh and handling of the banner if we want to do something more.


-- 
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] cshannon commented on issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
cshannon commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1297261859

   > I found this excerpt on the [datatables website](https://datatables.net/manual/tech-notes/7#Resolution):
   > 
   > > If you are willing to accept the error (for example if you cannot alter the backend system to fix the error), but don't want your end users to see the alert() message, you can change DataTables' error reporting mechanism to throw a Javascript error to the browser's console, rather than alerting it. This can be done using:
   > > $.fn.dataTable.ext.errMode = 'throw';
   > 
   > Which is a possibility and might be the easiest "fix" for this.
   
   Yeah, the simplest thing I think is just handling the error and not showing the pop up. I figured datatables had a way to do it but I've never used the library (hence deferring to you to see if you knew). I can give this a shot later today and create a PR if it works.


-- 
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 issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1297474979

   > I found this excerpt on the [datatables website](https://datatables.net/manual/tech-notes/7#Resolution):
   > 
   > > If you are willing to accept the error (for example if you cannot alter the backend system to fix the error), but don't want your end users to see the alert() message, you can change DataTables' error reporting mechanism to throw a Javascript error to the browser's console, rather than alerting it. This can be done using:
   > > $.fn.dataTable.ext.errMode = 'throw';
   > 
   > Which is a possibility and might be the easiest "fix" for this.
   
   So I had some time to try this out and it works well. The errors just gets pushed to the console instead of creating alerts.
   
   @cshannon I can go ahead and put up a PR for this since I have a branch with these changes anyways.
   
   One thing that I thought of, that I'm not sure how we want to handle, is if the page is refreshed, the banner indicating "Compaction Coordinator Not Running" shows up. But it doesn't appear that this will ever happen automatically via auto-refresh and will only show up on a full page reload. An alternative to moving the datatables errors to the console could be to just refresh the page (via `location.reload()`) so that the banner will show up and also not create the alerts. Maybe for now, we can just merge the change that logs the errors in the console and then as a follow on try the approach described here if it seems like that would be better.


-- 
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 issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1297254759

   I found this excerpt on the [datatables website](https://datatables.net/manual/tech-notes/7#Resolution):
   
   > If you are willing to accept the error (for example if you cannot alter the backend system to fix the error), but don't want your end users to see the alert() message, you can change DataTables' error reporting mechanism to throw a Javascript error to the browser's console, rather than alerting it. This can be done using:
   $.fn.dataTable.ext.errMode = 'throw';
   
   Which is a possibility and might be the easiest "fix" for this. 


-- 
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] dlmarion commented on issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
dlmarion commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1297124132

   > I don't have an opinion about which to pursue.
   
   Same. I would implement the easiest solution to stop the pop-ups, whatever that might be. 


-- 
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 issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1297499300

   > > One thing that I thought of, that I'm not sure how we want to handle, is if the page is refreshed, the banner indicating "Compaction Coordinator Not Running" shows up. But it doesn't appear that this will ever happen automatically via auto-refresh and will only show up on a full page reload. An alternative to moving the datatables errors to the console could be to just refresh the page (via `location.reload()`) so that the banner will show up and also not create the alerts. Maybe for now, we can just merge the change that logs the errors in the console and then as a follow on try the approach described here if it seems like that would be better.
   > 
   > Definitely don't do the `location.reload()` thing. One of the biggest advantages of the 2.x monitor improvements to use AJAX principles is that you don't need to do a full page reload. You can do asynchronous updates to a page using less bandwidth and less disruption to the user experience scrolling, etc. We achieve this by creating REST endpoints for data we want to load, and use background JavaScript to load it and/or keep it up-to-date.
   
   Yea that makes sense. The logic to show the mentioned banner is in the freemarker code and I'm not exact sure how that works but from what it seems, the page needs to be refreshed in order for that if statement that shows/hides the banner to be reached again. We could work out a way for that logic to be moved into the javascript so that it can be updated as part of the pages `refresh()` but that should probably take place as part of a separate ticket.


-- 
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] ctubbsii closed issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
ctubbsii closed issue #3057: Annoying pop-up in Monitor
URL: https://github.com/apache/accumulo/issues/3057


-- 
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 issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
DomGarguilo commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1295902584

   > Also, @DomGarguilo - Can you look at this too and see what you think about the best way to handle the issue?
   
   Sure thing 👍 ill take a look when I get a chance. Probably this coming week. 


-- 
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] cshannon commented on issue #3057: Annoying pop-up in Monitor

Posted by GitBox <gi...@apache.org>.
cshannon commented on issue #3057:
URL: https://github.com/apache/accumulo/issues/3057#issuecomment-1297253543

   Sounds good, I will let @DomGarguilo take a look this week and see what he thinks and I figure we can work together on a solution here.


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