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

[Tatsuo Ishii]

> Le 27/12/2010 15:59, Guillaume Lelarge a écrit :
>> 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.
> Damn you're right, I just simply omit this part. Sorry for the
> additional work.
>> 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.
> No, this is a flag to delay PgPool logging to syslog only after the end
> of configuration file reading. The problem appears when PgPool is run in
> debug mode, during the configuration file it detect that log_destination
> is set to syslog so if you don't wait (or use a flag)  it will try to
> write to syslog before the syslog connection is opened.
> 
>>> 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.
> Ok, memorized: one purpose per patch. Please remove it and make an other
> one if you think it should be applied.
> 
>> 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.
> Yes this is because in cvs pool_config.c has not been fully regenerated
> from pool_config.l and some part of the regex patch have not been
> applied to pool_config.c

I think you only need patches against pool_config.l. I will generate
pool_config.c from pool_config.l.

>> And thanks Gilles for this second patch. I should really start working
>> on my own :)
> 
> Thanks a lot for your patch review and improvement.
> 
> -- 
> Gilles Darold
> http://dalibo.com - http://dalibo.org
> 
> _______________________________________________
> Pgpool-hackers mailing list
> Pgpool-hackers at pgfoundry.org
> http://pgfoundry.org/mailman/listinfo/pgpool-hackers