Transaction Sticky LB policy for 4.2/trunk

[galder.zamarreno]

A few months ago, out of a Skype chat between Brian and myself, the following jira was created: http://jira.jboss.com/jira/browse/JBAS-4455

A few weeks ago, a user came up with exactly the same request, and out of it, an example 4.0.x implementation was created, see: http://wiki.jboss.org/wiki/Wiki.jsp?page=TransactionStickyLoadBalancePolicy40x

It is my intention now, seeing the user's interest in having this in 4.2, to implement this in trunk and 4.2. I'd like to provide a transanction sticky load balance policy for both transactions started via client side UserTransaction and within AS that involves:

1.- Enhancing UnifiedInvokerHAProxy so that:
1a- txFailoverAuthorizations is keyed on tpc (transaction propagation context) rather on javax.transaction.Transaction. By doing this, both client side transactions and in-AS transactions are covered.
1b- for txFailoverAuthorizations, to associate a target per tpc, which is the transaction sticky target. This target would be the typical output from a LoadBalance policy.
1c- whenever an invocation comes in, it checks whether there's a target associated with the incoming tpc within the txFailoverAuthorizations map. If it is, it'll add the target to the transient invocation payload.
1d- invocationHasReachedAServer() associates, if there's no target for that the current running tpc (i.e it's the first invocation of the tx and has already come back from the server), the chosen target with the tpc in the txFailoverAuthorizations map.
1e- rename txFailoverAuthorizations to something more meaningful.
1f- assuming there's no other external usages, encapsulate the use of the renamed txFailoverAuthorizations. This map is currently public.

2.- Create a new load balance policy called TransactionSticky that:
2a- checks whether there's any target in the invocation transient payload map (take in account 2c that comes first and would have added the target to the transient payload, if the transaction had a target in the txFailoverAuthorizations) and whether this target is part of the familyClusterInfo (if it isn't, then this is an unavailable target and should not be used), in which case, returns this target.
2b- extends RoundRobin, so if there's no target in the transient paylod, or the target is invalid, then a new target is chosen based on RoundRobin chooseTarget method implementation. With this, we achieve tx1 invocations to node1, tx2 to node2, tx3 to node1, tx4 to node2...etc.

For the 4.0.x implementation, I have:
1- tested Client side user transactions
2- tested how different ejb invocations (including usage of different ejbs) within a transaction hit the same node for the duration of the transaction.
3- created mock implementations of my invoker and invoker proxy implementations so that they can simulate different failure scenarios. This led to deploying mock EJB versions that used mock invoker and invoker proxy based invoker proxy bindings and container configurations.
4- failure scenarios tested are:
4a- within a transaction, failure of the Nth invocation before reaching the server, i.e. before proxy.invoker(). This failure should be propagated back to the client.
4b- within a transaction, failure of the Nth invocation after reaching the server. This failure should be propagated back to the client because the transaction had already started in the server.
4c- within a transaction, failure of the 1st invocation in the 1st server (as GenericClusteringException.COMPLETED_NO) but success in the second, that should be transparent to the client as the failover is allowed.
4c- within a transaction, failure of the 1st invocation in the 1st server (as GenericClusteringException.COMPLETED_YES). This failure should be propagated back to the client because the transaction reached the server and so failover is not allowed.
4d- within a transaction, failure of the 1st invocation in both servers. This failure should be propagated back to the client.

On top of the tests created for 4.0.x, I hope to add the following to 4.2/trunk:
1.- tests with in-AS started transactions, i.e. with EJBs as clients.
2.- concurrent EJB invocations (using different transactions) to guarantee the safety on the UnifiedInvokerHA/UnifiedInvokerHAProxy if they don't exist already.

Thoughts?

[brian.stansberry]

A quick thought is that one LoadBalancePolicy impl is not enough. The LBP needs to work with the existing semantics if there is no transaction.

[galder.zamarreno]

What do you mean by existing semantics? Do you mean existing defaults?

TransactionSticky LBP would do:

Object txStickyTarget = invocation.getTransientValue("TX_STICKY_TARGET");
 if (txStickyTarget != null && familyClusterInfo.getTargets().contains(txStickyTarget))
 {
 log.trace("transaction bound target exists: " + txStickyTarget);
 return txStickyTarget;
 }

 return chooseNewTarget(familyClusterInfo, invocation);

where chooseNewTarget is:

private Object chooseNewTarget(FamilyClusterInfo familyClusterInfo, Invocation invocation)
 {
 Object newTarget = super.chooseTarget(familyClusterInfo, invocation);
 log.trace("new target chosen: " + newTarget);
 invocation.getTransientPayload().put("TX_STICKY_TARGET", newTarget);

 return newTarget;
 }

If there's no transaction, txStickyTarget will be null. txStickyTarget will also be null in the very first call of a transaction.

This is because at the beginning of each invocation, UnifiedInvokerHAProxy checks whether there's a running transaction and whether this transaction has a target in the map. If both are true, it adds the txStickyTarget to the transient invocation payload.

If there's no transaction running, or is the first call of a transaction, the txStickyTarget will be null falling back on standard RoundRobin LBP and will also be added to the transient invocation payload.

Adding txStickyTarget to the transient invocation payload if no transactions are running would have no secondary effects, because any access/check in the txFailoverAuthorizations would require an existing tpc. However, I agree that adding it is an unncessary operation in this situation.

[galder.zamarreno]

There's one situation where existing defaults would change which is if TransactionSticky LBP was assumed the default for SFSBs.

If there's no transactions running, SFSB invocations would default on round robin, whereas the current default is FirstAvailable.

[brian.stansberry]

Don't assume the client uses a transaction. A requirement for JBAS-4455 is that the LBP impl has to work the same as the existing impls if there is no tx in effect.

1) If you only create a RoundRobin version of this, and people configure it for an SFSB, a client that doesn't start a tx can't get sticky session semantics (FirstAvailable). Even if they do start w/ a tx, they become unstuck if they keep using the bean after they commit the tx.

2) People who want Random semantics if no tx is in effect also can't have that.

3) Same for FirstAvailableIdenticalAllProxies.

[brian.stansberry]

Cross-post. :)

[brian.stansberry]

A minor limitation of this is the tpc/target map is stored as a static field in the invoker proxy. So, if two beans involved in the tx use different proxy types (say unified and pooled), the chosen server won't be the same. There's no real good way around this, since use of different proxy types implies different invokers on the server side, and hence different sets of targets.

The above comment is meant for the tiny fine print at the bottom of the documentation of this. ;-)

[galder.zamarreno]

Don't assume the client uses a transaction. A requirement for JBAS-4455 is that the LBP impl has to work the same as the existing impls if there is no tx in effect.

1) If you only create a RoundRobin version of this, and people configure it for an SFSB, a client that doesn't start a tx can't get sticky session semantics (FirstAvailable). Even if they do start w/ a tx, they become unstuck if they keep using the bean after they commit the tx.

2) People who want Random semantics if no tx is in effect also can't have that.

3) Same for FirstAvailableIdenticalAllProxies.

Yeah, you're right on this. So, I believe you suggest that I create different versions of TxLoadBalancePolicy which default on the ones mentioned above (as well as RR) if no transactions are running, correct?

[galder.zamarreno]

A minor limitation of this is the tpc/target map is stored as a static field in the invoker proxy. So, if two beans involved in the tx use different proxy types (say unified and pooled), the chosen server won't be the same. There's no real good way around this, since use of different proxy types implies different invokers on the server side, and hence different sets of targets.

The above comment is meant for the tiny fine print at the bottom of the documentation of this. ;-)

As far as 4.2 is concerned, JRMP/Pooled and Unified invokers exist although the default ones are Unified. JRMP/Pooled are legacy so is there any reason why customers would want to use these? I'd expect for Unified to be at least as good as JRMP/Pooled, but I can see customers wanting to stick their environment to what it was like in 4.0.x, i.e. having as less changes as possible.

I haven't checked the trunk codebase, but the descriptors in trunk don't even show the JRMP/Pooled invokers.

To sum up, +1 on fine print :)

[brian.stansberry]

So, I believe you suggest that I create different versions of TxLoadBalancePolicy which default on the ones mentioned above (as well as RR) if no transactions are running, correct?

Correct.

Re: the different invoker types, is it a big deal to do them all? I suppose the biggest hassle is testing. Do you think it's possible to unit test this with mock objects, letting you basically plug in the different invokers into the same test fixture without having to deploy to the AS?

One reason to fix them all is that I believe the current mechanism for checking if failover is allowed is broken; i.e it uses the Transaction from Invocation.getTransaction() rather than the TransactionPropagationContext. Thus it misses the case where UserTransaction is used. So, fixing that is a side benefit of JBAS-4555.

The JRMP/Pooled invokers still exist in trunk and are deployed. They are gone from standard-jboss.xml though.

[galder.zamarreno]

Re: the different invoker types, is it a big deal to do them all? I suppose the biggest hassle is testing. Do you think it's possible to unit test this with mock objects, letting you basically plug in the different invokers into the same test fixture without having to deploy to the AS?

Without deploying to AS?Hmmmmm, not sure. My current tests for 4.0.x worked by deploying EJBs using Mock invoker/invokerproxy versions that injected failures. I can look into it though.

One reason to fix them all is that I believe the current mechanism for checking if failover is allowed is broken; i.e it uses the Transaction from Invocation.getTransaction() rather than the TransactionPropagationContext. Thus it misses the case where UserTransaction is used. So, fixing that is a side benefit of JBAS-4555.

+1

Ok, I'll implement this for all invokers and will look into the most efficient way to unit test them all.

Re: different load balance load balance policies

Couple of questions came to my mind here:
1.- Rather than implementing 4 brand new policies, it might be easier to add a new element to cluster-config XML called transaction-sticky with true/false values and modify the existing load balance policies to act upon the value of that element, what do you think? This hasn't been fully baked, it's just something that I had in mind.

2.- Should transaction sticky (either implementations or solution mentioned in 1) be the default? IMO, 4.x should maintain existing (default) behavior while transaction sticky should be default in trunk. Thoughts?

[brian.stansberry]

Ok, I'll implement this for all invokers and will look into the most efficient way to unit test them all.

Good. Take the "don't deploy in AS" thing as just an idea I threw out. That comes from a general feeling I have that unit testing in clustering is inadequate because it's too hard to 1) write complex deployment descriptors that cover all the cases, package them and then deploy them 2) properly check system state when you can only expose that state via JMX or as part of a web or ejb response.

1.- Rather than implementing 4 brand new policies, it might be easier to add a new element to cluster-config XML called transaction-sticky with true/false values and modify the existing load balance policies to act upon the value of that element, what do you think? This hasn't been fully baked, it's just something that I had in mind.

Hmm. I don't want to reject that idea yet, but I'm concerned about pushing more config stuff into both EJB3 and EJB2. Plus, theoretically these policies can be used outside of EJB, so we'd have to find a way to expose the option wherever they are used.

Also, doing that requires a new field in the LBP impls, which changes their serialization. That will break clients using the old versions, so no good in 4.x and something to avoid in 5.

Also means changing the LBP interface to add a setTransactionSticky(boolean) method.

I know where you're coming from though. The proliferation of classes gets worse when we start talking about doing things properly for Remoting based proxies (e.g. EJB3) vs. the existing detached invoker-based ones. LoadBalancePolicy.chooseTarget (FamilyClusterInfo clusterFamily, Invocation routingDecision) is not meaningful in a Remoting based proxy, because the "org.jboss.invocation.Invocation" class is not used, "org.jboss.aop.joinpoint.Invocation" is. So, handling that doubles the number of classes. :(

2.- Should transaction sticky (either implementations or solution mentioned in 1) be the default? IMO, 4.x should maintain existing (default) behavior while transaction sticky should be default in trunk. Thoughts?

+1

[galder.zamarreno]

Good. Take the "don't deploy in AS" thing as just an idea I threw out. That comes from a general feeling I have that unit testing in clustering is inadequate because it's too hard to 1) write complex deployment descriptors that cover all the cases, package them and then deploy them 2) properly check system state when you can only expose that state via JMX or as part of a web or ejb response.

V good point. Testing having to start AS and deploy stuff is quite heavyweight and makes the tests take much longer. I agree with you and so, I'll look into how to resolve this via simpler UTs.

1.- Rather than implementing 4 brand new policies, it might be easier to add a new element to cluster-config XML called transaction-sticky with true/false values and modify the existing load balance policies to act upon the value of that element, what do you think? This hasn't been fully baked, it's just something that I had in mind.

Hmm. I don't want to reject that idea yet, but I'm concerned about pushing more config stuff into both EJB3 and EJB2. Plus, theoretically these policies can be used outside of EJB, so we'd have to find a way to expose the option wherever they are used.

Also, doing that requires a new field in the LBP impls, which changes their serialization. That will break clients using the old versions, so no good in 4.x and something to avoid in 5.

Also means changing the LBP interface to add a setTransactionSticky(boolean) method.

I know where you're coming from though. The proliferation of classes gets worse when we start talking about doing things properly for Remoting based proxies (e.g. EJB3) vs. the existing detached invoker-based ones. LoadBalancePolicy.chooseTarget (FamilyClusterInfo clusterFamily, Invocation routingDecision) is not meaningful in a Remoting based proxy, because the "org.jboss.invocation.Invocation" class is not used, "org.jboss.aop.joinpoint.Invocation" is. So, handling that doubles the number of classes. :(

Serialization change and having to add configuration for EJB3 on top of EJB2 seems two good reasons to create separate load balance policies.

[galder.zamarreno]

Brian,

Coding is in progress at the moment. While starting to code the tests for each new LB policy (ok/failure + different invokers), I have realised that it'd be very easy to use the mock infrastructure created to test standard LBs.

Looking around, haven't seen tests for standard LBs, do you think it's worth adding them? I don't think it'd take much longer to add them.

[brian.stansberry]

Yes, please add for standard LBs. :)

There are some tests of EJB 2 and 3 load balancing but they don't really cover all the LB impls and probably don't cover as many cases as you could.