Makes perfectly sense Ruwan, and I did think about it as a seconds step as well! Just wanted to mention it, as from a user’s perspective the same problems which may arise at startup can also arise at shutdown. And once something is fresh in memory, those changes are easier to perform. Thanks for taking the time for improving this.
Once you are done, I’m of course willing to do a review. Thanks, Eric ________________________________ From: Ruwan Linton [mailto:[email protected]] Sent: Thursday, April 02, 2009 2:51 PM To: [email protected] Subject: Re: startup order - correct place to start transport listeners Eric, I agree with the comments and I will be looking into the start order first to address your issue, and then I will change the stop process in a way that it does exactly the opposite. If we change it now and had to change it after fixing the start order that is going to be a double work for the stop process. Thanks, Ruwan On Thu, Apr 2, 2009 at 2:17 PM, Hubert, Eric <[email protected]> wrote: Hi Ruwan, thanks for taking the time to review the startup/shutdown logic implemented. In terms of structure and readability I also widely liked the changes. I have only those real world usage’s concerns. So if you are already at it could you please also look at the shutdown process! In most situations the correct shutdown order is exactly the opposite of the startup order. And honestly, this is what I also would expect here. Specifically please have a look at ServerManager.doStart() versus ServerManager.doStop()! Start: Create Synapse Configuration Create Synapse Environment Stop: Destroy Synapse Configuration Destroy Synapse Environment Destroy <-- only here listeners will be stopped (in the mean time the instance keeps accepting requests which can’t be processed as everything else has already been stopped/deactivated) To me this looks wrong. Regards, Eric ________________________________ From: Ruwan Linton [mailto:[email protected]] Sent: Thursday, April 02, 2009 3:59 AM To: [email protected] Subject: Re: startup order - correct place to start transport listeners I went through the new synapse startup logic and it seems OK but this makes the following concrete changes to the synapse architecture * Synapse can no longer be deployed just as a pure module in axis2, it requires much more work. * The message mediation is restricted to HTTP and HTTPS, which I am not sure whether we want to keep it as it is. But still this new logic even doesn't address the original problem in the discussion. In the new logic transport listeners starts even before the mediators getting loaded into synapse. I think we need to improve this, and to me Eric's point is completely a valid point. I will further look into resolving this and will keep the list posted. -- Ruwan Linton Senior Software Engineer & Product Manager; WSO2 ESB; http://wso2.org/esb WSO2 Inc.; http://wso2.org email: [email protected]; cell: +94 77 341 3097 blog: http://ruwansblog.blogspot.com
