camel-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christian Schneider <ch...@die-schneider.net>
Subject Re: [HEADS UP] - Adjustments to ExecutorServiceManager on trunk
Date Thu, 18 Aug 2011 11:25:08 GMT
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


Mime
View raw message