JBAS-4180 - HA <-> JMX coupling

[galder.zamarreno]

Re: http://jira.jboss.com/jira/browse/JBAS-4180

------

The first thing I've done is evaluate our current usage of JMX, more specifically subclasses of ServiceMBeanSupport, in clustering related code. Here's the list:

- BarrierController
- Barrier (inner class of above)

- HAServiceMBeanSupport
- HASingletonSupport (extends above)
- HASingletonController (extends above)

- HASingletonElectionPolicyBase
- HASingletonElectionPolicySimple (extends above)

- FarmMemberService

- ClusterPartition

- UnifiedInvoker
- UnifiedInvokerHA (extends above)

- DetachedHANamingService
- HANamingService (extends above)

- HASessionStateService

- ListenerServiceMBeanSupport
- BarrierController (extends above)

- PingJndi

------

And below is my analisys of these classes:

+ Barrier, BarrierController, HASingletonController, HASingletonSupport and HAServiceMBeanSupport

HASingletonController extends HASingletonSupport which sends notifications related to node becoming coordinator, or node stopping being coordinator. BarrierControler is the listener of these notifications, who's job is to start/stop of dependant services. Barrier is wrapper around the dependant service, which is started/stopped upon BarrierController's command via lifecycle methods.

1. BarrierController listens for JMX notifications, so should probably remain the same. Does not seem to make much sense to have JMX pulled out because it's its core business.
2. Whether Barrier should be JMX aware is debatable. Extends ServiceMBeanSupport directly and does little more. It's a mere delegator.
3. HASingletonController does nothing JMX related, except publicise (via getters) the configuration options for target start/stop methods. Needs JMX factoring out IMO.
4. HASingletonSupport knows when a node is coordinator or not, and via its parent, sends JMX notifications (only local notifications). HAServiceMBeanSupport can send JMX notifications locally or broadcast them accross the cluster. Both rely heavily on JMX functionality, so could stay the same.
5.- JBAS-3982 could be resolved with the JMX factored version of HAServiceMBeanSupport.

+ HASingletonElectionPolicyBase and HASingletonElectionPolicySimple

This is new functionality to decide where to run HA singleton services instead of the coordinator. It makes sense to factor JMX out of these classes to separate functionality from any JMX managed methods. An election policy should be independent, like the load balance policies.

+ FarmMemberService

This class extends URLDeploymentScanner, but has no meaningful JMX methods, FarmMemberServiceMBean. Does not seem to require a JMX aspect to it. I guess in the future it could send notifications when a deployment was successfully deployed in all nodes, once farm deployments are atomic, but this has not been implemented yet.

+ ClusterPartition

Does not make sense to have a non JMX version.

+ UnifiedInvoker and UnifiedInvokerHA

I'm not going to discuss legacy invokers here because they should remain as they are for backwards compatibility. To discuss JMX factoring out, let's focus on the unified invokers.

To factor out JMX from invokers makes sense to help unit testing and potentially make path for them to be migrated to Remoting (there's a JIRA on this) and be used standalone. It would also make my life a lot easier unit testing JBAS-4455 :)

+ DetachedHANamingService and HANamingService

DetachedHANamingService uses JMX (via Query) to find an HAPartition with a given name via findHAPartitionWithName method. Couldn't find any references to this method and it's not a managed method either. Javadoc for DetachedHANamingService states: "Management Bean for the protocol independent HA-JNDI service. This allows the naming service transport layer to be provided by a detached invoker service like JRMPInvokerHA + ProxyFactoryHA.".

HANamingService's javadoc says: "Management Bean for HA-JNDI service for the legacy version that is coupled to the RMI/JRMP protocol. The DetachedHANamingService should be used with the appropriate detached invoker service."

I'm not sure about the meaning of the javadocs for these two services. The invokers they refer to are legacy, so maybe legacy these are legacy classes and we shouldn't touch them? Does HA JNDI without JMX makes sense? If you're using JNDI to start with, chances are you're in a managed environment.

In fact, any legacy/deprecated classes should be marked as such for newcomers to be aware of :)

+ HASessionStateService

This looks like legacy code, so no need to touch it?

+ PingJndi

Once again, legacy code?

-----

Thoughts? Comments? Bored? :)

[galder.zamarreno]

Sorry, "5.- JBAS-3982 could be resolved with the JMX factored version of HAServiceMBeanSupport. " should have said: "5.- JBAS-3982 should take in account the decision on HAServiceMBeanSupport, which provides infrastructure for cluster wide notifications"

[galder.zamarreno]

It's worth noting that I can see a argument in favour of factoring out JMX from HASingletonSupport.

For example, you could factor sending JMX HASingletonSupport when stop-being/becoming master by injecting this aspects via AOP as the forum thread in the JIRA suggests. But then again, does it make sense for HASingletonSupport to be a Pojo?

[brian.stansberry]

Very quick response:

Note the issue here is whether classes extend ServiceMBeanSupport, not whether they expose a JMX interface.

Also, have a look at http://www.jboss.com/index.html?module=bb&op=viewtopic&t=118608 . Came up a couple weeks back. Showed me a benefit of subclassing ServiceMBeanSupport -- get to take advantage of the already coded methods that prevent duplicate lifecycle invocations when a MC bean is annotated with @JMX.

+ Barrier, BarrierController, HASingletonController, HASingletonSupport and HAServiceMBeanSupport

HASingletonController extends HASingletonSupport which sends notifications related to node becoming coordinator, or node stopping being coordinator. BarrierControler is the listener of these notifications, who's job is to start/stop of dependant services. Barrier is wrapper around the dependant service, which is started/stopped upon BarrierController's command via lifecycle methods.

1. BarrierController listens for JMX notifications, so should probably remain the same. Does not seem to make much sense to have JMX pulled out because it's its core business.

Makes sense

2. Whether Barrier should be JMX aware is debatable. Extends ServiceMBeanSupport directly and does little more. It's a mere delegator.

No opinion. Not a priority either way.

3. HASingletonController does nothing JMX related, except publicise (via getters) the configuration options for target start/stop methods. Needs JMX factoring out IMO.

The barrier counts on the notifications, and other users may be counting on the notifications as well, so if there is a refactoring to provide a non-JMX version, the std HASingletonController class needs to provide the notifications.

4. HASingletonSupport knows when a node is coordinator or not, and via its parent, sends JMX notifications (only local notifications). HAServiceMBeanSupport can send JMX notifications locally or broadcast them accross the cluster. Both rely heavily on JMX functionality, so could stay the same.
5.- 5.- JBAS-3982 should take in account the decision on HAServiceMBeanSupport, which provides infrastructure for cluster wide notifications"

Sounds right.

+ HASingletonElectionPolicyBase and HASingletonElectionPolicySimple

This is new functionality to decide where to run HA singleton services instead of the coordinator. It makes sense to factor JMX out of these classes to separate functionality from any JMX managed methods. An election policy should be independent, like the load balance policies.

+1.

+ FarmMemberService

Ignore this.

+ ClusterPartition

Does not make sense to have a non JMX version.

Sure it does. Again, the issue here is whether classes extend ServiceMBeanSupport, not whether they expose a JMX interface.

That said, changing this isn't a priority.

+ UnifiedInvoker and UnifiedInvokerHA

I'm not going to discuss legacy invokers here because they should remain as they are for backwards compatibility. To discuss JMX factoring out, let's focus on the unified invokers.

+1.

To factor out JMX from invokers makes sense to help unit testing and potentially make path for them to be migrated to Remoting (there's a JIRA on this) and be used standalone. It would also make my life a lot easier unit testing JBAS-4455 :)

Sadly, unlikely to happen before beta3 and thus not in 5.0.0. In any case, this should be driven by what's done with the non-HA version.

+ DetachedHANamingService and HANamingService

DetachedHANamingService uses JMX (via Query) to find an HAPartition with a given name via findHAPartitionWithName method. Couldn't find any references to this method and it's not a managed method either.

That method should be removed; the partition should be dependency injected. I'm quite sure it already is injected; that method is probably just leftover cruft.

+ HASessionStateService

This looks like legacy code, so no need to touch it?

+ PingJndi

Once again, legacy code?

+1 to ignoring both. Don't even know what PingJndi is.

Thoughts? Comments? Bored? :)

Bored ;-)

Thanks much for this. Even if we don't change much for 5.0.0 this is most helpful.

[brian.stansberry]

Note the issue here is whether classes extend ServiceMBeanSupport, not whether they expose a JMX interface.

Beyond even whether they subclass ServiceMBeanSupport, the question is whether they can function properly if deployed as simple microcontainer beans.

Example, for a class providing functions like HASingletonController, as an implementation detail we may decide to subclass ServiceMBeanSupport and thus pick up some of its code base. But, if I don't need the controller to send out JMX notifications, it shouldn't require access to a JMX server to function.

[galder.zamarreno]

3. HASingletonController does nothing JMX related, except publicise (via getters) the configuration options for target start/stop methods. Needs JMX factoring out IMO.

The barrier counts on the notifications, and other users may be counting on the notifications as well, so if there is a refactoring to provide a non-JMX version, the std HASingletonController class needs to provide the notifications.

Not sure I understand what you mean here. HASingletonSupport provides the notifications, not HASingletonController. HASingletonController just extends the start/stop singleton methods that are called by HASingletonSupport. In between the call to these methods, HASingletonSupport sends the notifications.

HASingletonController, using the configured target methods, within start/stop singleton methods, calls them via the mbean server.

From this point of view, not sure what we gain by having HASingletonController extend HASingletonSupport and so extend ServiceMBeanSupport.

I'm not sure how to do this cos I know little about MC, but any call to lifecycle methods (read singleton lifecycle) methods should really be done by the MC, shouldn't it? Other than that, HASingletonController just tells what are these methods. Maybe HASingletonController could just go?

Maybe we could create @StartSingleton/@StopSingleton annotations and target beans could annotate such methods with these annotations. The MC then can call them. I might be talking absolute rubbish here though! :)

How HASingletonSupport would instruct MC to do this is unknown to me.

+ HASingletonElectionPolicyBase and HASingletonElectionPolicySimple

This is new functionality to decide where to run HA singleton services instead of the coordinator. It makes sense to factor JMX out of these classes to separate functionality from any JMX managed methods. An election policy should be independent, like the load balance policies.

+1.

TODO1 for JBAS-4180

+ ClusterPartition

Does not make sense to have a non JMX version.

Sure it does. Again, the issue here is whether classes extend ServiceMBeanSupport, not whether they expose a JMX interface.

That said, changing this isn't a priority.

Hmmmm, I guess it'd make unit testing CP a lot easier if it didn't extend ServiceMBeanSupport? Other than that, I don't see it being used standalone to provide a non ServiceMBeanSupport extending version.

To factor out JMX from invokers makes sense to help unit testing and potentially make path for them to be migrated to Remoting (there's a JIRA on this) and be used standalone. It would also make my life a lot easier unit testing JBAS-4455 :)

Sadly, unlikely to happen before beta3 and thus not in 5.0.0. In any case, this should be driven by what's done with the non-HA version.

Not a TODO of JBAS-4180

+ DetachedHANamingService and HANamingService

DetachedHANamingService uses JMX (via Query) to find an HAPartition with a given name via findHAPartitionWithName method. Couldn't find any references to this method and it's not a managed method either.

That method should be removed; the partition should be dependency injected. I'm quite sure it already is injected; that method is probably just leftover cruft.

TODO2 for JBAS-4180, remove the findHAPartitionWithName method.

What about whether DetachedHANamingService and HANamingService should extend ServiceMBeanSupport?

+ PingJndi
...

... Don't even know what PingJndi is.

Hidden gem maybe? hehehe ;)

Thanks much for this. Even if we don't change much for 5.0.0 this is most helpful.

no probs :-D

[brian.stansberry]

HASingletonController can have the target bean directly injected, in which case it will not invoke the target's methods via JMX.

Your comment about @StartSingleton, etc. is valid, but lets defer to a different discussion. Not going to happen for beta3.

What does a class gain by subclassing ServiceMBeanSupport:

1) Benefits from the lifecycle coding. Stuff that prevents startService() being called again when service is already started, etc.
2) Support for exposing service lifecycle state as an mbean attribute (getState())
3) I gets a ref to the mbean server when an instance is registered in JMX. Can use that to make JMX calls.
4) Probably others; part of task is to identify and see if relevant

1) and 2) are conceptually different from 3). Illustration:

Class implements the lifecycle methods; deployed by MC as a pojo service. MC invokes lifecycle methods.

In the -beans.xml the bean is annotated with @JMX. Sole purpose of this is to get the bean registered in JMX. Doing this is not a functional requirement of the class; it's a deployment decision for *that particular instance of the class*. MC will create a JMX proxy to the service and deploy that as well. When the proxy is deployed, the create/start/stop/destroy methods get called *again*. This is where benefit 1) is useful. You could get the same benefit by coding equivalent #1 functionality in the bean class itself, or in some PojoServiceSupport superclass.

If the class needs to use the mbean server to do its work, then you start getting into benefit #3. You need a ref to the server.

What I want here is the #3 cases isolated, particularly for the HASingletonController/HASingletonSupport. That class should be usable without it having a ref to an MBeanServer if:

1) user doesn't need the JMX notifications
2) user configures the deployment to dependency inject the target rather than injecting an ObjectName.
3) Other things you might find by digging.

Don't worry about making DetachedHANamingService not extend ServiceMBeanSupport. It's the singleton stuff where I can imagine wanting to have a lot of deployment flexibility.