[Pgpool-hackers] [hackers] PgPool II patches : regex and syslog
Guillaume Lelarge
guillaume at lelarge.info
Fri Dec 24 15:06:27 UTC 2010
Hi guys,
Here are some comments after trying this patch.
First, I splitted the patch in two patches, so to have one patch per
functionality. You'll find the first one, syslog, attached.
Le 21/12/2010 15:11, Gilles Darold a écrit :
> [...]
> Syslog patch :
>
> It adds two configuration directives : "logsyslog = true|false" and
> "logfacility = 'LOCAL0'"
>
I would much prefer to have the same name than PostgreSQL options. No
logsyslog, but a log_destination which accepts stderr and syslog. No
logfacility, but syslog_facility.
> If 'logsyslog' is enabled, messages are sent to syslog (through
> pool_error.c) just after the call of pool_get_config() in main.c.
>
> Syslog ident it hard coded to "pgpool".
>
It shouldn't be hardcoded. I would better have a syslog_ident parameter.
> Connection to syslog is closed from pool_shmem_exit() in
> pool_shmem.c because this is the only method always called at exit.
> A better place should be in main.c but it has to be repeated so many
> time so that I choose this lazy place :-)
>
Seems enough for me.
> Samples configuration files has been patched too.
>
Apart from the options name, and the missing option, I have a big issue
with the FACILITY variable. Where do you initialize it? AFAICT, nowhere.
BTW, there is no reason to set only INIT_CONFIG for the facility
parameter. I should be INIT_CONFIG|RELOAD_CONFIG.
So, all in all, I like this patch but it still needs some work. Gilles,
could you work on the few issues I talked about? and send another patch
(and this time only on syslog). If you can't, I'll do it.
Now, I'll work on your second patch.
Thanks.
--
Guillaume
http://www.postgresql.fr
http://dalibo.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: syslog.patch
Type: text/x-diff
Size: 14090 bytes
Desc: not available
URL: <http://pgfoundry.org/pipermail/pgpool-hackers/attachments/20101224/f881e83d/attachment.bin>
More information about the Pgpool-hackers
mailing list