Client-side failover valve

[ovidiu.feodorov]

Clebert,

I would like to understand the logic behind the way you implemented the Valve.

Here are some questions:

1. I noticed that ValveAspect extends HAAspect but also implements Interceptor. Why do you need to extend HAAspect? Is the Valve construct an aspect (is it intended to encapsulate multiple advices) or it is essentially an interceptor (a single advice)?

2. I don't see a ValveAspect in aop-messaging-client.xml, which means you're using it in a non-standard way. You've got to decide whether it's an aspect/interceptor, and then you initialize it consistently with the rest of the stack, or it's something else, but in that case don't call it "aspect", call it something else.

3. Please write down (no more than three-four paragraphs) the way your client-side valve is intended to work.

Some general observations:

1. Please use the commonly-accepted coding style where you guard various sections of your class file with labels (// Constants ---, // Constructors ---, etc)
2. Make sure that after you check in the $Id$ tag is correctly updated. I noticed that in your case it isn't. It has something to do with your SVN client set-up, it happened to Tim as well, and he fixed it.

[clebert.suconic]

1. The Valve is an interceptor.
I first wanted to implement it as an aspect, but then I had a problem of enabling/disabling the valve when we are calling a ClusteredCF and a non ClusteredCF. Another point also is... the Valve needs to be instantiated per connection, i.e. All objects created after a connection will share the same instance.
We could use ConnectionState to store the data. That was one of the questions I had on another thread but we left the discussion for later.

2. As I said on 1. ValveAspect is an interceptor. Maybe we should rename it.

I have kept is as an extension, because HAAspect is per-instance and exists per connectionFactory, while ValveAspect exists per Connection and any object created after Connection will share the same instance. The valve works more like a proxy.

For example, a Connection creates a Session...

Both Connection and Session will share the same ValveInterceptor.

The Valve also needs some features existent on HAAspect that's why we have the inheritance. Maybe we could merge ValveAspect into HAAspect (even I wouldn't agree, since the Valve is more like an interceptor).

Maybe we could extract some functionality out of HAAspect and have a common API to be called from HAAspect or ValveInterceptor. (This actually would be one good option)

3. As I said on 1 and 2... The Valve is a proxy, holding any calls while failover is happening on any thread or object associated with a Connection. It's using readWriteLock to perform such feature.

Regarding the observations I will see what's going on with 2 and will change 1.