[
https://issues.apache.org/jira/browse/AMQ-6252?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15241891#comment-15241891
]
ASF subversion and git services commented on AMQ-6252:
------------------------------------------------------
Commit 19fd084a83c990f9fb75a5f2becb48ac808a1b36 in activemq's branch
refs/heads/master from [~tabish121]
[ https://git-wip-us.apache.org/repos/asf?p=activemq.git;h=19fd084 ]
https://issues.apache.org/jira/browse/AMQ-6252
Update for some added thread safety. Adds method healthStatus that will
regenrate the status from the healthList data which is more intuitive
than the getCurrentStatus which doesn't update state and requires
periodic calls to healthList to capture current metrics.
> 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)
