[Pgpool-hackers] [hackers] PgPool II patches : regex and syslog
Tatsuo Ishii
ishii at sraoss.co.jp
Sun Dec 26 03:30:11 UTC 2010
> 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.
Me too.
>> 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.
Right.
>> 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.
I think so too.
>> 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
More information about the Pgpool-hackers
mailing list