I had this conversation in private with Ovidiu, and I wanted to make a public post about
this.
I'm kind of concerned about the fact we are using multiple valves (instead of a single
instance per connection).
The way is implemented now, you have a Valve on every descendant of Connection (Sessions,
Producers, Consumers and Browsers).
When we get a failure, we have to close every single valve before performing a failover,
i.e. The lock is not atomic and you have multiple locks.
The problem is when you place a try..catch(IOException) on the ValveIntercetor to capture
failure exceptions. In cases where you have multiple threads using the same Connection
(multiple sessions/threads on the same Connection). As the lock should be aways atomic
(close every valve for the failed Connection), you can have multiple Threads trying to
close the whole hierarchy of valves.
Lets talk in examples:
- You have one Connection and 50 Threads/50 Sessions. Suppose all these 50 Threads are
communicating at the same time.
- We kill the server, all of the 50 threads are going to catch an IOException at the same
time.
- As all of the 50 threads are going to catch the exception at the same time... All of
them are going to close the valve at the same time. Valve close is a loop on the
hierarchy:
| // from HierarchicalStateSupport
| public void closeChildrensValves() throws Exception
| {
| if (children == null)
| {
| return;
| }
|
| for(Iterator i = children.iterator(); i.hasNext(); )
| {
| HierarchicalState s = (HierarchicalState)i.next();
| ((Valve)s.getDelegate()).closeValve();
| }
| }
|
| public void openChildrensValves() throws Exception
| {
| if (children == null)
| {
| return;
| }
|
| for(Iterator i = children.iterator(); i.hasNext(); )
| {
| HierarchicalState s = (HierarchicalState)i.next();
| ((Valve)s.getDelegate()).openValve();
| }
| }
|
I belive we should use a single Valve instance, the way it was done before.
The only reason for this refactoring was the fact I was using a single instance but using
AOP API to install the Valve on the right place with the proper instantiation. This could
be refactored in another way such as delegating the Valve to an external object on the
ValveAspect (which was going to be renamed to FailureInterceptor).
View the original post :
http://www.jboss.com/index.html?module=bb&op=viewtopic&p=3998012#...
Reply to the post :
http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&a...