Re: [PATCH] implement "track backend/server"

On Tue, Feb 05, 2008 at 09:37:06PM +0100, Krzysztof Oledzki wrote:
> >From 75107d3319f77f4fcf7f5cfe8174046aca1766cf Mon Sep 17 00:00:00 2001
> From: Krzysztof Piotr Oledzki <ole#ans.pl>
> Date: Tue, 5 Feb 2008 21:29:31 +0100
> Subject: [RFC] implement "track backend/server"
>
> This rfc-quality patch implements ability to set a current state of
> one server by tracking another one. It:

Wow, that's an excellent news!

BTW, I think that it is a good thing to use a notifier list, because it will make it easier to later track scripts. That's what I did in keepalived IIRC. Just a few comments :

• when you parse the "track" keyword, I think that the name of the proxy and server should not be checked immediately, but deffered. This would allow to track independently of the declaration order. That's what is done for use_backend for instance. The change is not necessarily hard, it basically consists in moving the assignment code further (around line 3000 in cfg_parse.c), and adding a check to ensure that srv->tracked->tracked == NULL to ensure there's no loop and that we're not chaining tracking.
• there's one extra comma in the chunk below :

> diff --git a/src/dumpstats.c b/src/dumpstats.c
> index 59ee93b..9f8d21c 100644
> --- a/src/dumpstats.c
> +++ b/src/dumpstats.c
> @@ -706,8 +706,8 @@ int stats_dump_proxy(struct session *s, struct proxy *px, struct uri_auth *uri,
> "%s,"
> /* rest of server: nothing */
> ",,,,,,,,"
> - /* pid, iid, sid, throttle, lbtot, */
> - "%d,%d,0,,,"
> + /* pid, iid, sid, throttle, lbtot, tracked*/
> + "%d,%d,0,,,,,"
> "\n",
> px->id,
> px->feconn, px->feconn_max, px->maxconn, px->cum_feconn,

Other than that, the code looks good.

Do you think that it would be useful to implement a more generic notifier mechanism now or should we delay it when we implement scripted checks ?

The idea would be to separate servers and checks, with checks affecting server states by propagating the change along a list, and doing so for themselves too, so that the code remains as generic as possible. This way, simple checks and complex scripts would simply consist in normal tasks notifying a server list about state changes.

Regards,
Willy Received on 2008/02/10 13:47