Sorry for the late reply, I’ve been very busy this week. Please see my comments
inline!
The steps you have listed are correct and please note that the ServerManager
has been now instrumented with the following sequence of life cycle methods and
I have realized that at least for now we do not need this same method structure
on the ManagedLifecycle interface.
ServerManager#init()
ServerManager#start()
ServerManager#stop()
ServerManager#destroy()
If you look for the state transition it is as shown in this diagram.
Thanks for creating this diagram which illustrates all the states and their
particular intend. Maybe this can be included in some developer documentation?
1) Who shall call destroy() on ServerManager? (Only the shutdown
hook defined in SynapseServer after calling stop()?) Or was it originally meant
that destroy is called instead of stop()?
Well stop and destroy are two operations, once you stop you may again start the
server without initializing the server, where as if you call destroy the server
has to be reinitialized to start. In contrast if you call destroy while the
state is being ServerState.STARTED we first stop the server and then destroy it.
Yes, the state diagram already explained this. So calling just destroy() in the
shutdown hook would have the same effect as destroy() checks the current state
first and then calls the appropriate methods to do the proper transition(s). In
case the server is started, it first calls stop() and then calls doDestroy().
Maybe this was a bit confusing me.
On the other hand I asked myself, why SynapseStartupServlet calls
ServerManager.stop() and not ServerManager.destroy() in its destroy() method.
If I understand it correctly this Servlet is only used for the deployment
option where Synapse runs inside a web container as a web application (WAR).
For this case Synapse is initialized and destroyed via this servlet. But
shouldn’t the server not only but stopped but rather completely destroyed? Who
else should take care of calling ServerManager.destroy()?
2) Why is destroy public? Why does stop() not call doDestroy()
directly? Answer to 1) may already answer this.
I think the answer above explains why I did it like this. Even though from the
code that we have right now it seem like we could merge these two, in order to
properly manage the states and keeping the room for any extensibility on the
shutdown process, I implemented it like this. If you look at the shutdown
process now, it is the exact opposite of the start order which has to be the
case.
Well not the exact opposite order as discussed later… ;-)
3) What is the relation of the task_scheduler and
synapse.startup.taskscheduler (TaskConstants/SynapseConstants)? So far I had no
contact with tasks at all. I’m only a bit confused about to places where some
schedulers are going to be destroyed: Axis2SynapseController.cleanupDefault()
and SynapseConfiguration.destroy()
Good point, there seems to be some further improvements to the task scheduling
logic... the references that you pointed does the same and we should get rid of
one. I think it has to be the cleanupDefaults keeping the destroying process in
the SynapseConfig#destroy
+1
Regarding the shutdown order I’m still not quite sure whether it is
entirely correct. The position of start and stop of listeners seems to be
correct, though. I’m wondering why SynapseConfiguration.destroy() is called
rather late. I would have expected it to be called even before
Axis2SynapseController.destroySynapseEnvironment(), but I have had no time to
think about the reasoning.
We could change the order of these two.
+1
Maybe you concentrated on the startup and did only some changes to the
shutdown process. I will be able to take more than just a few minutes at the
weekend. I just wanted to give a quick feedback on your efforts which are
highly appreciated.
Thanks, I think apart from the above swap it is OK, I highly
concentrated on the shutdown order as well, and even had to get the Axis2
Listener Manager fixed to support this scenario.
Yes, I have noticed the Axis2 changes and updated it locally at the
time the changes were not available. Thanks for keeping us updated about your
changes. This way it is much easier to follow changes.
Thanks,
Eric
