Re: PATCH: Was: Re: Timed cookies

Hi Krzysztof,

On Mon, Oct 22, 2007 at 04:21:10PM +0200, Krzysztof Oledzki wrote:
> Hello,
>
> This patch implements new statistics for SLA calculation by adding new
> filed 'Dwntime' with total down time since restart (both HTTP/CSV) and
> extending status filed (HTTP) or inserting a new one (CVS) with time
> showing how long each server/backend is in a current state. Additionaly,
> down transations are also calculated and displayed for backends, so it is
> possible to know how many times selected backend was down, generating "No
> server is available to handle this request." error.
>
> New information are presentetd in two different ways:
> - for HTTP: a "human redable form", one of "100000d 23h", "23h 59m" or
> "59m 59s"
> - for CSV: seconds
>
> I believe that seconds resolution is enough.
>
> As there are more columns in the status page I decided to shrink some
> names to make more space:
> - Weight -> Wght
> - Check -> Chk
> - Down -> Dwn
>
> Making described changes I also made some improvements and fixed some
> small bugs:
> - don't increment s->health above 's->rise + s->fall - 1'. Previously it
> was incremented an then (re)set to 's->rise + s->fall - 1'.
> - do not set server down if it is down already
> - do not set server up if it is up already
> - fix colspan in multiple places (mostly introduced by my previous patch)
> - add missing "status" header to CSV
> - fix order of retries/redispatches in server (CSV)
> - s/Tthen/Then/
> - s/server/backend/ in DATA_ST_PX_BE (dumpstats.c)
>
> Changes from previous version:
> - deal with negative time intervales
> - don't relay on s->state (SRV_RUNNING)
> - little reworked human_time + compacted format (no spaces). If needed it
> can be used in the future for other purposes by optionally making "cnt"
> as an argument
> - live set_server_down mostly unchanged
> - only little reworked "process_chk: 9"
> - additional fields in CSV are appended to the rigth
> - fix "SEC" macro
> - named arguments (human_time, be_downtime, srv_downtime)
>
> Hope it is OK. If there are only cosmetic changes needed please fill free
> to correct it, however if there are some bigger changes required I would
> like to discuss it first or at last to know what exactly was changed
> especially since I already put this patch into my production server. :)

After a quick review, your patch looks perfect to me. I'll merge it. BTW, since you're using GIT too, I don't know if you already know it, but it's easy to regenerate a complete patch this way, assuming you have the official tree in the "mainline" branch, and your changes in the "mine" branch :

  $ git-format-patch -k -m mainline..mine

It will create one file per patch, with the description, author, date, and subject, along with the references to the ancestor. This is very convenient. I even use this inside my own branches to rearrange and combine changes by functional associations before merging them in "master".

> Thank you,
>
> Best regards,
>
> Krzysztof Ol?dzki

Thanks very much,
Willy Received on 2007/10/22 16:38