james created AMQ-6252:
--------------------------
Summary: HealthView has a variety of weaknesses
Key: AMQ-6252
URL: https://issues.apache.org/jira/browse/AMQ-6252
Project: ActiveMQ
Issue Type: Bug
Components: Broker
Affects Versions: 5.13.2, 5.9.1
Reporter: james
Priority: Minor
The HealthView utility has a variety of weaknesses.
* The currentState field should be volatile to ensure correct visibility in a
multi-threaded environment
* the assignment to the currentState field at the end of the healthList method
should only happen once the _final_ string has been built. the current code
could expose a caller to a partially built string (since the field is
continually re-assigned as the string is built).
On a separate note, the semantics of this class are non-obvious. We were
monitoring our broker via jmx using the CurrentState attribute for a year or so
now and i only just realized that the value is useless. The value is useless
because _it is only updated if you call the healthList() method first_. The
CurrentState attribute is the one someone would most likely monitor using an
external automated management product, yet it is not useful by itself. I would
recommend one of the following approaches:
* at the very least, the current behavior should be _clearly documented_ so
that users know they have to call one of the other methods first in order to
get a valid CurrentState
* Alternately, it would be nice if calling the getCurrentState() by itself were
sufficient. this could be accomplished via one of the following approaches:
** Have the BrokerService schedule a periodic task to invoke the healthList()
method in order to ensure that the CurrentState attribute gets updated.
** Track a "last updated" field within the HealthView class and have the
getCurrentState() method itself invoke the healthList() method first if the
currentState is too old.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
