Re: Making start/stop idempotent
http://www.squid-cache.org/bugs/show_bug.cgi?id=2599 On 31/10/2008, at 3:59 AM, Alex Rousskov wrote: On Thu, 2008-10-30 at 00:58 +0100, Henrik Nordstrom wrote: On tor, 2008-10-30 at 10:03 +1100, Mark Nottingham wrote: Hmm, good point. My aim was just start and stop. Seem reasonable to just limit it to those two? Yes. And maybe maybe rotate. Perhaps rotate should continue to fail. Those who do not like that, can always do something like !check || rotate in their cron jobs, right? In general, an operation that cannot accomplish its goal should fail. I am not going to fight against shutdown pretending that it has shutdown Squid, but faking rotate is more dangerous than convenient, IMO. Thanks, Alex. -- Mark Nottingham m...@yahoo-inc.com
Re: Making start/stop idempotent
On Thu, 2008-10-30 at 00:58 +0100, Henrik Nordstrom wrote: On tor, 2008-10-30 at 10:03 +1100, Mark Nottingham wrote: Hmm, good point. My aim was just start and stop. Seem reasonable to just limit it to those two? Yes. And maybe maybe rotate. Perhaps rotate should continue to fail. Those who do not like that, can always do something like !check || rotate in their cron jobs, right? In general, an operation that cannot accomplish its goal should fail. I am not going to fight against shutdown pretending that it has shutdown Squid, but faking rotate is more dangerous than convenient, IMO. Thanks, Alex.
Re: Making start/stop idempotent
This tests for opt_send_signal == 0. idempotent_start.patch Description: Binary data On 29/10/2008, at 12:13 PM, Henrik Nordstrom wrote: On ons, 2008-10-29 at 11:32 +1100, Mark Nottingham wrote: I'm not seeing that; no crash, and -k check still correctly finds syntax errors in squid.conf. Behaviour appears the same as without the patch. squid -k check is for checking if Squid is running. squid -k parse is the one parsing the config only.. Regards Henrik -- Mark Nottingham [EMAIL PROTECTED]
Re: Making start/stop idempotent
How soft should this be? Do we really want -k rotate/reconfigure/debug etc to consider Squid not running to be a normal situation? Well, rotate maybe, to avoid cron jobs failing. But certainly not reconfigure. On tor, 2008-10-30 at 09:46 +1100, Mark Nottingham wrote: This tests for opt_send_signal == 0. On 29/10/2008, at 12:13 PM, Henrik Nordstrom wrote: On ons, 2008-10-29 at 11:32 +1100, Mark Nottingham wrote: I'm not seeing that; no crash, and -k check still correctly finds syntax errors in squid.conf. Behaviour appears the same as without the patch. squid -k check is for checking if Squid is running. squid -k parse is the one parsing the config only.. Regards Henrik -- Mark Nottingham [EMAIL PROTECTED] signature.asc Description: This is a digitally signed message part
Re: Making start/stop idempotent
Hmm, good point. My aim was just start and stop. Seem reasonable to just limit it to those two? On 30/10/2008, at 9:52 AM, Henrik Nordstrom wrote: How soft should this be? Do we really want -k rotate/reconfigure/ debug etc to consider Squid not running to be a normal situation? Well, rotate maybe, to avoid cron jobs failing. But certainly not reconfigure. On tor, 2008-10-30 at 09:46 +1100, Mark Nottingham wrote: This tests for opt_send_signal == 0. On 29/10/2008, at 12:13 PM, Henrik Nordstrom wrote: On ons, 2008-10-29 at 11:32 +1100, Mark Nottingham wrote: I'm not seeing that; no crash, and -k check still correctly finds syntax errors in squid.conf. Behaviour appears the same as without the patch. squid -k check is for checking if Squid is running. squid -k parse is the one parsing the config only.. Regards Henrik -- Mark Nottingham [EMAIL PROTECTED] -- Mark Nottingham [EMAIL PROTECTED]
Re: Making start/stop idempotent
The first part next to checkRunningPid makes sense. But the second part crashes squid -k check, by always returning true if the pid file doesn't exists.. On tis, 2008-10-28 at 20:16 +1100, Mark Nottingham wrote: I realised I never sent the patch without the option; On 12/08/2008, at 4:14 PM, Amos Jeffries wrote: So, effectively just change to exit(0) in those two places? For the HEAD commit(s) yes. If you need it ported the option may be needed to keep other users going. Amos On 12/08/2008, at 4:02 PM, Amos Jeffries wrote: Our management tools like processes to be idempotent; i.e., you should be able to start or stop a process any number of times without it throwing an error. Currently, Squid will return 1 if a squid process is already running (upon start) and when there isn't one (upon -k shutdown). I'm writing a patch to change this behaviour, and the most reasonable way to do it seems to be with a command-line option; I've somewhat arbitrarily chosen -p. Does this seem reasonable? If so, I'll submit a patch shortly. I agree with Robert. The behavior change is nice. Requiring another option to enable/disable it is not. I'd personally go with just the behavior change to the HEAD trees. Only disabling and adding the option to enable if its ported to currently stable releases (2.7, 3.0). Amos -- Mark Nottingham [EMAIL PROTECTED] -- Mark Nottingham [EMAIL PROTECTED] signature.asc Description: This is a digitally signed message part
Re: Making start/stop idempotent
I'm not seeing that; no crash, and -k check still correctly finds syntax errors in squid.conf. Behaviour appears the same as without the patch. On 29/10/2008, at 7:26 AM, Henrik Nordstrom wrote: The first part next to checkRunningPid makes sense. But the second part crashes squid -k check, by always returning true if the pid file doesn't exists.. On tis, 2008-10-28 at 20:16 +1100, Mark Nottingham wrote: I realised I never sent the patch without the option; On 12/08/2008, at 4:14 PM, Amos Jeffries wrote: So, effectively just change to exit(0) in those two places? For the HEAD commit(s) yes. If you need it ported the option may be needed to keep other users going. Amos On 12/08/2008, at 4:02 PM, Amos Jeffries wrote: Our management tools like processes to be idempotent; i.e., you should be able to start or stop a process any number of times without it throwing an error. Currently, Squid will return 1 if a squid process is already running (upon start) and when there isn't one (upon -k shutdown). I'm writing a patch to change this behaviour, and the most reasonable way to do it seems to be with a command-line option; I've somewhat arbitrarily chosen -p. Does this seem reasonable? If so, I'll submit a patch shortly. I agree with Robert. The behavior change is nice. Requiring another option to enable/disable it is not. I'd personally go with just the behavior change to the HEAD trees. Only disabling and adding the option to enable if its ported to currently stable releases (2.7, 3.0). Amos -- Mark Nottingham [EMAIL PROTECTED] -- Mark Nottingham [EMAIL PROTECTED] -- Mark Nottingham [EMAIL PROTECTED]
Re: Making start/stop idempotent
On ons, 2008-10-29 at 11:32 +1100, Mark Nottingham wrote: I'm not seeing that; no crash, and -k check still correctly finds syntax errors in squid.conf. Behaviour appears the same as without the patch. squid -k check is for checking if Squid is running. squid -k parse is the one parsing the config only.. Regards Henrik signature.asc Description: This is a digitally signed message part
Re: Making start/stop idempotent
Duh. OK, will fix. On 29/10/2008, at 12:13 PM, Henrik Nordstrom wrote: On ons, 2008-10-29 at 11:32 +1100, Mark Nottingham wrote: I'm not seeing that; no crash, and -k check still correctly finds syntax errors in squid.conf. Behaviour appears the same as without the patch. squid -k check is for checking if Squid is running. squid -k parse is the one parsing the config only.. Regards Henrik -- Mark Nottingham [EMAIL PROTECTED]
Re: Making start/stop idempotent
On 12/08/2008, at 3:50 PM, Robert Collins wrote: On Tue, 2008-08-12 at 15:30 +1000, Mark Nottingham wrote: Well, here's a patch for the behaviour I described... A patch for 3.x would be nice; I'll see what I can do, but undoubtedly I'll be slower than others... your patch seems fine on the surface, but adding an option does just seem ugly to me :) Agreed, but I wasn't sure how important staying compatible with current behaviour is for others... IME messing with startup/shutdown behaviour should be done cautiously... -Rob -- GPG key available at: http://www.robertcollins.net/keys.txt. -- Mark Nottingham [EMAIL PROTECTED]
Re: Making start/stop idempotent
So, effectively just change to exit(0) in those two places? For the HEAD commit(s) yes. If you need it ported the option may be needed to keep other users going. Amos On 12/08/2008, at 4:02 PM, Amos Jeffries wrote: Our management tools like processes to be idempotent; i.e., you should be able to start or stop a process any number of times without it throwing an error. Currently, Squid will return 1 if a squid process is already running (upon start) and when there isn't one (upon -k shutdown). I'm writing a patch to change this behaviour, and the most reasonable way to do it seems to be with a command-line option; I've somewhat arbitrarily chosen -p. Does this seem reasonable? If so, I'll submit a patch shortly. I agree with Robert. The behavior change is nice. Requiring another option to enable/disable it is not. I'd personally go with just the behavior change to the HEAD trees. Only disabling and adding the option to enable if its ported to currently stable releases (2.7, 3.0). Amos -- Mark Nottingham [EMAIL PROTECTED]
Re: Making start/stop idempotent
On Tue, 2008-08-12 at 14:43 +1000, Mark Nottingham wrote: Our management tools like processes to be idempotent; i.e., you should be able to start or stop a process any number of times without it throwing an error. Currently, Squid will return 1 if a squid process is already running (upon start) and when there isn't one (upon -k shutdown). I'm writing a patch to change this behaviour, and the most reasonable way to do it seems to be with a command-line option; I've somewhat arbitrarily chosen -p. Does this seem reasonable? If so, I'll submit a patch shortly. I would be happy with not needing an option to have idempotent start-and-stop behaviour; we could use an option to preserve the current behaviour (though that doesn't seem particularly useful). -Rob -- GPG key available at: http://www.robertcollins.net/keys.txt. signature.asc Description: This is a digitally signed message part