ClosedInterceptor design approach

[timfox]

Hi All-

I've been looking at implementing the ClosedInterceptor functionality for the JMS Facade, and wanted to run my design approach by you to make sure I'm on the right track.

I've had a look at how this was implemented in JBoss 4 messaging, and I believe there is a fair amount in the actually ClosedInterceptor logic that could be re-used :) (In particular the valve logic itself.)

However, I believe the code that maintains the tree of "containers" (JBoss 4 messaging terminology ??), so that close and closing() can be cascaded down the nodes of the tree (i.e. close on a connection causes a session to close that causes a producer to close...), needs to be done somewhat differently.

In JBoss 4, the hierarchy is held in the InvocationHandler classes (Container class) and the parent child relation-ship is set-up when the Container class is constructed). This is fine since in JBoss 4 the client side interceptor stack is created on the client side.

In the new JBoss messaging, the client side interceptor stack is created on the server side and sent back to the client so the approach of maintaining the hierarchy on the server side won't seem to work (and also is more stuff to shift across the network), since each call to createSessionDelegate would end up with it's own copy of it's parent JMSInvocationHandler being sent back.

A solution to this seems to be to maintain the hierarchy on the client side. This can be done by intercepting calls to createSessionDelegate(), createProducer() etc. and adding the child to the parent's invocation handler, so each invocation handler maintains a set of it's child invocation handlers.

Assuming this sounds the right thing to do, then the question arises of which interceptor to use to actually maintain the hierarchy. Choices seem to be:

1. Just use the ClosedInterceptor itself.
2. Create a new client side interceptor
3. FactoryInterceptor?

So far I've assumed 1. but not sure if that's the right approach.

I'd much like to hear what you think here.

Another question I have involves the MessageProducer and MessageConsumer close() method.

Is there any need to do anything on the server side for this method. I.e. is there any server side clean-up to do?

I notice currently the ProducerDelegate does not have a close() method, but not sure if this will be implemented in the future.

If there is nothing to do, this could simplify the ClosedInterceptor since you'd just be traversing from Connection to Session.

So, basically I have, so far, implemented everything as mentioned above, and have knocked some tests together and everything seems to be working ok, so if the approach seems reasonable I should be able to get this finished by tomorrow hopefully.

Sorry for boring you with the long mail ;)

-Tim


Almost forgot... A couple of misc. questions:

Do people at JBoss maintain copyright over their code? If so, I believe I would need to ask permission of the authors if I were to re-use any code from R4.0? I'm not really sure of the procedure here.

Eclipse coding template. Is there an eclipse coding template (or something like that) available for the JBoss coding style?

[ovidiu.feodorov]

If you need to maintain the hierarchy on the client side, I don's see a problem with that. Until now I didn't need it, but close() is a good use case for it.

The easiest way to do is with another interceptor. FactoryInterceptor would be a misnomer in the current situation, since the delegates are not constructed by it, but come from server. Actually, there is a FactoryInterceptor in the Session's interceptor stack, which only creates Messages so far.

The first name that comes to mind is HierarchyInterceptor, but's got to be a better one ...

In the new JBoss messaging, the client side interceptor stack is created on the server side and sent back to the client so the approach of maintaining the hierarchy on the server side won't seem to work (and also is more stuff to shift across the network), since each call to createSessionDelegate would end up with it's own copy of it's parent JMSInvocationHandler being sent back.

I don't exactly understand what are you trying to say here. JMSInvocationHandler being sent back? Why? There is a hierarchy of ServerDelegates on the server. It is not useful for the close logic, since the closing valve must be on the client.

About reusing code: just do it. Mention the original author in the header.
About Eclipse: I don't use it. Maybe Adrian can help you here?

[timfox]

If you need to maintain the hierarchy on the client side, I don's see a problem with that. Until now I didn't need it, but close() is a good use case for it.

The easiest way to do is with another interceptor. FactoryInterceptor would be a misnomer in the current situation, since the delegates are not constructed by it, but come from server. Actually, there is a FactoryInterceptor in the Session's interceptor stack, which only creates Messages so far.

The first name that comes to mind is HierarchyInterceptor, but's got to be a better one ...

Ok, I'll go for HierarchyInterceptor for now. We can always change it later if need be.

In the new JBoss messaging, the client side interceptor stack is created on the server side and sent back to the client so the approach of maintaining the hierarchy on the server side won't seem to work (and also is more stuff to shift across the network), since each call to createSessionDelegate would end up with it's own copy of it's parent JMSInvocationHandler being sent back.

I don't exactly understand what are you trying to say here. JMSInvocationHandler being sent back? Why? There is a hierarchy of ServerDelegates on the server. It is not useful for the close logic, since the closing valve must be on the client.

I'm probably not explaining this very well. It's my understanding that, e.g. in ServerConnectionDelegate.createSession, the object returned is a dynamic proxy that (internally) references an instance of JMSInvocation handler that's been constructed in that method. This instance holds a reference to the interceptor stack and a few other things, and is serialized back to the client.
In JB4 it's this class (called Container in JB4) that maintains the hierarchy.
So... basically I'm just saying in a very long winded way what you said much more concisely yourself :) I.e. we need to maintain the state on the client side.

About reusing code: just do it. Mention the original author in the header.
About Eclipse: I don't use it. Maybe Adrian can help you here?

sure, will do.

One more quick question: Do I need to worry about server side close for MessageProducer and MessageConsumer? I.e is there any that needs to be done on close() for these objects in the server side delegates?

Thanks

Tim

[ovidiu.feodorov]

It's my understanding that, e.g. in ServerConnectionDelegate.createSession, the object returned is a dynamic proxy that (internally) references an instance of JMSInvocation handler that's been constructed in that method. This instance holds a reference to the interceptor stack and a few other things, and is serialized back to the client.
In JB4 it's this class (called Container in JB4) that maintains the hierarchy.

That is correct. It's called JMSInvocationHandler because this is what it implements: java.lang.reflect.InvocationHandler. However, it is a container, since it "contains" the client-side interceptor stack. Since you mentioned this name several time, I am wondering whether we should rename it "JMSClientContainer". Anyway, it's a simple refactoring. If at least one more person starts refering to it as "container", I probably will :).

Do I need to worry about server side close for MessageProducer and MessageConsumer? I.e is there any that needs to be done on close() for these objects in the server side delegates?

ServerProducerDelegate itself does not need any clean-up on close() at this point. The Messaging Core destinations that receive messages from such endpoints do not care (do not have a reference to it). The only thing that is needed is to remove the producer delegate from the session's map.

However, situation is different for Consumers. In a way, they are the equivalent of a ServerProducerDelegate, on the receiving side. They are not called ServerConsumerDelegate because they don't implement ConsumerDelegate, there is no need for that, all "ConsumerDelegate-ish" behavior is taking care of by interceptors: see ReceiverInterceptor on client-side and AsynchronousDeliveryInterceptor on the server-side. Here, there is need for clean-up: when a Consumer is closed, it must be de-registered from Messaging Core destination it is connected to.

So far, I see ServerSessionDelegate doing this, in a method symmetrical to createConsumer(), that is called by a server-side CloseInterceptor. The current design is a little bit asymetrical and not exactly ellegant, but I would wait a little bit more to see if other needs arise, then maybe refactor.