|
Hi Sean,
Thanks for all your changes. I have a couple of comments, but they look
good to me. I have pushed them into http://hg.vergenet.net/linux-ha/dev/
and will push them up to http://hg.linux-ha.org/dev/ once my access
to that tree is fixed (sorry for the delay there, I only noticed
it was broken this morning).
One or two things about the patch posting, as I hope you send some
more patches some day :-)
1. I'd prefer one patch per email, as that makes it
easier to discuss individual patches. But its not a big deal.
2. I know the commit logs on linux-ha.org are rather brief,
but I'm happty if you want to include the longer descriptions
you put in the cover portion of your email.
3. Again this isn't really common practice on linux-ha.org,
but if you want to add a Signed-off-by line, that is also fine by me.
I tend to do that myself - except when I forget.
On Tue, Apr 21, 2009 at 01:51:41PM -0400, Sean E. Millichamp wrote:
> I have some relatively minor patches for ldirectord to address some
> issues we had in our environment. The patches are attached to this
> email. All are developed against the mercurial checkout at
> http://hg.linux-ha.org/dev/
>
> They are relatively independent in concept, but I developed them
> cumulatively, so if they weren't all applied in order I would expect
> some issues.
>
> ldirectord-01-cleanup-forked-children.patch:
>
> I observed that when ldirectord exits while using fork=yes the children
> forked to perform the checks are not killed or otherwise cleaned up and
> continue running. This patch sends the children SIGTERM when ldirectord
> exits.
I'm surprised that this is needed. But it isn't the first time this
has come up, so I'm happy top put the change in.
> ldirectord-02-dont-disable-realservers-in-forking.patch:
>
> When using fork=no there is a sanity check on the state of the LVS
> pools, and ldirectord modifies them as necessary. If there are
> realservers already present in existing pools ldirectord doesn't disable
> them until checks can complete. However, when using fork=yes this same
> sanity check is performed, but all realservers are set down until one
> check passes successfully. This patch fixes this behavior so that the
> realservers are not set down initially when fork=yes
I'm not sure that I completely understand the difference in desired
behaviour between forking and non-forking.
> ldirectord-03-prefix-forked-processes-with-ldirectord.patch:
>
> It is very nice how ldirectord changes the process name of the forked
> children to be more informative, but it makes it difficult to easily
> grep for all ldirectord related processes and non-obvious for people
> less familiar with ldirectord what those processes are. This patch just
> prefixes the names of the forked children with "ldirectord", otherwise
> keeping the current names.
Agreed
> ldirectord-04-add-cleanstop-option.patch:
>
> We would like the ability to stop ldirectord without it clearing out our
> pools. The new "cleanstop" option allows global or per virtual service
> ability to disable this cleanup. The default remains to perform the
> cleanup.
Good idea
> ldirectord-05-add-per-virtual-service-checkinterval.patch:
>
> The current checkinterval option is global only. However, when using
> fork=yes it is possible and sensible to want to adjust the checkinterval
> on a per-service basis. This patch allows you to do that, expands on
> the documentation for the option, and warns you if you try to set
> checkinterval in a virtual service definition when fork=no.
Also a good idea.
--
Simon Horman
VA Linux Systems Japan K.K. Satellite Lab in Sydney, Australia
H: www.vergenet.net/~horms/ W: www.valinux.co.jp/en
_______________________________________________
Please read the documentation before posting - it's available at:
http://www.linuxvirtualserver.org/
LinuxVirtualServer.org mailing list - lvs-users@xxxxxxxxxxxxxxxxxxxxxx
Send requests to lvs-users-request@xxxxxxxxxxxxxxxxxxxxxx
or go to http://lists.graemef.net/mailman/listinfo/lvs-users
|
