>> Le 28/12/2010 01:53, Tatsuo Ishii a écrit : >>>> 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. >>> >> >> Do you need another patch from us, or is the current enough for you? > > No, your patches look good. I will commit as soon as possible.
Done. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp _______________________________________________ Pgpool-hackers mailing list [email protected] http://pgfoundry.org/mailman/listinfo/pgpool-hackers
