Thread: max_slot_wal_keep_size and wal_keep_segments
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
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
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
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
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
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
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
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
Attachment
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
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
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
Attachment
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
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
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
Attachment
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
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
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
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
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
Attachment
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
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