Bounding the number of threads created by the MDB container

[ovidiu.feodorov]

Each MDB container deployment that uses the standard JBoss JMS JCA adapter creates a pool of 15 (default number, but configurable) server sessions, and each such server session starts by default at least 2 new threads (the executors of the server side session endpoint and the client side session delegate). That is 30 JBM threads per MDB deployment.

Each MDB deployment can be tuned to reduce its number of server sessions, but a little more elegant solution would be to allow JBM to accept a thread pool for all its primitives to use. This is the spirit of http://jira.jboss.com/jira/browse/JBMESSAGING-1356.

This way, an instance administrator can put a upper bound to the total number of threads used by MDBs within an instance, regardless of an MDB individual configuration.

Generalizing, if one wants to limit the maximum number of threads used by JBM within a VM, should be able to configure all thread-related JBM primitives to use a central thread pool.


I'll take a stab at implementing this.

What branch should I apply my patch on?

[timfox]

I don't really see how a pooling approach will work.

Each session requires an executor so it can ensure that deliveries (etc) for that session are executed on a different thread but in the correct order.

If you got a thread from the pool each time to execute the delivery then you couldn't guarantee that order any more, since you won't guarantee it's the same thread being used.

What might work is pooling executors rather than threads, but then you could get into deadlock situations where one session is waiting on getting an executor but can't complete because the executor pool is full and the system will just hang.

[ovidiu.feodorov]

That's the whole point, you don't need a thousand threads to get order guarantees. This simply doesn't scale, as I've recently seen in a couple of cases in production.

[timfox]

I guess I have misunderstood what you are suggesting then.

If you pool the threads, then how do you guarantee that all deliveries (say) for a particular session are delivered in order. This is the piece of the puzzle I am missing.

[ovidiu.feodorov]

All you need is to maintain the order of the deliveries relative to a session. It doesn't really matter what thread runs a delivery, whether is the same thread or different thread, as long as the relative order is preserved.

"Booking" a thread for each session, while it is a solution that works at a small scale, breaks down when the problem grows in size.

As long as you agree with this, solutions can be found.

[timfox]

We already do something similar in JBM 2.0 (to preserve the relative ordering)

See http://anonsvn.jboss.org/repos/messaging/trunk/src/main/org/jboss/messaging/util/OrderedExecutorFactory.java

The problems of deadlock still remain though.

[ovidiu.feodorov]

OK, let me think about a way to apply this to the 1 branch. As I said, the issue comes from very practical considerations, and it would be good if it gets fixed.

What branch should I use to experiment with? Do you still keep the "release map" up to date?

[timfox]

Ok, you could give this a shot on Branch_Stable.

Take a look at how we used the OrderedExecutorFactory in JBM 2.0.

If you allow each session to maintain one of these child executors, then they could be backed by a single pool like you suggest.

[ovidiu.feodorov]

How do I run a full testsuite? I would like to take a snapshot before applying any change, so I can figure out whether I broke something or not.

[timfox]

How do I run a full testsuite? I would like to take a snapshot before applying any change, so I can figure out whether I broke something or not.

Should just be able to run ant from the tests directory.

[ovidiu.feodorov]

I've checked in a tentative implementation on https://svn.jboss.org/repos/messaging/branches/Branch_Experimental_JBMESSAGING_1356/

The essence of change is that I introduces a new type, SerialExecutor, whose contract guarantees tasks execution in the order in which they were submitted, regardless of how many threads execute those threads in background. The new type comes with several implementations: QueuedSerialExecutor which is essentially your QueueExecutor, a new PooledSerialExecutor, whose instances can share a common pool of threads and some other stubs (DirectSerialExecutor, etc.). Plus a whole bunch of tests.

Everything that previously was a concrete QueuedExecutor now is a generic SerialExecutor, whose implementation is interchangeable, depending on configuration. Your NamedThreadQueueExecutor is now a SerialExecutor too, so it can be used as a possible implementation.

I tried to run the tests (ant clean; ant jar; cd tests; ant) and they hang, so I don't have a "before" snapshot:

[junit] TEST org.jboss.test.messaging.jms.selector.SelectorTest(Remote-bisocket) FAILED

... and it gets stuck here.

Things that still need be done:

- Currently, the shared PooledExecutor is hardcoded into ServerPeer. I need to add a configuration interface that specifies at least the maximum number of threads.
- I only modified the server-side session endpoints to use it. I see no reason not to share the thread pools with the client-side delegates, if they live in the same VM.
- If you decide to merge this branch into the stable on and go with this extension in production, I need to really stress the implementation of PooledSerialExecutor to make sure it will hold in production.

[timfox]

Any reason why you didn't use OrderedExecutorFactory, like we do in JBM 2.0?

It should be a copy and paste job from TRUNK to 1.4..

[ovidiu.feodorov]

The idea was to introduce a new type and a pluggable implementation. As long OrderedExecutorFactory guarantees the contract, could be a run-time configuration.

However, looking at OrderedExecutorFactory, I see:

"public final class OrderedExecutorFactory"

[timfox]

I'm invoking the KISS principle here.

I don't think the class needs to be pluggable. AFAICT (and this is how we do it in JBM 2.0) only the pool size needs to be configurable.

[ovidiu.feodorov]

That is fine. When I said "pluggable" I actually meant "use type instead of implementation" in the code.

Along the lines of "always use List instead of ArrayList, when applicable".