Thread: Unnecessary limit on max_standby_streaming_delay

Unnecessary limit on max_standby_streaming_delay

From
Magnus Hagander
Date:
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/


Re: Unnecessary limit on max_standby_streaming_delay

From
Greg Smith
Date:
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@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services and Support        www.2ndQuadrant.us




Re: Unnecessary limit on max_standby_streaming_delay

From
Tom Lane
Date:
Greg Smith <greg@2ndquadrant.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


Re: Unnecessary limit on max_standby_streaming_delay

From
Peter Eisentraut
Date:
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.



Re: Unnecessary limit on max_standby_streaming_delay

From
Bruce Momjian
Date:
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 andlog_min_duration_statement    *
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01517.php
 

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + It's impossible for everything to be true. +