[
https://issues.apache.org/jira/browse/AMQ-6252?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15241837#comment-15241837
]
Timothy Bish commented on AMQ-6252:
-----------------------------------
Since the Broker doesn't know if anyone is even looking at the HealthView bits
it wouldn't be optimal to be doing such a heavyweight operation for no good
reason which is why there isn't a scheduled task. The last updated value has
some merit although it is hard to settle on what an optimal default for that
would be given that everyone would have their own opinion.
> 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.9.1, 5.13.2
> 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)
