1 2 Previous Next 19 Replies Latest reply on Jan 12, 2009 8:25 PM by David Lloyd

    Declaration and deployment of threads and thread pools in th

    David Lloyd Master

      I've developed an experimental POJO-based deployer for defining thread groups, thread factories, and thread pools for the MC. The source is in my sandbox area at https://svn.jboss.org/repos/sandbox/david.lloyd/jboss-threads/trunk.

      Basically it lets you write a jboss-threads.xml file which contains declarations like thread-group, thread-factory, thread-pool-executor, etc. You can configure things like the daemon status, priority, set parent groups, pool sizes (scaled linearly or by CPU count, or both), the queue type and rejection policies for the executor, etc. These get created as named beans which can be injected using regular POJO deployment stuff.

      Since it's just an experiment I didn't go through the mbean stuff to see if every little function is replicated but this should be enough to tell whether it's a good idea or a dumb idea anyway. At the very least it should make thread pool configuration a lot more uniform, and hopefully simpler for the end-user...

      I don't have a build script yet, but you can use your IDE of choice to compile the two modules into a single JAR and deploy it as a deployer. The XSD for the jboss-threads.xml files is in there too.

        • 1. Re: Declaration and deployment of threads and thread pools i
          Ales Justin Master

          Nice!

          But I guess you should provide some examples.
          e.g. few xml snippets, explaining what does what

          ps: I'll definitely mention this in my MC's VDF article ;-)

          • 2. Re: Declaration and deployment of threads and thread pools i
            Adrian Brock Master

            Looks good (but I only gave it a quick scan).
            Obviously it needs some tests to avoid regressions. :-)

            A few issues:

            * THREAD PRIORITY

            Why are you using float for thread priority?
            A thread can only take an integer priority between
            1 (Thread.MIN_PRIORiTY) and 10 (Thread.MAX_PRIORITY)

            Even if it was a decimal, BigDecimal would be better than
            float because of the rounding errors.
            See for example the xsd:decimal bindings here:
            http://www.xyzws.com/scdjws/studyguide/jaxb_samples2.0.html
            or look at the jaxb spec for recommended bindings.

            * MBEANS

            You can inject into MBeans just like you can pojos

            * JBoss's BasicThreadPool and Task abstraction

            We already have an abstraction in the common-core project
            that adds to the basic thread pool executor abstraction.
            (Mostly by wrapping it).

            This adds things like lifecycle and error handling callbacks via the task abstraction,
            and management exposure via an MBean.

            But there are also some different policies such as MinThreadPoolExecutor
            so you can avoid the known race condition if you implement a WAIT policy
            when the thread pool is full (something that is not part of java.util.concurrent
            because of the race condition)

            • 3. Re: Declaration and deployment of threads and thread pools i
              David Lloyd Master

               

              "adrian@jboss.org" wrote:
              Why are you using float for thread priority?
              A thread can only take an integer priority between
              1 (Thread.MIN_PRIORiTY) and 10 (Thread.MAX_PRIORITY)


              I got the notion in my head that MAX_PRIORITY might not be 10, so my thought was to take a float between 1.0 and 10.0 inclusive and scale it between MIN and MAX. Though I guess I overthought it, since if someone decided to arbitrarily change the priority scale, there would probably be compatibility issues, so that would seem to be a rather unlikely scenario.

              "adrian@jboss.org" wrote:
              Even if it was a decimal, BigDecimal would be better than float because of the rounding errors.


              I doubt that someone would care if their thread at priority 10.1 was really running at 10.099999999999999644728632119950, since it would very likely end up being the same priority value anyway :-)

              In any case, making this a float was probably a dumb idea - I think I'll just make it an int.

              "adrian@jboss.org" wrote:
              But there are also some different policies such as MinThreadPoolExecutor so you can avoid the known race condition if you implement a WAIT policy when the thread pool is full (something that is not part of java.util.concurrent because of the race condition)


              I saw the code and the comment to that effect, but at first glance it wasn't clear what the race condition was, or how the fix worked. I'll have to look at that one more time...

              • 4. Re: Declaration and deployment of threads and thread pools i
                Adrian Brock Master

                 

                "david.lloyd@jboss.com" wrote:

                "adrian@jboss.org" wrote:
                But there are also some different policies such as MinThreadPoolExecutor so you can avoid the known race condition if you implement a WAIT policy when the thread pool is full (something that is not part of java.util.concurrent because of the race condition)


                I saw the code and the comment to that effect, but at first glance it wasn't clear what the race condition was, or how the fix worked. I'll have to look at that one more time...


                The issue comes with small thread pools. e.g. a singleton MDB configuration
                has a thread pool size of 1

                Depending upon the reaping policy of unused threads it is possible for a race
                condition to go something like this (I'll assume no queue although it makes
                no real difference in practice since the race would still happen after dequeuing):

                1) Thread 1: Submit work to the pool
                2) Thread 2: Execute the work
                3) Thread 2: Complete work
                4) Thread 1: Submit some work - pool is busy so wait
                5) Thread 2: Return thread to the pool
                6) // CPU starvation for longer the idle time of threads
                7) Thread 1: Stop waiting and check we have enough threads (we do)
                8) Thread 2: I've been idle too long so end

                So you end up with thread 1 thinking there are enough threads
                to execute the work, but Thread 2 has stopped an instant after the check.

                The work will never be executed unless some other thread also tries
                to execute some work on the pool (meaning the number of threads gets rechecked).
                For a singleton MDB with a read-ahead of messages of size 1 this never occurs
                so message delivery to the MDB stalls.

                The fix/work around we have is to stop step (8) from occuring.
                i.e. you never allow the thread pool to go below a minimum of one thread
                which avoids the race condition - there is always at least one thread
                available to execute the work.

                The real fix would be to make the check in step (7) more atomic
                i.e. move the thread pool size check to later when it tries to execute the work
                but that's not easy to do if you look at the code. :-)

                • 5. Re: Declaration and deployment of threads and thread pools i
                  Adrian Brock Master

                   

                  "adrian@jboss.org" wrote:

                  4) Thread 1: Submit some work - pool is busy so wait


                  NOTE: Without this requirement in step 4 which isn't a policy available in java.util.concurrent
                  you can't have the race condition. The submission of the work will either
                  throw an error immediately, after a timeout or run the work on Thread 1

                  But for an MDB singleton none of those options are what we want. ;-)

                  • 6. Re: Declaration and deployment of threads and thread pools i
                    David Lloyd Master

                     

                    "alesj" wrote:
                    Nice!

                    But I guess you should provide some examples.
                    e.g. few xml snippets, explaining what does what


                    OK, it's pretty straightforward. Basically if you have a pojo component that requires an executor, you can build it declaratively via XML (imagine a security manager scenario - by building your executor this way, your code needs no special thread-control permissions).

                    So to create a thread pool for my application which keeps from 2 + 1.2n to 4 + 2n threads (where n = # of CPUs), with a bounded queue and a blocking reject-policy, I would just add a jboss-threads.xml file like this:

                    <threads xmlns="urn:jboss:threads:1.0">
                     <thread-group name="MyThreadGroup" group-name="my-app" daemon="false"/>
                     <thread-factory name="MyThreadFactory" group="MyThreadGroup"/>
                     <thread-pool-executor name="MyExecutor" thread-factory="MyThreadFactory">
                     <core-pool-size count="2" per-cpu="1.2"/>
                     <max-pool-size count="4" per-cpu="2"/>
                     <bounded-queue size="50"/>
                     <reject-policy name="block"/>
                     </thread-pool-executor>
                    </threads>


                    This makes an injectable bean called "MyExecutor" which I can add to my application using the normal mechanisms.

                    Alternately I could "borrow" a thread pool executor declared elsewhere, with a hook so that it changes the thread's name when it's running one of my tasks:

                    <threads>
                     <notating-executor name="MyExecutor" parent="SomeOtherExecutor" note="Running my task"/>
                    </threads>


                    There are three different queue types available (bounded, unbounded, and direct), a variety of reject-policies, and pluggable interrupt and exception handlers.

                    One thing I did not (yet) account for is a facility to propagate various thread context information to tasks (things like a context classloader, security context, transaction context, etc). I imagine some kind of pluggable system where a context type can be registered with code that knows how to get context from one thread and put context to a new thread. Then it's just a question of adding a plugin for each context type, and new services which keep a thread context could just be registered.

                    • 7. Re: Declaration and deployment of threads and thread pools i
                      David Lloyd Master

                       

                      "adrian@jboss.org" wrote:
                      The fix/work around we have is to stop step (8) from occuring.i.e. you never allow the thread pool to go below a minimum of one thread which avoids the race condition - there is always at least one thread available to execute the work.


                      Ah, I understand. I intuited that this might be the case so I made the core thread pool size be implicitly at least 1, but I agree that this is undesirable overall. It would be nice if thread pools which are never used would never produce any threads. For my "blocking" policy, I'm not actually using a special rejection handler; instead I wrap the BlockingQueue with one that blocks on "offer". I still have to do some testing to see if that's a good approach, however I suspect it will not suffice to avoid this race condition even if it is a usable approach overall.

                      • 8. Re: Declaration and deployment of threads and thread pools i
                        David Lloyd Master

                        In regards to the empty pool vs. blocking policy race condition...

                        I think my solution solves the deadlock. Since the blocking occurs on the offer() rather than reject(), ThreadPoolExecutor has code in place to handle this problem (they need to handle it anyway - even if offer() works in the normal nonblocking fashion, the condition may still occur where the task is added to the queue but no threads remain to handle it). Namely, after successfully adding an element to the queue, it checks to verify that there is at least one thread around to handle the task, and if not, it starts one.

                        I'm in the process now of figuring out a test case that artificially creates this scenario...

                        • 9. Re: Declaration and deployment of threads and thread pools i
                          David Lloyd Master

                          ...and I've created one. It makes sure that the queue submit unblocks after the worker thread dies. The executor used in the test is basically a regular ThreadPoolExecutor with the same queue wrapper, except I wrap the wrapped queue again to add addtional blocking so that the queue unblocks right when the race would occur - when the worker thread from the first task exits leaving zero threads in the pool.

                          http://anonsvn.jboss.org/repos/sandbox/david.lloyd/jboss-threads/trunk/main/src/test/java/org/jboss/threads/ThreadPoolTestCase.java

                          • 10. Re: Declaration and deployment of threads and thread pools i
                            Adrian Brock Master

                             

                            "david.lloyd@jboss.com" wrote:
                            ...and I've created one. It makes sure that the queue submit unblocks after the worker thread dies. The executor used in the test is basically a regular ThreadPoolExecutor with the same queue wrapper, except I wrap the wrapped queue again to add addtional blocking so that the queue unblocks right when the race would occur - when the worker thread from the first task exits leaving zero threads in the pool.

                            http://anonsvn.jboss.org/repos/sandbox/david.lloyd/jboss-threads/trunk/main/src/test/java/org/jboss/threads/ThreadPoolTestCase.java


                            You can't test a race condition by blocking.
                            Its a race condition because the thing is not threadsafe.
                            At best, all you'll do is cause a deadlock in your test. ;-)

                            The only way to test for a race condition is to stress test something that is
                            likely to reproduce the problem (in this case it is very difficult because
                            it requires starving a thread of cpu longer than its idle time :-)

                            On your solution, I don't think it resolves the real issue?

                            The issue is that the
                            * offer to the queue
                            * end previous runnable
                            * add a thread
                            are not synchronized with each other.

                            i.e. there are gaps between the addWorker() check and the offer to the queue
                            where the thread that addWorker() thinks exists
                            gets ended due to idleness leaving the Runnable in limbo.
                            Only a new call to addWorker() would recheck the active threads and spot
                            that there's no threads to execute the runnable.

                            So another workaround (besides the minimum of 1 threads) would be to introduce
                            a periodic redo of the addWorker() check.

                            Or put another way, whether you block in the queue.offer() or the rejection()
                            won't make any real difference as you can see from the openjdk execute() code
                            they are essentially in the same place
                             if (isRunning(c) && workQueue.offer(command)) {
                             int recheck = ctl.get();
                             if (! isRunning(recheck) && remove(command))
                             reject(command);
                             else if (workerCountOf(recheck) == 0)
                             addWorker(null, false);
                             }
                             else if (!addWorker(command, false))
                             reject(command);
                            


                            • 11. Re: Declaration and deployment of threads and thread pools i
                              David Lloyd Master

                              Hmm. There's differences between the openjdk version and the JDK6 version...

                              I need to think about this some more. Maybe ThreadPoolExecutor is not something we should be using.

                              • 12. Re: Declaration and deployment of threads and thread pools i
                                David Lloyd Master

                                After spending far too long on this problem, the solution I have come up with (this time) is to have an alternate executor implementation which is used when the configuration given is known not to work with TPE. In this case, it is chosen whenever there is a blocking policy in place, but it could be made more (or less) specific if desired.

                                The alternate implementation is designed to be correct and simple ("SimpleQueueExecutor"); thus it uses a Queue (rather than a BlockingQueue) to hold tasks and simply uses a Lock and Conditions to synchronize and block as needed, in order to allow for atomic "create-thread-or-queue" and "dequeue-or-destroy-thread" operations. It is designed to hold the lock for as small a time as possible (generally just long enough to do a couple field accesses, comparisons, and maybe a queue insert/remove) without making the code too complex. As such it is considerably simpler than ThreadPoolExecutor (less than 600 lines versus approx. 1800), though I won't place any bets as to comparative performance until I can figure out a way to get a somewhat reliable benchmark result. I'll consider it a runaway success if I achieve "approximately similar". In support of this class, there's also a simple bounded, array-backed queue class ("ArrayQueue") since there is (amazingly) no such thing in the JDK.

                                As for the common-core stuff - I think that (for the most part) the various extensions can work with this pojo-centric design, but maybe not exactly as-is (since there is some tight coupling with ThreadPoolExecutor in a few places that I see from my initial read through it all). For now I think I'll just postpone the issue - at least until I have fulfilled my immediate obligations.

                                Anyway, here's links to the more interesting points:

                                http://anonsvn.jboss.org/repos/sandbox/david.lloyd/jboss-threads/trunk/main/src/main/java/org/jboss/threads/SimpleQueueExecutor.java

                                http://anonsvn.jboss.org/repos/sandbox/david.lloyd/jboss-threads/trunk/jbossmc/src/main/resources/schema/jboss-threads_1_0.xsd

                                • 13. Re: Declaration and deployment of threads and thread pools i
                                  David Lloyd Master

                                  So, two questions. First: Is this something that can/should be included with JBossMC? Second: If so, what are the criteria and what is the procedure for inclusion?

                                  • 14. Re: Declaration and deployment of threads and thread pools i
                                    Ales Justin Master

                                     

                                    "david.lloyd@jboss.com" wrote:
                                    Is this something that can/should be included with JBossMC?

                                    I see you already have a split - common vs. MC.
                                    I would put the common into commons :-),
                                    e.g. common-threads:
                                    - http://anonsvn.jboss.org/repos/common/threads

                                    And the MC part into deployers/deployers-impl.
                                    (similar place where I put my -scanning.xml and -dependency.xml metadata)

                                    "david.lloyd@jboss.com" wrote:
                                    If so, what are the criteria and what is the procedure for inclusion?

                                    The criteria is usage/dependency, e.g. vfs based stuff goes to deployers-vfs.
                                    Procedure is full mavenization (we already have this) + complete test coverage (parsing + actual usage).



                                    1 2 Previous Next