Обсуждение: recovery_min_apply_delay with a negative value

Поиск
Список
Период
Сортировка

recovery_min_apply_delay with a negative value

От
Michael Paquier
Дата:
Hi all,

While reviewing another patch, I have noticed that recovery_min_apply_delay can have a negative value. And the funny part is that we actually attempt to apply a delay even in this case, per se this condition recoveryApplyDelay@xlog.c:
        /* nothing to do if no delay configured */
        if (recovery_min_apply_delay == 0)
                return false;
Shouldn't we simply leave if recovery_min_apply_delay is lower 0, and not only equal to 0?
Regards,
--
Michael

Re: recovery_min_apply_delay with a negative value

От
Fabrízio de Royes Mello
Дата:


On Sun, Dec 28, 2014 at 12:31 PM, Michael Paquier <michael.paquier@gmail.com> wrote:
>
> Hi all,
>
> While reviewing another patch, I have noticed that recovery_min_apply_delay can have a negative value. And the funny part is that we actually attempt to apply a delay even in this case, per se this condition recoveryApplyDelay@xlog.c:
>         /* nothing to do if no delay configured */
>         if (recovery_min_apply_delay == 0)
>                 return false;
> Shouldn't we simply leave if recovery_min_apply_delay is lower 0, and not only equal to 0?
>

Seems reasonable.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello

Re: recovery_min_apply_delay with a negative value

От
Michael Paquier
Дата:
On Tue, Dec 30, 2014 at 4:30 AM, Fabrízio de Royes Mello
<fabriziomello@gmail.com> wrote:
>
>
> On Sun, Dec 28, 2014 at 12:31 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>>
>> Hi all,
>>
>> While reviewing another patch, I have noticed that
>> recovery_min_apply_delay can have a negative value. And the funny part is
>> that we actually attempt to apply a delay even in this case, per se this
>> condition recoveryApplyDelay@xlog.c:
>>         /* nothing to do if no delay configured */
>>         if (recovery_min_apply_delay == 0)
>>                 return false;
>> Shouldn't we simply leave if recovery_min_apply_delay is lower 0, and not
>> only equal to 0?
>>
>
> Seems reasonable.
Trivial patch for master and REL9_4_STABLE attached as long as I don't
forget it..
--
Michael

Вложения

Re: recovery_min_apply_delay with a negative value

От
Tom Lane
Дата:
Michael Paquier <michael.paquier@gmail.com> writes:
> On Tue, Dec 30, 2014 at 4:30 AM, Fabr�zio de Royes Mello
> <fabriziomello@gmail.com> wrote:
>> Shouldn't we simply leave if recovery_min_apply_delay is lower 0, and not
>> only equal to 0?

> Trivial patch for master and REL9_4_STABLE attached as long as I don't
> forget it..

It was originally intentional that the apply delay could be negative, cf

http://www.postgresql.org/message-id/52A59D10.7020209@lab.ntt.co.jp

The argument for that was completely bogus, as noted further downthread:

http://www.postgresql.org/message-id/20131212110505.GA14510@alap2.anarazel.de

but it looks like there are still residues of it in the committed patch;
both this and the totally meaningless reference to timezone differential
in the parameter's documentation.

Of course, if recovery_min_apply_delay were a proper GUC, we'd just
configure it with a minimum value of zero and be done :-(
        regards, tom lane



Re: recovery_min_apply_delay with a negative value

От
Robert Haas
Дата:
On Sat, Jan 3, 2015 at 12:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Of course, if recovery_min_apply_delay were a proper GUC, we'd just
> configure it with a minimum value of zero and be done :-(

Amen.  We should *really* convert all of the recovery.conf parameters
to be GUCs.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: recovery_min_apply_delay with a negative value

От
Petr Jelinek
Дата:
On 05/01/15 20:44, Robert Haas wrote:
> On Sat, Jan 3, 2015 at 12:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Of course, if recovery_min_apply_delay were a proper GUC, we'd just
>> configure it with a minimum value of zero and be done :-(
>
> Amen.  We should *really* convert all of the recovery.conf parameters
> to be GUCs.
>

Well, there is an ongoing effort on that and I think the patch is very 
close to the state where committer should take a look IMHO, I have only 
couple of gripes with it now and one of them needs opinions of others 
anyway.

--  Petr Jelinek                  http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training &
Services