Unnecessary limit on max_standby_streaming_delay

The limit on max_standby_streaming_delay is currently 35 minutes
(around) - or you have to set it to unlimited. This is because the GUC
is limited to MAX_INT/1000, unit milliseconds.

Is there a reason for the /1000, or is it just an oversight thinking
the unit was in seconds?

If we can get rid of the /1000, it would make the limit over three
weeks, which seems much more reasonable. Or if we made it a 64-bit
counter it would go away completely.

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Magnus Hagander wrote:
> The limit on max_standby_streaming_delay is currently 35 minutes
> (around) - or you have to set it to unlimited. This is because the GUC
> is limited to MAX_INT/1000, unit milliseconds.
>
> Is there a reason for the /1000, or is it just an oversight thinking
> the unit was in seconds?
>
> If we can get rid of the /1000, it would make the limit over three
> weeks, which seems much more reasonable. Or if we made it a 64-bit
> counter it would go away completely.
>

The code for this uses TimestampTzPlusMilliseconds to determine its
deadline:

return TimestampTzPlusMilliseconds(rtime, max_standby_streaming_delay);

Which is implemented like this:

#define TimestampTzPlusMilliseconds(tz,ms) ((tz) + ((ms) * (int64) 1000))

My guess is that the range was limited at some point to avoid concerns
of integer overflow in that multiplication, which I don't think actually
is a risk due the int64 cast there.

I confirmed the upper limit on this thing is the <36 minutes that Magnus
suggests. This is a terrible limitation to have slipped through. For
the pg_dump from the standby use case, the appropriate setting for most
people is in the multiple hours range, but not unlimited (lest an out of
control backup linger to overlap with what happens the next day).
Whichever approach is taken to make this better, I think it deserves a
backport too.

--
Greg Smith 2ndQuadrant US greg(at)2ndQuadrant(dot)com Baltimore, MD
PostgreSQL Training, Services and Support www.2ndQuadrant.us

Greg Smith <greg(at)2ndquadrant(dot)com> writes:
> Magnus Hagander wrote:
>> The limit on max_standby_streaming_delay is currently 35 minutes
>> (around) - or you have to set it to unlimited. This is because the GUC
>> is limited to MAX_INT/1000, unit milliseconds.
>>
>> Is there a reason for the /1000, or is it just an oversight thinking
>> the unit was in seconds?

> My guess is that the range was limited at some point to avoid concerns
> of integer overflow in that multiplication, which I don't think actually
> is a risk due the int64 cast there.

Yes, it's certainly there on the thought that somebody might try to
convert the value to microseconds in integer arithmetic. If you run
through all the uses of the variable and confirm that that never
happens, maybe it'd be safe to enlarge the limit. Check the units-aware
GUC printing code in particular.

regards, tom lane

On fre, 2010-12-17 at 12:57 +0100, Magnus Hagander wrote:
> The limit on max_standby_streaming_delay is currently 35 minutes
> (around) - or you have to set it to unlimited. This is because the GUC
> is limited to MAX_INT/1000, unit milliseconds.
>
> Is there a reason for the /1000, or is it just an oversight thinking
> the unit was in seconds?

Yeah, I actually noticed this week that log_min_duration_statement is
limited to the same 35 minutes for the same reason. Might be good to
address that, too. Note that statement_timeout does not have that
limit.

Peter Eisentraut wrote:
> On fre, 2010-12-17 at 12:57 +0100, Magnus Hagander wrote:
> > The limit on max_standby_streaming_delay is currently 35 minutes
> > (around) - or you have to set it to unlimited. This is because the GUC
> > is limited to MAX_INT/1000, unit milliseconds.
> >
> > Is there a reason for the /1000, or is it just an oversight thinking
> > the unit was in seconds?
>
> Yeah, I actually noticed this week that log_min_duration_statement is
> limited to the same 35 minutes for the same reason. Might be good to
> address that, too. Note that statement_timeout does not have that
> limit.

Added to TODO:

Increase maximum values for max_standby_streaming_delay and
log_min_duration_statement

* http://archives.postgresql.org/pgsql-hackers/2010-12/msg01517.php

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +