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, 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).

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.







Reply via email to