recovery_min_apply_delay with a negative value
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
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
Show quoted text
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
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
Attachments:
20150103_min_delay_negative_fix.patchtext/x-diff; charset=US-ASCII; name=20150103_min_delay_negative_fix.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index e5dddd4..5cc7e47 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5430,7 +5430,7 @@ recoveryApplyDelay(XLogReaderState *record)
int microsecs;
/* nothing to do if no delay configured */
- if (recovery_min_apply_delay == 0)
+ if (recovery_min_apply_delay <= 0)
return false;
/*
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
/messages/by-id/52A59D10.7020209@lab.ntt.co.jp
The argument for that was completely bogus, as noted further downthread:
/messages/by-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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers