Hi Claus,
I just saw that you also changed the ThreadPoolFactory interface to not
use the profiles and instead have several methods again. This completely
defeats the purpose of having a small factory interface.
I will revert that back.
I worked on these changes quite a long time so the fact that you simply
changed things back after we discussed on them and agreed on the changes
makes me a bit sad. It also causes me a lot of unplanned work now. I
agree with you on some of the things you mentioned.
Like for example that it makes sense to offer an API on the
ExecutorServiceManager that people are familiar with. So I think using
the almost same API as in java is a good thing. I also like the fact
that the change on the components is now really small after your change
and also that there is a completely compatible ExecutorServiceStrategy
as a fallback. That really makes sense for all external components.
The problem is that with your changes you also rolled back good things I
did that I now have to spend a lot of time bringing in again.
Christian
Am 12.08.2011 17:29, schrieb Claus Ibsen:
Hi
Recently the ExecutorServiceStrategy had a bigger refactor
(https://issues.apache.org/jira/browse/CAMEL-4244)
Such as renaming the class to a better name ExecutorServiceManager.
Also introducing a ThreadPoolFactory, ThreadFactory among others. The
ThreadPoolFactory is a great addition, which makes it easier for 3rd
party to just implement a custom factory, and then reply on the
default manager.
However the refactoring introduced a few issues such as configuring
thread pools on EIPs was flawed. Well it also revealed we were short
of an unit test to cover this. So I created a ticket, and committed a
test to the 2.8 branch.
https://issues.apache.org/jira/browse/CAMEL-4328
The refactoring did also make backwards compatibility an issue, so we
will restore that but mark the old API as @deprecated.
The changes are summarized as follows
- ThreadPoolFactory is the light weight and easier for SPI
implementators to create a custom thread pool provider, such as JEE
servers with a WorkManager API. This API has much fewer methods and
they have been adjusted to be 100% JDK API (There is no Camel API in
there, such as ThreadPoolProfile).
- ExecutorServiceManager is the full fledged SPI in case you need 100%
control. But it has Camel APIs such as ThreadPoolProfiles and a number
of more methods. Its is encouraged to just implement a custom
ThreadPoolFactory instead. And keep using the
DefaultExecutorServiceManager.
- ThreadPoolBuilder is adjusted to create a thread pool on the build
method. This is how end users would expect. A builder to create what
its name implies. This also removes entanglement of ThreadPool and
ThreadPoolProfile. The latter is Camel specific and is only part of
the ExecutorServiceManager which manges a list of profiles. To help
uniform and make it easy to adjust thread pool settings at a higher
level. So ThreadPoolProfiles should only be party of the manager.
- EIPs configured using an explicit thread pool by an
executorServiceRef, will now act as expected. If the reference could
not be found, an exception is thrown, stating the reference is not
found
- Will add the ExecutorServiceStrategy SPI to have Camel 2.9 being
backwards compatible, but mark it as @deprecated. This gives end users
some time to adjust their custom Camel components, and source code if
needed.
- Naming of the methods will use the naming convention used by the
JDK. xxxThreadPool for a pool of threads, xxxExecutor if its a single
threaded (eg used for background tasks)
- Made the API of ExecutorServiceManager more similar to the old
ExecutorServiceStrategy so migrating is a breeze, usually just change
the getExecutorServiceStrategy() to getExecutorServiceManager() and
you are settled.
I ran a complete unit test on my local laptop before commiting the changes.
There is a few TODO in the code, which I will work on as well, so
expect a few more commits. The TODO is minor/cosmetic changes I have
spotted, that can be improved.
--
--
Christian Schneider
http://www.liquid-reality.de
Open Source Architect
Talend Application Integration Division http://www.talend.com
