I've implemented stopasgroup on a branch. Setting stopasgroup=true implies killasgroup=true. If killasgroup was specified as false, an exception is raised.
https://github.com/Supervisor/supervisor/compare/master...stopasgroup I'll wait for Mike to weigh in. Have a good weekend. Roger On Fri, Mar 30, 2012 at 4:26 PM, Roger Hoover <[email protected]>wrote: > Sorry, I accidentally forgot to include the mailing list. > > Ales, thanks for the feedback. One more question: > > It seems like a single config param such as config.stopcommand="signalall > SIGTERM", would combine two params into one: stopsignal and stopasgroup. > By itself, I'm not sure that's an improvement b/c it now requires parsing > one config option to break the two out. Do you see any other advantages to > combing the two params into one? > > Thanks, > > Roger > > On Fri, Mar 30, 2012 at 4:17 PM, Ales Zoulek <[email protected]>wrote: > >> Oh. We need to keep the conversation just in one place :) >> >> So just really quickly here: >> ----------------------------------- >> On Sat, Mar 31, 2012 at 1:07 AM, Roger Hoover <[email protected]>wrote: >> >>> >>> >>> On Fri, Mar 30, 2012 at 3:53 PM, Ales Zoulek <[email protected]>wrote: >>> >>>> Ah, got your point. >>>> >>>> You could always set stopsignal to SIGKILL, if you don't mind that the >>>> whole process group get's killed without prior sigterm. (Hackish..) >>>> >>> >>> Even this might not work b/c the parent will be killed during the stop >>> phase and supervisor won't go on to do the killasgroup. I haven't looked >>> at the implementation to be sure. >>> >>> >> I haven't tried that. but from what I could tell from the implementation, >> it could work. But not 100 % sure. >> >>> >>>> Would stopasgroup imply killasgroup? Or what would happen when >>>> stopasgroup=True and killasgroup=False? Would that kill SIGTERM to the >>>> whole group, but then killing just the parent? >>>> >>> >>> It seems like it should. I can't imagine why you'd >>> want stopasgroup=True and killasgroup=False. >>> >>> >> Sure. >> >> >>> >>>> This may be a nice time to talk about the signal handling as a whole. >>>> My idea was that we could have: >>>> 1] Exposing "signal" command, that would send arbitrary signal to the >>>> process and "signalgroup" that would send it to the whole process group. >>>> (for example: "signal SIGHUP myprocess" and "signalgroup SIGTERM >>>> myprocess") >>>> 2] Option for signal "chains", so for example sending SIGTERM could >>>> imply sending SIGKILL after stopwaitsecs >>>> 3] "stop myprocess" would be then just and alias for "signal SIGTERM >>>> myprocess" with chain TERM-KILL enabled. >>>> 4] Defaults would be of course backwards compatible. >>>> >>>> It may sound complicated, but process.kill() method is already >>>> implementing the "signal" command. So the change wouldn't be so dramatic. >>>> >>> >>> I assume you mean adding this command to supervisorctl. Seems like a >>> nice idea. We still need killasgroup and stopasgroup options in the >>> process config, yes? >>> >>> Mostly supervisorctl, but it would need a bit more support at RPC server >> side. >> Not necessarily, instead of stopasgroup, you could have something like >> config.stopcommand="signalall SIGTERM". That means generalizing the process >> management more broadly. But this is for wider signal management >> discussion. Not probly something that could be decided at a spot. >> >> >> I started thinking that we could combine those into a single config param >>> like "signalgroup=True" but as Samuele points out here ( >>> https://github.com/Supervisor/supervisor/pull/30#issuecomment-3826211), >>> killasgroup does not necessarily imply stopasgroup. >>> >>> >> >> Yes, the implication should go just one way. >> >> >>> >>>> >>>> But anyway, if there's not a bit agreement on refactoring and making >>>> bigger changes, I vote for "stopasgroup" for now... >>>> >>>> >>>> >>>> Ales >>>> >>>> ------------------------------------------------------ >>>> Ales Zoulek >>>> +420 604 332 515 >>>> Jabber: [email protected] >>>> ------------------------------------------------------ >>>> >>>> >>>> On Sat, Mar 31, 2012 at 12:16 AM, Roger Hoover >>>> <[email protected]>wrote: >>>> >>>>> This is in addition to killasgroup. Killasgroup doesn't work for this >>>>> situation b/c the parent process exits after receiving SIGTERM. >>>>> Supervisor >>>>> never sends SIGKILL b/c the parent is dead. This leaves the child >>>>> orphaned. >>>>> >>>>> >>>>> On Fri, Mar 30, 2012 at 2:59 PM, Ales Zoulek <[email protected]>wrote: >>>>> >>>>>> Hey, >>>>>> >>>>>> we already have that [1] and it's called killasgroup :) >>>>>> >>>>>> 1] >>>>>> https://github.com/Supervisor/supervisor/blob/master/supervisor/process.py#L354 >>>>>> >>>>>> >>>>>> Cheers, Ales >>>>>> >>>>>> ------------------------------------------------------ >>>>>> Ales Zoulek >>>>>> +420 604 332 515 >>>>>> Jabber: [email protected] >>>>>> ------------------------------------------------------ >>>>>> >>>>>> >>>>>> On Fri, Mar 30, 2012 at 8:40 PM, Roger Hoover <[email protected] >>>>>> > wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Any objections to a "stopasgroup" option? >>>>>>> >>>>>>> I've run into a case where I want to run Flask in debug mode under >>>>>>> supervisord in development and the parent Flask process doesn't >>>>>>> propagate >>>>>>> the SIGTERM or SIGINT signals to it's child, >>>>>>> leaving it orphaned. This doesn't happen on the command line b/c the >>>>>>> shell sends SIGINT to the entire foreground process group. >>>>>>> >>>>>>> For cases like this, it would be useful to be able to set an option, >>>>>>> called stopasgroup, that sends the stop signal to the whole process >>>>>>> group. >>>>>>> >>>>>>> Cheers, >>>>>>> >>>>>>> Roger >>>>>>> >>>>>>> >>>>>>> _______________________________________________ >>>>>>> Supervisor-users mailing list >>>>>>> [email protected] >>>>>>> http://lists.supervisord.org/mailman/listinfo/supervisor-users >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> >
_______________________________________________ Supervisor-users mailing list [email protected] http://lists.supervisord.org/mailman/listinfo/supervisor-users
