max_slot_wal_keep_size and wal_keep_segments

Started by Fujii Masaoalmost 6 years ago24 messageshackers
Jump to latest
#1Fujii Masao
masao.fujii@gmail.com

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

#2Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Fujii Masao (#1)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fujii Masao (#1)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#4Fujii Masao
masao.fujii@gmail.com
In reply to: Alvaro Herrera (#3)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#5Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fujii Masao (#4)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#6David Steele
david@pgmasters.net
In reply to: Alvaro Herrera (#5)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#7Bruce Momjian
bruce@momjian.us
In reply to: David Steele (#6)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#8Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Bruce Momjian (#7)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#9Bruce Momjian
bruce@momjian.us
In reply to: Alvaro Herrera (#8)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#10Masahiko Sawada
sawada.mshk@gmail.com
In reply to: David Steele (#6)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#11Fujii Masao
masao.fujii@gmail.com
In reply to: David Steele (#6)
Re: max_slot_wal_keep_size and wal_keep_segments

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

Attachments:

wal_keep_size_v1.patchtext/plain; charset=UTF-8; name=wal_keep_size_v1.patch; x-mac-creator=0; x-mac-type=0Download+57-47
#12Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Fujii Masao (#11)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#13Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Fujii Masao (#11)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#14Fujii Masao
masao.fujii@gmail.com
In reply to: Alvaro Herrera (#12)
Re: max_slot_wal_keep_size and wal_keep_segments

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

Attachments:

wal_keep_size_v2.patchtext/plain; charset=UTF-8; name=wal_keep_size_v2.patch; x-mac-creator=0; x-mac-type=0Download+58-48
#15Fujii Masao
masao.fujii@gmail.com
In reply to: Kyotaro Horiguchi (#13)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#16Kyotaro Horiguchi
horikyota.ntt@gmail.com
In reply to: Fujii Masao (#15)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#17Fujii Masao
masao.fujii@gmail.com
In reply to: Kyotaro Horiguchi (#16)
Re: max_slot_wal_keep_size and wal_keep_segments

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

Attachments:

wal_keep_size_v3.patchtext/plain; charset=UTF-8; name=wal_keep_size_v3.patch; x-mac-creator=0; x-mac-type=0Download+58-48
#18David Steele
david@pgmasters.net
In reply to: Fujii Masao (#17)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#19Fujii Masao
masao.fujii@gmail.com
In reply to: David Steele (#18)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#20David Steele
david@pgmasters.net
In reply to: Fujii Masao (#19)
Re: max_slot_wal_keep_size and wal_keep_segments

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

#21Fujii Masao
masao.fujii@gmail.com
In reply to: David Steele (#20)
#22Fujii Masao
masao.fujii@gmail.com
In reply to: Fujii Masao (#21)
#23David Steele
david@pgmasters.net
In reply to: Fujii Masao (#22)
#24Fujii Masao
masao.fujii@gmail.com
In reply to: David Steele (#23)