On Mon, Jul 26, 2010 at 1:27 AM, William Tam <email.w...@gmail.com> wrote:
>
>
> On 07/25/2010 01:56 PM, Claus Ibsen wrote:
>>
>> On Sun, Jul 25, 2010 at 6:18 PM, William Tam<email.w...@gmail.com> wrote:
>>
>>>
>>> Hi Claus,
>>>
>>> Thanks for your response.
>>>
>>> * A ProcessorEndpoint is a hybrid between a Processor and a Endpoint.
>>> Therefore, SendProcessor does have a child if the destination is a
>>> ProcessorEndpoint. Please take a look at
>>> RouteService.doGetChildServices().
>>> You will see why Navigate can allow services associated with the
>>> ProcessEndpoint to be restarted.
>>>
>>>
>>
>> No it does not have a child. It sends a message to an endpoint, period.
>>
>> The processor endpoint is just an endpoint which uses an processor
>> where end users implement the logic what should happen when the
>> message is being sent to it.
>>
>> > From the route perspective the message is sent to an endpoint, no
>> matter what. How and where that endpoint is, should not be part of
>> navigate. Navigate is to be able to traverse the route graph deployed
>> in the camel context. For example to be used by tooling or the likes.
>>
>
> OK, I don't really care whether it has a child or not (whatever that means).
> RouteService.doGetChildServices() is traversing the children using the
> Navigate interface in order to start all of their services. It is wrong in
> RouteService method that starts all the children services by using Navigate
> interface then? This is the only way the services associated with the
> ProcessEndpoint can be restarted. Or, you are suggesting ProcessEndpoint
> should not implement Service since there is no guarantee the service will be
> restarted by design?
>
Yes I agree that ProcessorEndpoint should be a Service so it has the
start/stop methods, you can leverage.
In fact the Endpoint should be a Service so all endpoints is being
started/stopped when you restart CamelContext.
I am working on adding this right now.
>>
>>
>>
>>
>>>
>>> * Yes, when I say 'type converter", I mean the DefaultTypeConverter.
>>>
>>> * If I am not mistaken, ShutdownCompleteAllTasksTest is one example that
>>> the
>>> DefaultTypeConverter was accessed after the DefaultTypeConverter has been
>>> shutdown.
>>>
>>> * I think you are against lazy loading of DefaultTypeConverter. I am
>>> fine
>>> with that. Then, we should really get rid of the lazy loading (in
>>> DefaultCamelContext.getTypeConverter). That is really what the problem
>>> is
>>> because since it is a service, it gets stopped but the lazy loading does
>>> NOT
>>> re-start it. (Maybe you just fixed it. I haven't got a chance to try
>>> it.)
>>> And believe me, having both (a Service and lazy load and start) is
>>> troublesome.
>>>
>>>
>>
>> No the problem is that James S. named the method
>> forceInitLazyService() or something (the name is
>> forceLazyInitialization)
>> The type converter is started when CamelContext starts. And in essence
>> its not lazy. The getter appears to be lazy because you can plugin a
>> custom type converter registry and therefore the default instance is
>> created only if no custom has been assigned.
>>
>
> I (or anyone) can tell that DefaultContextContent.getTypeConverter is
> definitely doing lazy initialization. See it for yourself. :-) Yes, I know
> getTypeConverter() is called when the CamelContext is started but this is
> not the only place getTypeConverter() is called. So, the first time it is
> called, the type converter is created. In some cases (although mainly unit
> tests), getTypeConverter() is called without CamelContext started is called.
>
> public TypeConverter getTypeConverter() {
> if (typeConverter == null) {
> synchronized (this) {
> // we can synchronize on this as there is only one instance
> // of the camel context (its the container)
> typeConverter = createTypeConverter();
> try {
> addService(typeConverter);
> } catch (Exception e) {
> throw ObjectHelper.wrapRuntimeCamelException(e);
> }
> }
> }
> return typeConverter;
> }
>
> You can also see addService() (which starts typeConverter) was only called
> when a typeConverter was created by createTypeConverter(). Therefore, when
> you restart the CamelContext, addService() is not called and therefore
> typeConverter is not started. My point earlier was that combining lazy
> initialization and addService() is troublesome. (I have not yet read your
> latest fix though).
>
This has been fixed. I commented this on the JIRA ticket
Claus Ibsen added a comment - 25/Jul/10 03:28 AM
trunk: 978994.
I have fixed the issue in CamelContext to re-initialize type converter
registry on restart
>> The fix will no ensure CamelContext will start the type converter if
>> CamelContext is started again.
>> I checked that it worked using the TimerRestartTest unit test.
>>
>>
>>
>>>
>>> * IMO, If DefaultTypeConverter is a Service, it should not be usable
>>> (throw
>>> an error something) in the "stopped/not started" state when someone tries
>>> to
>>> use it. Accessing a stopped DefaultTypeConverter is bad to say the
>>> least.
>>> You don't get an error, you just get different/wrong result.
>>>
>>>
>>
>> Well you often use a producer/consumer and access type converters from
>> those. The producer/consumer should have been stopped before the type
>> converter (as its one of the last to be stopped). So your logic in the
>> producer/consumer should have caught that we have stopped.
>>
>
> At least, the "system utils" stuffs like type converter should be the last
> to be stopped. Just adding to servciesToStop List is probably not ideal.
>>
>> And there are zillion ways you can access stuff in Camel and we can't
>> be police men and add isStarted() checks everywhere.
>>
>
> Ideally, type converter should not be stopped (i.e. not a service). So,
> callers can guarantee getting consistent results.
>>
>> You just hit a issue with restarting an application which is really
>> hard to do reliable.
>> The best way is to re-create CamelContext to ensure all can be started
>> nicely again.
>>
>>
>
> The CamelContext should be able to start, stop and re-start robustly. I
> don't think we far from there but these issues should be addressed correctly
> if we want to get there.
>>
>>
>>>
>>> Cheers,
>>> William
>>>
>>>
>>> On 07/25/2010 02:46 AM, Claus Ibsen wrote:
>>>
>>>>
>>>> On Sun, Jul 25, 2010 at 5:55 AM, William Tam<email.w...@gmail.com>
>>>> wrote:
>>>>
>>>>
>>>>>
>>>>> I observed some issue regarding restarting Camel Context that I want to
>>>>> discuss. The detail is in the CAMEL-2991.
>>>>>
>>>>> * First off, any objection if SendProcessor implements Navigate? It
>>>>> allows
>>>>> services associated with the destination endpoint to be restarted.
>>>>>
>>>>>
>>>>>
>>>>
>>>> Why does navigate allow services associated with the destination to be
>>>> restarted? Can you be more clear?
>>>>
>>>> The idea with navigate is for navigating the process graph.
>>>> For example a Content Based Router, where you can navigate the
>>>> when/otherwise processors etc.
>>>> A SendProcessor doesn't have any "children" to navigate, and therefore
>>>> should NOT be navigate.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>>
>>>>> * There is a problem with type converter not getting restarted because
>>>>> the
>>>>> lazy instantiation won't start the services on the type converter
>>>>> unless
>>>>> the
>>>>> type converter is newly created. I notice (in a lot of unit tests)
>>>>> that
>>>>> it
>>>>> is OK to use the type converter even if the CamelContext (its
>>>>> "container")
>>>>> is stopped or has never been started. If we want this behavior, then
>>>>> it
>>>>> does not seem to make sense for type converter to be a service. Type
>>>>> converter could be used anytime including during the shutdown cycling.
>>>>> Since type converter has been added to the "servicesToClose" list, the
>>>>> type
>>>>> converter could be stopped (unpredictably) when it is needed during the
>>>>> shutdown cycle. IMO, type converter probably should not be a service.
>>>>>
>>>>>
>>>>>
>>>>
>>>> I assume you mean the DefaultTypeConverter ?
>>>>
>>>> The DefaultTypeConverter must be a Service so CamelContext can
>>>> start/stop
>>>> it.
>>>> It's important that it's the CamelContext that starts it on startup so
>>>> all the type converters can be loaded up-front.
>>>> Then there is no lazy loading.
>>>>
>>>> The problem with a lazy loading would be at runtime you can end up
>>>> with concurrent access to the type converter registry
>>>> and then multiple threads need to load the type converters, which
>>>> enforces locking/synchronization. And you don't want this overhead
>>>> at runtime. Hence the converters are loaded on startup.
>>>> And to do this the DefaultTypeConverter MUST be a service so we have
>>>> the start/stop methods.
>>>>
>>>> There is no strict check in several places of the code to check that a
>>>> "service" is started before its being used.
>>>> And therefore you can access code which has been stopped.
>>>>
>>>> Can you give an example of an unit test, which uses a type converter
>>>> after CamelContext has been stopped?
>>>>
>>>>
>>>> And when you say type converter you have to be more precise. There is
>>>> a TypeConverterRegistry and then there are the 150+ individual type
>>>> converters.
>>>> The former is the DefaultTypeConverter which loads all the type
>>>> converters. The latter is most often not a service because they are
>>>> @Converter scanned.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>>
>
--
Claus Ibsen
Apache Camel Committer
Author of Camel in Action: http://www.manning.com/ibsen/
Open Source Integration: http://fusesource.com
Blog: http://davsclaus.blogspot.com/
Twitter: http://twitter.com/davsclaus
