Horton

Thread

Commits

  1. doc: Mention the rename of wal_keep_segments GUC in release note.

  2. Rename wal_keep_segments to wal_keep_size.

  1. max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-06-30T14:51:40Z

    Hi,

When I talked about max_slot_wal_keep_size as new feature in v13
at the conference, I received the question like "Why are the units of
setting values in max_slot_wal_keep_size and wal_keep_segments different?"
from audience. That difference looks confusing for users and
IMO it's better to use the same unit for them. Thought?

There seem to be several options to do this.

(1) Rename max_slot_wal_keep_size to max_slot_wal_keep_segments,
     and make users specify the number of WAL segments in it instead of
     WAL size.

(2) Rename wal_keep_segments to wal_keep_size, and make users specify
      the WAL size in it instead of the number of WAL segments.

(3) Don't rename the parameters, and allow users to specify not only
     the number of WAL segments but also the WAL size in wal_keep_segments.

Since we have been moving away from measuring in segments, e.g.,
max_wal_size, I don't think (1) is good idea.

For backward compatibility, (3) is better. But which needs more
(maybe a bit complicated) code in guc.c. Also the parameter names
are not consistent yet (i.e., _segments and _size).

So for now I like (2).

Thought?

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  2. Re: max_slot_wal_keep_size and wal_keep_segments

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2020-07-01T00:19:18Z

    At Tue, 30 Jun 2020 23:51:40 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in 
> Hi,
> 
> When I talked about max_slot_wal_keep_size as new feature in v13
> at the conference, I received the question like "Why are the units of
> setting values in max_slot_wal_keep_size and wal_keep_segments
> different?"
> from audience. That difference looks confusing for users and
> IMO it's better to use the same unit for them. Thought?

We are moving the units for amount of WAL from segments to MB.  The
variable is affected by the movement.  I'm not sure wal_keep_segments
is going to die soon but we may change it to wal_keep_size(_mb) sooner
or later if it going to stay alive.

> There seem to be several options to do this.
> 
> (1) Rename max_slot_wal_keep_size to max_slot_wal_keep_segments,
>     and make users specify the number of WAL segments in it instead of
>     WAL size.

I don't think this is not the way.

> (2) Rename wal_keep_segments to wal_keep_size, and make users specify
>      the WAL size in it instead of the number of WAL segments.

Yes. I agree to this (as I wrote above before reading this).

> (3) Don't rename the parameters, and allow users to specify not only
>     the number of WAL segments but also the WAL size in wal_keep_segments.

Possible in a short term, but not for a long term.

> Since we have been moving away from measuring in segments, e.g.,
> max_wal_size, I don't think (1) is good idea.
> 
> For backward compatibility, (3) is better. But which needs more
> (maybe a bit complicated) code in guc.c. Also the parameter names
> are not consistent yet (i.e., _segments and _size).
> 
> So for now I like (2).
> 
> Thought?

I agree to you.  If someone found that wal_keep_segment is no longer
usable, the alternative would easily be found by searching config file
for "wal_keep". Or we could have a default config line like this:

wal_keep_size = 0     # in megabytes: 0 disables (formerly wal_keep_segments)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  3. Re: max_slot_wal_keep_size and wal_keep_segments

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2020-07-01T03:26:05Z

    On 2020-Jun-30, Fujii Masao wrote:

> Hi,
> 
> When I talked about max_slot_wal_keep_size as new feature in v13
> at the conference, I received the question like "Why are the units of
> setting values in max_slot_wal_keep_size and wal_keep_segments different?"
> from audience. That difference looks confusing for users and
> IMO it's better to use the same unit for them. Thought?

Do we still need wal_keep_segments for anything?  Maybe we should
consider removing that functionality instead ... and even if we don't
remove it in 13, then what is the point of renaming it only to remove it
shortly after?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  4. Re: max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-07-01T12:50:56Z

    
On 2020/07/01 12:26, Alvaro Herrera wrote:
> On 2020-Jun-30, Fujii Masao wrote:
> 
>> Hi,
>>
>> When I talked about max_slot_wal_keep_size as new feature in v13
>> at the conference, I received the question like "Why are the units of
>> setting values in max_slot_wal_keep_size and wal_keep_segments different?"
>> from audience. That difference looks confusing for users and
>> IMO it's better to use the same unit for them. Thought?
> 
> Do we still need wal_keep_segments for anything?

Yeah, personally I like wal_keep_segments because its setting is very
simple and no extra operations on replication slots are necessary.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  5. Re: max_slot_wal_keep_size and wal_keep_segments

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2020-07-01T14:54:28Z

    On 2020-Jul-01, Fujii Masao wrote:

> On 2020/07/01 12:26, Alvaro Herrera wrote:
> > On 2020-Jun-30, Fujii Masao wrote:
> > 
> > > When I talked about max_slot_wal_keep_size as new feature in v13
> > > at the conference, I received the question like "Why are the units of
> > > setting values in max_slot_wal_keep_size and wal_keep_segments different?"
> > > from audience. That difference looks confusing for users and
> > > IMO it's better to use the same unit for them. Thought?
> > 
> > Do we still need wal_keep_segments for anything?
> 
> Yeah, personally I like wal_keep_segments because its setting is very
> simple and no extra operations on replication slots are necessary.

Okay.  In that case I +1 the idea of renaming to wal_keep_size.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  6. Re: max_slot_wal_keep_size and wal_keep_segments

    David Steele <david@pgmasters.net> — 2020-07-01T17:18:06Z

    On 7/1/20 10:54 AM, Alvaro Herrera wrote:
> On 2020-Jul-01, Fujii Masao wrote:
> 
>> On 2020/07/01 12:26, Alvaro Herrera wrote:
>>> On 2020-Jun-30, Fujii Masao wrote:
>>>
>>>> When I talked about max_slot_wal_keep_size as new feature in v13
>>>> at the conference, I received the question like "Why are the units of
>>>> setting values in max_slot_wal_keep_size and wal_keep_segments different?"
>>>> from audience. That difference looks confusing for users and
>>>> IMO it's better to use the same unit for them. Thought?
>>>
>>> Do we still need wal_keep_segments for anything?
>>
>> Yeah, personally I like wal_keep_segments because its setting is very
>> simple and no extra operations on replication slots are necessary.
> 
> Okay.  In that case I +1 the idea of renaming to wal_keep_size.

+1 for renaming to wal_keep_size.

-- 
-David
david@pgmasters.net

  7. Re: max_slot_wal_keep_size and wal_keep_segments

    Bruce Momjian <bruce@momjian.us> — 2020-07-01T19:45:08Z

    On Wed, Jul  1, 2020 at 01:18:06PM -0400, David Steele wrote:
> On 7/1/20 10:54 AM, Alvaro Herrera wrote:
> > On 2020-Jul-01, Fujii Masao wrote:
> > 
> > > On 2020/07/01 12:26, Alvaro Herrera wrote:
> > > > On 2020-Jun-30, Fujii Masao wrote:
> > > > 
> > > > > When I talked about max_slot_wal_keep_size as new feature in v13
> > > > > at the conference, I received the question like "Why are the units of
> > > > > setting values in max_slot_wal_keep_size and wal_keep_segments different?"
> > > > > from audience. That difference looks confusing for users and
> > > > > IMO it's better to use the same unit for them. Thought?
> > > > 
> > > > Do we still need wal_keep_segments for anything?
> > > 
> > > Yeah, personally I like wal_keep_segments because its setting is very
> > > simple and no extra operations on replication slots are necessary.
> > 
> > Okay.  In that case I +1 the idea of renaming to wal_keep_size.
> 
> +1 for renaming to wal_keep_size.

We have the following wal*size GUC settings:

	SELECT name FROM pg_settings WHERE name LIKE '%wal%size%';
	          name
	------------------------
	 max_slot_wal_keep_size
	 max_wal_size
	 min_wal_size
	 wal_block_size
	 wal_segment_size

Does wal_keep_size make sense here?

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

  8. Re: max_slot_wal_keep_size and wal_keep_segments

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2020-07-01T20:25:35Z

    On 2020-Jul-01, Bruce Momjian wrote:

> We have the following wal*size GUC settings:
> 
> 	SELECT name FROM pg_settings WHERE name LIKE '%wal%size%';
> 	          name
> 	------------------------
> 	 max_slot_wal_keep_size
> 	 max_wal_size
> 	 min_wal_size
> 	 wal_block_size
> 	 wal_segment_size
> 
> Does wal_keep_size make sense here?

I think it does.  What do you think?

Are you suggesting that "keep_wal_size" is better, since it's more in
line with min/max?  I lean towards no.

(I think it's okay to conceptually separate these three options from
wal_block_size, since that's a compile time option and thus it's an
introspective GUC rather than actual configuration, but as I recall that
argument does not hold for wal_segment_size. But at one point, even that
one was an introspective GUC too.)

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  9. Re: max_slot_wal_keep_size and wal_keep_segments

    Bruce Momjian <bruce@momjian.us> — 2020-07-01T20:33:54Z

    On Wed, Jul  1, 2020 at 04:25:35PM -0400, Alvaro Herrera wrote:
> On 2020-Jul-01, Bruce Momjian wrote:
> 
> > We have the following wal*size GUC settings:
> > 
> > 	SELECT name FROM pg_settings WHERE name LIKE '%wal%size%';
> > 	          name
> > 	------------------------
> > 	 max_slot_wal_keep_size
> > 	 max_wal_size
> > 	 min_wal_size
> > 	 wal_block_size
> > 	 wal_segment_size
> > 
> > Does wal_keep_size make sense here?
> 
> I think it does.  What do you think?
> 
> Are you suggesting that "keep_wal_size" is better, since it's more in
> line with min/max?  I lean towards no.

No, I am more just asking since I saw wal_keep_size as a special version
of wal_size.  I don't have a firm opinion.

> 
> (I think it's okay to conceptually separate these three options from
> wal_block_size, since that's a compile time option and thus it's an
> introspective GUC rather than actual configuration, but as I recall that
> argument does not hold for wal_segment_size. But at one point, even that
> one was an introspective GUC too.)

Yep, just asking.

-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EnterpriseDB                             https://enterprisedb.com

  The usefulness of a cup is in its emptiness, Bruce Lee

  10. Re: max_slot_wal_keep_size and wal_keep_segments

    Masahiko Sawada <masahiko.sawada@2ndquadrant.com> — 2020-07-03T07:54:40Z

    On Thu, 2 Jul 2020 at 02:18, David Steele <david@pgmasters.net> wrote:
>
> On 7/1/20 10:54 AM, Alvaro Herrera wrote:
> > On 2020-Jul-01, Fujii Masao wrote:
> >
> >> On 2020/07/01 12:26, Alvaro Herrera wrote:
> >>> On 2020-Jun-30, Fujii Masao wrote:
> >>>
> >>>> When I talked about max_slot_wal_keep_size as new feature in v13
> >>>> at the conference, I received the question like "Why are the units of
> >>>> setting values in max_slot_wal_keep_size and wal_keep_segments different?"
> >>>> from audience. That difference looks confusing for users and
> >>>> IMO it's better to use the same unit for them. Thought?
> >>>
> >>> Do we still need wal_keep_segments for anything?
> >>
> >> Yeah, personally I like wal_keep_segments because its setting is very
> >> simple and no extra operations on replication slots are necessary.
> >
> > Okay.  In that case I +1 the idea of renaming to wal_keep_size.
>
> +1 for renaming to wal_keep_size.
>

+1 from me, too.

Regards,


--
Masahiko Sawada            http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  11. Re: max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-07-08T15:37:57Z

    
On 2020/07/02 2:18, David Steele wrote:
> On 7/1/20 10:54 AM, Alvaro Herrera wrote:
>> On 2020-Jul-01, Fujii Masao wrote:
>>
>>> On 2020/07/01 12:26, Alvaro Herrera wrote:
>>>> On 2020-Jun-30, Fujii Masao wrote:
>>>>
>>>>> When I talked about max_slot_wal_keep_size as new feature in v13
>>>>> at the conference, I received the question like "Why are the units of
>>>>> setting values in max_slot_wal_keep_size and wal_keep_segments different?"
>>>>> from audience. That difference looks confusing for users and
>>>>> IMO it's better to use the same unit for them. Thought?
>>>>
>>>> Do we still need wal_keep_segments for anything?
>>>
>>> Yeah, personally I like wal_keep_segments because its setting is very
>>> simple and no extra operations on replication slots are necessary.
>>
>> Okay.  In that case I +1 the idea of renaming to wal_keep_size.
> 
> +1 for renaming to wal_keep_size.

I attached the patch that renames wal_keep_segments to wal_keep_size.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  12. Re: max_slot_wal_keep_size and wal_keep_segments

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2020-07-08T16:20:31Z

    On 2020-Jul-09, Fujii Masao wrote:

> I attached the patch that renames wal_keep_segments to wal_keep_size.

Looks good in a quick once-over.  Just two small wording comments:

>    <para>
>     Independently of <varname>max_wal_size</varname>,
> -   <xref linkend="guc-wal-keep-segments"/> + 1 most recent WAL files are
> +   most recent <xref linkend="guc-wal-keep-size"/> megabytes
> +   WAL files plus one WAL file are
>     kept at all times. Also, if WAL archiving is used, old segments can not be
>     removed or recycled until they are archived. If WAL archiving cannot keep up
>     with the pace that WAL is generated, or if <varname>archive_command</varname>

This reads a little strange to me.  Maybe "the N most recent megabytes
plus ..."

>  			/* determine how many segments slots can be kept by slots ... */
> -			keepSegs = XLogMBVarToSegs(max_slot_wal_keep_size_mb, wal_segment_size);
> -			/* ... and override by wal_keep_segments as needed */
> -			keepSegs = Max(keepSegs, wal_keep_segments);
> +			slotKeepSegs = XLogMBVarToSegs(max_slot_wal_keep_size_mb, wal_segment_size);
> +			/* ... and override by wal_keep_size as needed */
> +			keepSegs = XLogMBVarToSegs(wal_keep_size_mb, wal_segment_size);

Since you change the way these two variables are used, I would not say
"override" in the above comment, nor keep the ellipses; perhaps just
change them to "determine how many segments can be kept by slots" and
"ditto for wal_keep_size".

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  13. Re: max_slot_wal_keep_size and wal_keep_segments

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2020-07-09T04:47:43Z

    At Thu, 9 Jul 2020 00:37:57 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in 
> 
> 
> On 2020/07/02 2:18, David Steele wrote:
> > On 7/1/20 10:54 AM, Alvaro Herrera wrote:
> >> On 2020-Jul-01, Fujii Masao wrote:
> >>
> >>> On 2020/07/01 12:26, Alvaro Herrera wrote:
> >>>> On 2020-Jun-30, Fujii Masao wrote:
> >>>>
> >>>>> When I talked about max_slot_wal_keep_size as new feature in v13
> >>>>> at the conference, I received the question like "Why are the units of
> >>>>> setting values in max_slot_wal_keep_size and wal_keep_segments
> >>>>> different?"
> >>>>> from audience. That difference looks confusing for users and
> >>>>> IMO it's better to use the same unit for them. Thought?
> >>>>
> >>>> Do we still need wal_keep_segments for anything?
> >>>
> >>> Yeah, personally I like wal_keep_segments because its setting is very
> >>> simple and no extra operations on replication slots are necessary.
> >>
> >> Okay.  In that case I +1 the idea of renaming to wal_keep_size.
> > +1 for renaming to wal_keep_size.
> 
> I attached the patch that renames wal_keep_segments to wal_keep_size.

It fails on 019_replslot_limit.pl for uncertain reason to me..


@@ -11323,7 +11329,7 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 	 * If archiving is enabled, wait for all the required WAL files to be
 	 * archived before returning. If archiving isn't enabled, the required WAL
 	 * needs to be transported via streaming replication (hopefully with
-	 * wal_keep_segments set high enough), or some more exotic mechanism like
+	 * wal_keep_size set high enough), or some more exotic mechanism like
 	 * polling and copying files from pg_wal with script. We have no knowledge

Isn't this time a good chance to mention replication slots?


-	"ALTER SYSTEM SET wal_keep_segments to 8; SELECT pg_reload_conf();");
+	"ALTER SYSTEM SET wal_keep_size to '128MB'; SELECT pg_reload_conf();");

wal_segment_size to 1MB here so, that conversion is not correct.
(However, that test works as long as it is more than
max_slot_wal_keep_size so it's practically no problem.)

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  14. Re: max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-07-13T05:11:05Z

    
On 2020/07/09 1:20, Alvaro Herrera wrote:
> On 2020-Jul-09, Fujii Masao wrote:
> 
>> I attached the patch that renames wal_keep_segments to wal_keep_size.
> 
> Looks good in a quick once-over.  Just two small wording comments:

Thanks for review comments!


> 
>>     <para>
>>      Independently of <varname>max_wal_size</varname>,
>> -   <xref linkend="guc-wal-keep-segments"/> + 1 most recent WAL files are
>> +   most recent <xref linkend="guc-wal-keep-size"/> megabytes
>> +   WAL files plus one WAL file are
>>      kept at all times. Also, if WAL archiving is used, old segments can not be
>>      removed or recycled until they are archived. If WAL archiving cannot keep up
>>      with the pace that WAL is generated, or if <varname>archive_command</varname>
> 
> This reads a little strange to me.  Maybe "the N most recent megabytes
> plus ..."

Yes, fixed.


> 
>>   			/* determine how many segments slots can be kept by slots ... */
>> -			keepSegs = XLogMBVarToSegs(max_slot_wal_keep_size_mb, wal_segment_size);
>> -			/* ... and override by wal_keep_segments as needed */
>> -			keepSegs = Max(keepSegs, wal_keep_segments);
>> +			slotKeepSegs = XLogMBVarToSegs(max_slot_wal_keep_size_mb, wal_segment_size);
>> +			/* ... and override by wal_keep_size as needed */
>> +			keepSegs = XLogMBVarToSegs(wal_keep_size_mb, wal_segment_size);
> 
> Since you change the way these two variables are used, I would not say
> "override" in the above comment, nor keep the ellipses; perhaps just
> change them to "determine how many segments can be kept by slots" and
> "ditto for wal_keep_size".

Yes, fixed.

Attached is the updated version of the patch.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  15. Re: max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-07-13T05:14:30Z

    
On 2020/07/09 13:47, Kyotaro Horiguchi wrote:
> At Thu, 9 Jul 2020 00:37:57 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
>>
>>
>> On 2020/07/02 2:18, David Steele wrote:
>>> On 7/1/20 10:54 AM, Alvaro Herrera wrote:
>>>> On 2020-Jul-01, Fujii Masao wrote:
>>>>
>>>>> On 2020/07/01 12:26, Alvaro Herrera wrote:
>>>>>> On 2020-Jun-30, Fujii Masao wrote:
>>>>>>
>>>>>>> When I talked about max_slot_wal_keep_size as new feature in v13
>>>>>>> at the conference, I received the question like "Why are the units of
>>>>>>> setting values in max_slot_wal_keep_size and wal_keep_segments
>>>>>>> different?"
>>>>>>> from audience. That difference looks confusing for users and
>>>>>>> IMO it's better to use the same unit for them. Thought?
>>>>>>
>>>>>> Do we still need wal_keep_segments for anything?
>>>>>
>>>>> Yeah, personally I like wal_keep_segments because its setting is very
>>>>> simple and no extra operations on replication slots are necessary.
>>>>
>>>> Okay.  In that case I +1 the idea of renaming to wal_keep_size.
>>> +1 for renaming to wal_keep_size.
>>
>> I attached the patch that renames wal_keep_segments to wal_keep_size.
> 
> It fails on 019_replslot_limit.pl for uncertain reason to me..

I could not reproduce this...


> 
> 
> @@ -11323,7 +11329,7 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
>   	 * If archiving is enabled, wait for all the required WAL files to be
>   	 * archived before returning. If archiving isn't enabled, the required WAL
>   	 * needs to be transported via streaming replication (hopefully with
> -	 * wal_keep_segments set high enough), or some more exotic mechanism like
> +	 * wal_keep_size set high enough), or some more exotic mechanism like
>   	 * polling and copying files from pg_wal with script. We have no knowledge
> 
> Isn't this time a good chance to mention replication slots?

+1 to do that. But I found there are other places where replication slots
need to be mentioned. So I think it's better to do this as separate patch.


> 
> 
> -	"ALTER SYSTEM SET wal_keep_segments to 8; SELECT pg_reload_conf();");
> +	"ALTER SYSTEM SET wal_keep_size to '128MB'; SELECT pg_reload_conf();");
> 
> wal_segment_size to 1MB here so, that conversion is not correct.
> (However, that test works as long as it is more than
> max_slot_wal_keep_size so it's practically no problem.)

So I changed 128MB to 8MB. Is this OK?
I attached the updated version of the patch upthread.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  16. Re: max_slot_wal_keep_size and wal_keep_segments

    Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2020-07-13T07:01:11Z

    At Mon, 13 Jul 2020 14:14:30 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in 
> 
> 
> On 2020/07/09 13:47, Kyotaro Horiguchi wrote:
> > At Thu, 9 Jul 2020 00:37:57 +0900, Fujii Masao
> > <masao.fujii@oss.nttdata.com> wrote in
> >>
> >>
> >> On 2020/07/02 2:18, David Steele wrote:
> >>> On 7/1/20 10:54 AM, Alvaro Herrera wrote:
> >>>> On 2020-Jul-01, Fujii Masao wrote:
> >>>>
> >>>>> On 2020/07/01 12:26, Alvaro Herrera wrote:
> >>>>>> On 2020-Jun-30, Fujii Masao wrote:
> >>>>>>
> >>>>>>> When I talked about max_slot_wal_keep_size as new feature in v13
> >>>>>>> at the conference, I received the question like "Why are the units of
> >>>>>>> setting values in max_slot_wal_keep_size and wal_keep_segments
> >>>>>>> different?"
> >>>>>>> from audience. That difference looks confusing for users and
> >>>>>>> IMO it's better to use the same unit for them. Thought?
> >>>>>>
> >>>>>> Do we still need wal_keep_segments for anything?
> >>>>>
> >>>>> Yeah, personally I like wal_keep_segments because its setting is very
> >>>>> simple and no extra operations on replication slots are necessary.
> >>>>
> >>>> Okay.  In that case I +1 the idea of renaming to wal_keep_size.
> >>> +1 for renaming to wal_keep_size.
> >>
> >> I attached the patch that renames wal_keep_segments to wal_keep_size.
> > It fails on 019_replslot_limit.pl for uncertain reason to me..
> 
> I could not reproduce this...

Sorry for the ambiguity.  The patch didn't applied on the file, and I
noticed that the reason is the wording change from master to
primary. So no problem in the latest patch.

> > @@ -11323,7 +11329,7 @@ do_pg_stop_backup(char *labelfile, bool
> > waitforarchive, TimeLineID *stoptli_p)
> >   	 * If archiving is enabled, wait for all the required WAL files to be
> >   	 * archived before returning. If archiving isn't enabled, the required
> >   	 * WAL
> >   	 * needs to be transported via streaming replication (hopefully with
> > -	 * wal_keep_segments set high enough), or some more exotic mechanism like
> > + * wal_keep_size set high enough), or some more exotic mechanism like
> >   	 * polling and copying files from pg_wal with script. We have no
> >   	 * knowledge
> > Isn't this time a good chance to mention replication slots?
> 
> +1 to do that. But I found there are other places where replication
> slots
> need to be mentioned. So I think it's better to do this as separate
> patch.

Agreed.

> > -	"ALTER SYSTEM SET wal_keep_segments to 8; SELECT pg_reload_conf();");
> > + "ALTER SYSTEM SET wal_keep_size to '128MB'; SELECT
> > pg_reload_conf();");
> > wal_segment_size to 1MB here so, that conversion is not correct.
> > (However, that test works as long as it is more than
> > max_slot_wal_keep_size so it's practically no problem.)
> 
> So I changed 128MB to 8MB. Is this OK?
> I attached the updated version of the patch upthread.

That change looks find by me.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center

  17. Re: max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-07-14T04:00:35Z

    
On 2020/07/13 16:01, Kyotaro Horiguchi wrote:
> At Mon, 13 Jul 2020 14:14:30 +0900, Fujii Masao <masao.fujii@oss.nttdata.com> wrote in
>>
>>
>> On 2020/07/09 13:47, Kyotaro Horiguchi wrote:
>>> At Thu, 9 Jul 2020 00:37:57 +0900, Fujii Masao
>>> <masao.fujii@oss.nttdata.com> wrote in
>>>>
>>>>
>>>> On 2020/07/02 2:18, David Steele wrote:
>>>>> On 7/1/20 10:54 AM, Alvaro Herrera wrote:
>>>>>> On 2020-Jul-01, Fujii Masao wrote:
>>>>>>
>>>>>>> On 2020/07/01 12:26, Alvaro Herrera wrote:
>>>>>>>> On 2020-Jun-30, Fujii Masao wrote:
>>>>>>>>
>>>>>>>>> When I talked about max_slot_wal_keep_size as new feature in v13
>>>>>>>>> at the conference, I received the question like "Why are the units of
>>>>>>>>> setting values in max_slot_wal_keep_size and wal_keep_segments
>>>>>>>>> different?"
>>>>>>>>> from audience. That difference looks confusing for users and
>>>>>>>>> IMO it's better to use the same unit for them. Thought?
>>>>>>>>
>>>>>>>> Do we still need wal_keep_segments for anything?
>>>>>>>
>>>>>>> Yeah, personally I like wal_keep_segments because its setting is very
>>>>>>> simple and no extra operations on replication slots are necessary.
>>>>>>
>>>>>> Okay.  In that case I +1 the idea of renaming to wal_keep_size.
>>>>> +1 for renaming to wal_keep_size.
>>>>
>>>> I attached the patch that renames wal_keep_segments to wal_keep_size.
>>> It fails on 019_replslot_limit.pl for uncertain reason to me..
>>
>> I could not reproduce this...
> 
> Sorry for the ambiguity.  The patch didn't applied on the file, and I
> noticed that the reason is the wording change from master to
> primary. So no problem in the latest patch.
> 
>>> @@ -11323,7 +11329,7 @@ do_pg_stop_backup(char *labelfile, bool
>>> waitforarchive, TimeLineID *stoptli_p)
>>>    	 * If archiving is enabled, wait for all the required WAL files to be
>>>    	 * archived before returning. If archiving isn't enabled, the required
>>>    	 * WAL
>>>    	 * needs to be transported via streaming replication (hopefully with
>>> -	 * wal_keep_segments set high enough), or some more exotic mechanism like
>>> + * wal_keep_size set high enough), or some more exotic mechanism like
>>>    	 * polling and copying files from pg_wal with script. We have no
>>>    	 * knowledge
>>> Isn't this time a good chance to mention replication slots?
>>
>> +1 to do that. But I found there are other places where replication
>> slots
>> need to be mentioned. So I think it's better to do this as separate
>> patch.
> 
> Agreed.
> 
>>> -	"ALTER SYSTEM SET wal_keep_segments to 8; SELECT pg_reload_conf();");
>>> + "ALTER SYSTEM SET wal_keep_size to '128MB'; SELECT
>>> pg_reload_conf();");
>>> wal_segment_size to 1MB here so, that conversion is not correct.
>>> (However, that test works as long as it is more than
>>> max_slot_wal_keep_size so it's practically no problem.)
>>
>> So I changed 128MB to 8MB. Is this OK?
>> I attached the updated version of the patch upthread.
> 
> That change looks find by me.

Thanks for the review!

The patch was no longer applied cleanly because of recent commit.
So I updated the patch. Attached.

Barring any objection, I will commit this patch.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  18. Re: max_slot_wal_keep_size and wal_keep_segments

    David Steele <david@pgmasters.net> — 2020-07-14T11:30:24Z

    On 7/14/20 12:00 AM, Fujii Masao wrote:
> 
> The patch was no longer applied cleanly because of recent commit.
> So I updated the patch. Attached.
> 
> Barring any objection, I will commit this patch.

This doesn't look right:

+   the <xref linkend="guc-wal-keep-size"/> most recent megabytes
+   WAL files plus one WAL file are

How about:

+   <xref linkend="guc-wal-keep-size"/> megabytes of
+   WAL files plus one WAL file are

Other than that, looks good to me.

Regards,
-- 
-David
david@pgmasters.net

  19. Re: max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-07-17T09:11:36Z

    
On 2020/07/14 20:30, David Steele wrote:
> On 7/14/20 12:00 AM, Fujii Masao wrote:
>>
>> The patch was no longer applied cleanly because of recent commit.
>> So I updated the patch. Attached.
>>
>> Barring any objection, I will commit this patch.
> 
> This doesn't look right:
> 
> +   the <xref linkend="guc-wal-keep-size"/> most recent megabytes
> +   WAL files plus one WAL file are
> 
> How about:
> 
> +   <xref linkend="guc-wal-keep-size"/> megabytes of
> +   WAL files plus one WAL file are

Thanks for the comment! Isn't it better to keep "most recent" part?
If so, what about either of the followings?

1. <xref linkend="guc-wal-keep-size"/> megabytes of WAL files plus
     one WAL file that were most recently generated are kept all time.

2. <xref linkend="guc-wal-keep-size"/> megabytes + <xref linkend="guc-wal-segment-size"> bytes
     of WAL files that were most recently generated are kept all time.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  20. Re: max_slot_wal_keep_size and wal_keep_segments

    David Steele <david@pgmasters.net> — 2020-07-17T11:24:07Z

    On 7/17/20 5:11 AM, Fujii Masao wrote:
> 
> 
> On 2020/07/14 20:30, David Steele wrote:
>> On 7/14/20 12:00 AM, Fujii Masao wrote:
>>>
>>> The patch was no longer applied cleanly because of recent commit.
>>> So I updated the patch. Attached.
>>>
>>> Barring any objection, I will commit this patch.
>>
>> This doesn't look right:
>>
>> +   the <xref linkend="guc-wal-keep-size"/> most recent megabytes
>> +   WAL files plus one WAL file are
>>
>> How about:
>>
>> +   <xref linkend="guc-wal-keep-size"/> megabytes of
>> +   WAL files plus one WAL file are
> 
> Thanks for the comment! Isn't it better to keep "most recent" part?
> If so, what about either of the followings?
> 
> 1. <xref linkend="guc-wal-keep-size"/> megabytes of WAL files plus
>      one WAL file that were most recently generated are kept all time.
> 
> 2. <xref linkend="guc-wal-keep-size"/> megabytes + <xref 
> linkend="guc-wal-segment-size"> bytes
>      of WAL files that were most recently generated are kept all time.

"most recent" seemed implied to me, but I see your point.

How about:

+   the most recent <xref linkend="guc-wal-keep-size"/> megabytes of
+   WAL files plus one additional WAL file are

Regards,
-- 
-David
david@pgmasters.net

  21. Re: max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-07-20T04:48:37Z

    
On 2020/07/17 20:24, David Steele wrote:
> 
> On 7/17/20 5:11 AM, Fujii Masao wrote:
>>
>>
>> On 2020/07/14 20:30, David Steele wrote:
>>> On 7/14/20 12:00 AM, Fujii Masao wrote:
>>>>
>>>> The patch was no longer applied cleanly because of recent commit.
>>>> So I updated the patch. Attached.
>>>>
>>>> Barring any objection, I will commit this patch.
>>>
>>> This doesn't look right:
>>>
>>> +   the <xref linkend="guc-wal-keep-size"/> most recent megabytes
>>> +   WAL files plus one WAL file are
>>>
>>> How about:
>>>
>>> +   <xref linkend="guc-wal-keep-size"/> megabytes of
>>> +   WAL files plus one WAL file are
>>
>> Thanks for the comment! Isn't it better to keep "most recent" part?
>> If so, what about either of the followings?
>>
>> 1. <xref linkend="guc-wal-keep-size"/> megabytes of WAL files plus
>>      one WAL file that were most recently generated are kept all time.
>>
>> 2. <xref linkend="guc-wal-keep-size"/> megabytes + <xref linkend="guc-wal-segment-size"> bytes
>>      of WAL files that were most recently generated are kept all time.
> 
> "most recent" seemed implied to me, but I see your point.
> 
> How about:
> 
> +   the most recent <xref linkend="guc-wal-keep-size"/> megabytes of
> +   WAL files plus one additional WAL file are

I adopted this and pushed the patch. Thanks!

Also we need to update the release note for v13. What about adding the following?

------------------------------------
Rename configuration parameter wal_keep_segments to wal_keep_size.

This allows how much WAL files to retain for the standby server, by bytes instead of the number of files.
If you previously used wal_keep_segments, the following formula will give you an approximately equivalent setting:

wal_keep_size = wal_keep_segments * wal_segment_size (typically 16MB)
------------------------------------


Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  22. Re: max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-07-20T10:02:50Z

    
On 2020/07/20 13:48, Fujii Masao wrote:
> 
> 
> On 2020/07/17 20:24, David Steele wrote:
>>
>> On 7/17/20 5:11 AM, Fujii Masao wrote:
>>>
>>>
>>> On 2020/07/14 20:30, David Steele wrote:
>>>> On 7/14/20 12:00 AM, Fujii Masao wrote:
>>>>>
>>>>> The patch was no longer applied cleanly because of recent commit.
>>>>> So I updated the patch. Attached.
>>>>>
>>>>> Barring any objection, I will commit this patch.
>>>>
>>>> This doesn't look right:
>>>>
>>>> +   the <xref linkend="guc-wal-keep-size"/> most recent megabytes
>>>> +   WAL files plus one WAL file are
>>>>
>>>> How about:
>>>>
>>>> +   <xref linkend="guc-wal-keep-size"/> megabytes of
>>>> +   WAL files plus one WAL file are
>>>
>>> Thanks for the comment! Isn't it better to keep "most recent" part?
>>> If so, what about either of the followings?
>>>
>>> 1. <xref linkend="guc-wal-keep-size"/> megabytes of WAL files plus
>>>      one WAL file that were most recently generated are kept all time.
>>>
>>> 2. <xref linkend="guc-wal-keep-size"/> megabytes + <xref linkend="guc-wal-segment-size"> bytes
>>>      of WAL files that were most recently generated are kept all time.
>>
>> "most recent" seemed implied to me, but I see your point.
>>
>> How about:
>>
>> +   the most recent <xref linkend="guc-wal-keep-size"/> megabytes of
>> +   WAL files plus one additional WAL file are
> 
> I adopted this and pushed the patch. Thanks!
> 
> Also we need to update the release note for v13. What about adding the following?
> 
> ------------------------------------
> Rename configuration parameter wal_keep_segments to wal_keep_size.
> 
> This allows how much WAL files to retain for the standby server, by bytes instead of the number of files.
> If you previously used wal_keep_segments, the following formula will give you an approximately equivalent setting:
> 
> wal_keep_size = wal_keep_segments * wal_segment_size (typically 16MB)
> ------------------------------------

Patch attached.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  23. Re: max_slot_wal_keep_size and wal_keep_segments

    David Steele <david@pgmasters.net> — 2020-07-20T12:21:46Z

    On 7/20/20 6:02 AM, Fujii Masao wrote:
> 
> 
> On 2020/07/20 13:48, Fujii Masao wrote:
>>
>>
>> On 2020/07/17 20:24, David Steele wrote:
>>>
>>> On 7/17/20 5:11 AM, Fujii Masao wrote:
>>>>
>>>>
>>>> On 2020/07/14 20:30, David Steele wrote:
>>>>> On 7/14/20 12:00 AM, Fujii Masao wrote:
>>>>>>
>>>>>> The patch was no longer applied cleanly because of recent commit.
>>>>>> So I updated the patch. Attached.
>>>>>>
>>>>>> Barring any objection, I will commit this patch.
>>>>>
>>>>> This doesn't look right:
>>>>>
>>>>> +   the <xref linkend="guc-wal-keep-size"/> most recent megabytes
>>>>> +   WAL files plus one WAL file are
>>>>>
>>>>> How about:
>>>>>
>>>>> +   <xref linkend="guc-wal-keep-size"/> megabytes of
>>>>> +   WAL files plus one WAL file are
>>>>
>>>> Thanks for the comment! Isn't it better to keep "most recent" part?
>>>> If so, what about either of the followings?
>>>>
>>>> 1. <xref linkend="guc-wal-keep-size"/> megabytes of WAL files plus
>>>>      one WAL file that were most recently generated are kept all time.
>>>>
>>>> 2. <xref linkend="guc-wal-keep-size"/> megabytes + <xref 
>>>> linkend="guc-wal-segment-size"> bytes
>>>>      of WAL files that were most recently generated are kept all time.
>>>
>>> "most recent" seemed implied to me, but I see your point.
>>>
>>> How about:
>>>
>>> +   the most recent <xref linkend="guc-wal-keep-size"/> megabytes of
>>> +   WAL files plus one additional WAL file are
>>
>> I adopted this and pushed the patch. Thanks!
>>
>> Also we need to update the release note for v13. What about adding the 
>> following?
>>
>> ------------------------------------
>> Rename configuration parameter wal_keep_segments to wal_keep_size.
>>
>> This allows how much WAL files to retain for the standby server, by 
>> bytes instead of the number of files.
>> If you previously used wal_keep_segments, the following formula will 
>> give you an approximately equivalent setting:
>>
>> wal_keep_size = wal_keep_segments * wal_segment_size (typically 16MB)
>> ------------------------------------

I would rework that first sentence a bit. How about:

+ This determines how much WAL to retain for the standby server,
+ specified in megabytes rather than number of files.

The rest looks fine to me.

Regards,
-- 
-David
david@pgmasters.net

  24. Re: max_slot_wal_keep_size and wal_keep_segments

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2020-07-28T02:25:54Z

    
On 2020/07/20 21:21, David Steele wrote:
> On 7/20/20 6:02 AM, Fujii Masao wrote:
>>
>>
>> On 2020/07/20 13:48, Fujii Masao wrote:
>>>
>>>
>>> On 2020/07/17 20:24, David Steele wrote:
>>>>
>>>> On 7/17/20 5:11 AM, Fujii Masao wrote:
>>>>>
>>>>>
>>>>> On 2020/07/14 20:30, David Steele wrote:
>>>>>> On 7/14/20 12:00 AM, Fujii Masao wrote:
>>>>>>>
>>>>>>> The patch was no longer applied cleanly because of recent commit.
>>>>>>> So I updated the patch. Attached.
>>>>>>>
>>>>>>> Barring any objection, I will commit this patch.
>>>>>>
>>>>>> This doesn't look right:
>>>>>>
>>>>>> +   the <xref linkend="guc-wal-keep-size"/> most recent megabytes
>>>>>> +   WAL files plus one WAL file are
>>>>>>
>>>>>> How about:
>>>>>>
>>>>>> +   <xref linkend="guc-wal-keep-size"/> megabytes of
>>>>>> +   WAL files plus one WAL file are
>>>>>
>>>>> Thanks for the comment! Isn't it better to keep "most recent" part?
>>>>> If so, what about either of the followings?
>>>>>
>>>>> 1. <xref linkend="guc-wal-keep-size"/> megabytes of WAL files plus
>>>>>      one WAL file that were most recently generated are kept all time.
>>>>>
>>>>> 2. <xref linkend="guc-wal-keep-size"/> megabytes + <xref linkend="guc-wal-segment-size"> bytes
>>>>>      of WAL files that were most recently generated are kept all time.
>>>>
>>>> "most recent" seemed implied to me, but I see your point.
>>>>
>>>> How about:
>>>>
>>>> +   the most recent <xref linkend="guc-wal-keep-size"/> megabytes of
>>>> +   WAL files plus one additional WAL file are
>>>
>>> I adopted this and pushed the patch. Thanks!
>>>
>>> Also we need to update the release note for v13. What about adding the following?
>>>
>>> ------------------------------------
>>> Rename configuration parameter wal_keep_segments to wal_keep_size.
>>>
>>> This allows how much WAL files to retain for the standby server, by bytes instead of the number of files.
>>> If you previously used wal_keep_segments, the following formula will give you an approximately equivalent setting:
>>>
>>> wal_keep_size = wal_keep_segments * wal_segment_size (typically 16MB)
>>> ------------------------------------
> 
> I would rework that first sentence a bit. How about:
> 
> + This determines how much WAL to retain for the standby server,
> + specified in megabytes rather than number of files.
> 
> The rest looks fine to me.

Thanks for the review!
I adopted your suggestion in the updated version of the patch and pushed it.

Regards,


-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION