kevinrr888 commented on code in PR #5873:
URL: https://github.com/apache/accumulo/pull/5873#discussion_r2334183995
##########
server/monitor/src/main/java/org/apache/accumulo/monitor/next/InformationFetcher.java:
##########
@@ -270,7 +275,13 @@ public void run() {
// Only refresh every 5s (old monitor logic).
while (!newConnectionEvent.get() && connectionCount.get() == 0
&& NanoTime.millisElapsed(refreshTime, NanoTime.now()) > 5000) {
- Thread.onSpinWait();
+ try {
+ Thread.sleep(100);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new IllegalStateException(
+ "Thread " + Thread.currentThread().getName() + " interrupted",
e);
+ }
Review Comment:
Will cause Monitor to terminate when the IllegalStateException occurs (this
is thrown in the run method of a critical thread). Maybe there's some other way
we want to handle this or maybe this is good.
##########
server/monitor/src/main/resources/org/apache/accumulo/monitor/resources/js/functions.js:
##########
@@ -637,14 +637,6 @@ function getSserversDetail(group) {
return getJSONForTable(url, sessionDataVar);
}
-/**
- * REST GET call for /manager,
- * stores it on a sessionStorage variable
- */
-function getManager() {
- return getJSONForTable(REST_V2_PREFIX + '/manager', 'manager');
-}
-
Review Comment:
With this, there existed 2 getManager functions. This was the one being used
since it's the last defined. The restv2 endpoint for manager was not compatible
with the front end, leading to issues. Deleting this causes the rest/manager
endpoint to be used instead for getManager, which is compatible with existing
front end.
##########
server/monitor/src/main/java/org/apache/accumulo/monitor/next/Endpoints.java:
##########
@@ -119,15 +121,27 @@ public Map<String,String> getEndpoints(@Context
HttpServletRequest request) {
@Produces(MediaType.APPLICATION_JSON)
@Description("Returns a list of the resource groups that are in use")
public Set<String> getResourceGroups() {
- return monitor.getInformationFetcher().getSummary().getResourceGroups();
+ try {
+ return monitor.getInformationFetcher().getSummary().getResourceGroups();
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ throw new ServiceUnavailableException(
+ Response.status(Response.Status.SERVICE_UNAVAILABLE).build(), e);
+ }
Review Comment:
https://en.wikipedia.org/wiki/List_of_HTTP_status_codes seems like 5xx
status code would be appropriate. `503 Service Unavailable` seemed appropriate
--
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]
