[Pgpool-hackers] worker process launch even when health_check_period=0

[Guillaume Lelarge]

On 05/23/2011 09:32 PM, Guillaume Lelarge wrote:
> On 05/23/2011 03:59 PM, Tatsuo Ishii wrote:
>>> Le 05/23/2011 03:02 PM, Tatsuo Ishii a écrit :
>>>>> AFAICT, the worker process has one work to do: check replication lag.
>>>>> pgPool only needs to check replication lag if we are using streaming
>>>>> replication and if health check is enabled. However, it seems it checks
>>>>> replication lag even if health check is disabled (with
>>>>> health_check_period=0).
>>>>>
>>>>> This looks like a bug to me.
>>>>>
>>>>> I see two fixes:
>>>>>   * only launch the worker process if health_check_period>0 (main.c)
>>>>>   * before checking lag, make sure that health_check_period>0
>>>>>     (pool_worker_child.c)
>>>>>
>>>>> I can write the patch, it won't be difficult. Just want to make sure
>>>>> you're OK with this and that I didn't miss something.
>>>>
>>>> People might want to check replication delay even if they are not
>>>> using health check, no?
>>>
>>> Sure. They might want. If we say so, we shouldn't say otherwise in the
>>> docs ("If delay_threshold is 0 or health checking is disabled, the delay
>>> checking is not performed.") Moreover, I don't see why the worker should
>>> sleep 30+health_check_period.
>>
>> Not really. sleep 30 only happens when health_check_period <= 0.
>>
> 
> Oh yeah, you're right.
> 
>>> And last one, it's kind of weird to
>>> disable helath-check but still use health_user to check the replication lag.
>>>
>>> Maybe we should use the worker for different kinds of checks:
>>> replication lag, health, others?
>>
>> Yes, I think we'd better to have "delay_check_period" or some such.
>>
> 
> Well, if we use different period of time, it'll get harder and harder to
> understand each part. I think it would be better to have a
> worker_naptime, worker_user (they would replace health_check_period and
> health_check_user). But we would keep health_check_timeout, and
> delay_threshold. And both activities would take place in the worker
> process. But this is more 3.1, or even 3.2 material.
> 
>>> Anyway, right now, we have one main issue. We say in the docs that
>>> replication lag is disabled if health_check is disabled, but the code
>>> doesn't do that. At least one of them should be fixed. And I think we
>>> need to fix the code.
>>
>> Oops. You are right. We should fix the code.
>>
>> BTW, this part:
>>>>>   * only launch the worker process if health_check_period>0 (main.c)
>>
>> If we want to do:
>>
>> If delay_threshold/health_check_period set to greater than 0 and
>> pgpool.conf is reloaded, I would like to see delay checking starts.
>> Like wise, if delay_threshold/health_check_period is set to 0 and
>> pgpool.conf reloded, we need to stop worker process.
>>
>> Probably it's lot easier to always start worker process right now,
>> just check those variables in the process.
> 
> Completely agree. I didn't think about configuration reloading, and
> you're completely right: it's better to launch the worker, it makes it
> more simple to handle configuration reloading.
> 
> Working on a patch to fix the issue in 3.0.
> 

Done.


-- 
Guillaume
 http://www.postgresql.fr
 http://dalibo.com