[Pgpool-hackers] [hackers] PgPool II patches : regex and syslog

[Guillaume Lelarge]

Hi,

Le 27/12/2010 10:47, Gilles Darold a écrit :
> [...]
> Here is the rewritten patch that add syslog support to PgPool with all
> your great suggestions applied.
> This patch has been created from the current cvs HEAD branch.
> 

Thanks. There is a bug in it, that I fixed in the attached patch. Even
if you declare log_destination and syslog_ident as reloadable, it was
without effect because there was no code to handle opening or closing
syslog if they changed.

There is another thing that annoys me, but it isn't a bug. What is the
purpose of pool_config->logsyslog? is it just to avoid a strcmp? which
seems a poor optimization wrt having two variables with same meaning.

> This patch also modify the comment about logdir configuraion directive.
> 
> "Logging directory" has been change into "PgPool status file logging
> directory"
> 
> Feel free to apply or remove it. It just seems to me that 'logdir' =>
> "Logging directory" is very confusing and false as this directive is
> only used to save the pgpool_status file. It will certainly be better to
> rename this directive, but for the moment just changing the comment help
> a lot for understanding and preserve backward compatibility.
> 

I completely agree with you on that change. But I would much prefer to
see it applied as another patch rather than in this patch. They have two
different purposes, so they shouldn't be mixed.

BTW, Tatsuo-san, Gilles and I have absolutely no idea if we should
change pool_config.c, pool_config.l, or both. We did both, but I would
really like to know which of these three possibilities is the good one.
Thanks.

And thanks Gilles for this second patch. I should really start working
on my own :)


-- 
Guillaume
 http://www.postgresql.fr
 http://dalibo.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pgpool-II-patch_syslog_v2.patch
Type: text/x-diff
Size: 20430 bytes
Desc: not available
URL: <http://pgfoundry.org/pipermail/pgpool-hackers/attachments/20101227/ba1d4652/attachment.bin>