pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: david@pgmasters.net
Cc: alvherre@2ndquadrant.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: max_slot_wal_keep_size and wal_keep_segments
Date: Tue, 14 Jul 2020 13:00:35 +0900
Message-ID: <d8d60d1a-5233-cbcc-099d-ee34033ea619@oss.nttdata.com> (raw)
In-Reply-To: <20200713.160111.1304073320526166244.horikyota.ntt@gmail.com>
References: <6e4f9605-97f3-7eba-f989-4ae41d637d2f@oss.nttdata.com>
	<20200709.134743.1136524996274963021.horikyota.ntt@gmail.com>
	<866c681c-3082-7010-cc9e-bf3b9483c991@oss.nttdata.com>
	<20200713.160111.1304073320526166244.horikyota.ntt@gmail.com>



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.patch (text/plain, 14.9K) download diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index e9cdff4864..3d973a5c8e 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -11249,7 +11249,7 @@ SELECT * FROM pg_locks pl LEFT JOIN pg_prepared_xacts ppx <para><literal>extended</literal> means that <varname>max_wal_size</varname> is exceeded but the files are still retained, either by the replication slot or - by <varname>wal_keep_segments</varname>. + by <varname>wal_keep_size</varname>. </para> </listitem> <listitem> diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index b353c61683..3384a5197d 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3124,7 +3124,7 @@ include_dir 'conf.d' checkpoints. This is a soft limit; WAL size can exceed <varname>max_wal_size</varname> under special circumstances, such as heavy load, a failing <varname>archive_command</varname>, or a high - <varname>wal_keep_segments</varname> setting. + <varname>wal_keep_size</varname> setting. If this value is specified without units, it is taken as megabytes. The default is 1 GB. Increasing this parameter can increase the amount of time needed for @@ -3751,21 +3751,21 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows </listitem> </varlistentry> - <varlistentry id="guc-wal-keep-segments" xreflabel="wal_keep_segments"> - <term><varname>wal_keep_segments</varname> (<type>integer</type>) + <varlistentry id="guc-wal-keep-size" xreflabel="wal_keep_size"> + <term><varname>wal_keep_size</varname> (<type>integer</type>) <indexterm> - <primary><varname>wal_keep_segments</varname> configuration parameter</primary> + <primary><varname>wal_keep_size</varname> configuration parameter</primary> </indexterm> </term> <listitem> <para> - Specifies the minimum number of past log file segments kept in the + Specifies the minimum size of past log file segments kept in the <filename>pg_wal</filename> directory, in case a standby server needs to fetch them for streaming - replication. Each segment is normally 16 megabytes. If a standby + replication. If a standby server connected to the sending server falls behind by more than - <varname>wal_keep_segments</varname> segments, the sending server might remove - a WAL segment still needed by the standby, in which case the + <varname>wal_keep_size</varname> megabytes, the sending server might + remove a WAL segment still needed by the standby, in which case the replication connection will be terminated. Downstream connections will also eventually fail as a result. (However, the standby server can recover by fetching the segment from archive, if WAL @@ -3773,14 +3773,15 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"' # Windows </para> <para> - This sets only the minimum number of segments retained in + This sets only the minimum size of segments retained in <filename>pg_wal</filename>; the system might need to retain more segments for WAL archival or to recover from a checkpoint. If - <varname>wal_keep_segments</varname> is zero (the default), the system + <varname>wal_keep_size</varname> is zero (the default), the system doesn't keep any extra segments for standby purposes, so the number of old WAL segments available to standby servers is a function of the location of the previous checkpoint and status of WAL archiving. + If this value is specified without units, it is taken as megabytes. This parameter can only be set in the <filename>postgresql.conf</filename> file or on the server command line. </para> diff --git a/doc/src/sgml/high-availability.sgml b/doc/src/sgml/high-availability.sgml index 6a9184f314..89f6d6eda6 100644 --- a/doc/src/sgml/high-availability.sgml +++ b/doc/src/sgml/high-availability.sgml @@ -785,7 +785,7 @@ archive_cleanup_command = 'pg_archivecleanup /path/to/archive %r' archiving, the server might recycle old WAL segments before the standby has received them. If this occurs, the standby will need to be reinitialized from a new base backup. You can avoid this by setting - <varname>wal_keep_segments</varname> to a value large enough to ensure that + <varname>wal_keep_size</varname> to a value large enough to ensure that WAL segments are not recycled too early, or by configuring a replication slot for the standby. If you set up a WAL archive that's accessible from the standby, these solutions are not required, since the standby can @@ -929,7 +929,7 @@ primary_conninfo = 'host=192.168.1.50 port=5432 user=foo password=foopass' </para> <para> In lieu of using replication slots, it is possible to prevent the removal - of old WAL segments using <xref linkend="guc-wal-keep-segments"/>, or by + of old WAL segments using <xref linkend="guc-wal-keep-size"/>, or by storing the segments in an archive using <xref linkend="guc-archive-command"/>. However, these methods often result in retaining more WAL segments than diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index aa41fb444f..e246efbdb5 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -305,7 +305,7 @@ PostgreSQL documentation <para> The write-ahead log files are collected at the end of the backup. Therefore, it is necessary for the - <xref linkend="guc-wal-keep-segments"/> parameter to be set high + <xref linkend="guc-wal-keep-size"/> parameter to be set high enough that the log is not removed before the end of the backup. If the log has been rotated when it's time to transfer it, the backup will fail and be unusable. diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml index 1902f36291..ff3625f855 100644 --- a/doc/src/sgml/wal.sgml +++ b/doc/src/sgml/wal.sgml @@ -578,7 +578,8 @@ <para> Independently of <varname>max_wal_size</varname>, - <xref linkend="guc-wal-keep-segments"/> + 1 most recent WAL files are + the <xref linkend="guc-wal-keep-size"/> most recent 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> diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 0a97b1d37f..184c6672f3 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -88,7 +88,7 @@ extern uint32 bootstrap_data_checksum_version; /* User-settable parameters */ int max_wal_size_mb = 1024; /* 1 GB */ int min_wal_size_mb = 80; /* 80 MB */ -int wal_keep_segments = 0; +int wal_keep_size_mb = 0; int XLOGbuffers = -1; int XLogArchiveTimeout = 0; int XLogArchiveMode = ARCHIVE_MODE_OFF; @@ -9525,7 +9525,7 @@ GetWALAvailability(XLogRecPtr targetLSN) /* * Calculate the oldest segment currently reserved by all slots, - * considering wal_keep_segments and max_slot_wal_keep_size. Initialize + * considering wal_keep_size and max_slot_wal_keep_size. Initialize * oldestSlotSeg to the current segment. */ currpos = GetXLogWriteRecPtr(); @@ -9576,9 +9576,9 @@ GetWALAvailability(XLogRecPtr targetLSN) /* * Retreat *logSegNo to the last segment that we need to retain because of - * either wal_keep_segments or replication slots. + * either wal_keep_size or replication slots. * - * This is calculated by subtracting wal_keep_segments from the given xlog + * This is calculated by subtracting wal_keep_size from the given xlog * location, recptr and by making sure that that result is below the * requirement of replication slots. For the latter criterion we do consider * the effects of max_slot_wal_keep_size: reserve at most that much space back @@ -9616,14 +9616,20 @@ KeepLogSeg(XLogRecPtr recptr, XLogSegNo *logSegNo) } } - /* but, keep at least wal_keep_segments if that's set */ - if (wal_keep_segments > 0 && currSegNo - segno < wal_keep_segments) + /* but, keep at least wal_keep_size if that's set */ + if (wal_keep_size_mb > 0) { - /* avoid underflow, don't go below 1 */ - if (currSegNo <= wal_keep_segments) - segno = 1; - else - segno = currSegNo - wal_keep_segments; + uint64 keep_segs; + + keep_segs = ConvertToXSegs(wal_keep_size_mb, wal_segment_size); + if (currSegNo - segno < keep_segs) + { + /* avoid underflow, don't go below 1 */ + if (currSegNo <= keep_segs) + segno = 1; + else + segno = currSegNo - keep_segs; + } } /* don't delete WAL segments newer than the calculated segment */ @@ -11328,7 +11334,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 * of those mechanisms, so it's up to the user to ensure that he gets all * the required WAL. diff --git a/src/backend/replication/slotfuncs.c b/src/backend/replication/slotfuncs.c index 9fe147bf44..f88694672f 100644 --- a/src/backend/replication/slotfuncs.c +++ b/src/backend/replication/slotfuncs.c @@ -413,19 +413,20 @@ pg_get_replication_slots(PG_FUNCTION_ARGS) else { XLogSegNo targetSeg; - XLogSegNo keepSegs; + uint64 slotKeepSegs; + uint64 keepSegs; XLogSegNo failSeg; XLogRecPtr failLSN; XLByteToSeg(slot_contents.data.restart_lsn, targetSeg, wal_segment_size); - /* 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); + /* determine how many segments slots can be kept by slots */ + slotKeepSegs = XLogMBVarToSegs(max_slot_wal_keep_size_mb, wal_segment_size); + /* ditto for wal_keep_size */ + keepSegs = XLogMBVarToSegs(wal_keep_size_mb, wal_segment_size); /* if currpos reaches failLSN, we lose our segment */ - failSeg = targetSeg + keepSegs + 1; + failSeg = targetSeg + Max(slotKeepSegs, keepSegs) + 1; XLogSegNoOffsetToRecPtr(failSeg, 0, wal_segment_size, failLSN); values[i++] = Int64GetDatum(failLSN - currlsn); diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 031ca0327f..8b71d16f92 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -2631,12 +2631,13 @@ static struct config_int ConfigureNamesInt[] = }, { - {"wal_keep_segments", PGC_SIGHUP, REPLICATION_SENDING, - gettext_noop("Sets the number of WAL files held for standby servers."), - NULL + {"wal_keep_size", PGC_SIGHUP, REPLICATION_SENDING, + gettext_noop("Sets the size of WAL files held for standby servers."), + NULL, + GUC_UNIT_MB }, - &wal_keep_segments, - 0, 0, INT_MAX, + &wal_keep_size_mb, + 0, 0, MAX_KILOBYTES, NULL, NULL, NULL }, diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index e430e33c7b..90beb87061 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -288,7 +288,7 @@ #max_wal_senders = 10 # max number of walsender processes # (change requires restart) -#wal_keep_segments = 0 # in logfile segments; 0 disables +#wal_keep_size = 0 # in megabytes; 0 disables #max_slot_wal_keep_size = -1 # in megabytes; -1 disables #wal_sender_timeout = 60s # in milliseconds; 0 disables diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm index 7516af7a01..41ed7d4b3b 100644 --- a/src/bin/pg_rewind/t/RewindTest.pm +++ b/src/bin/pg_rewind/t/RewindTest.pm @@ -135,11 +135,11 @@ sub setup_cluster extra => $extra, auth_extra => [ '--create-role', 'rewind_user' ]); - # Set wal_keep_segments to prevent WAL segment recycling after enforced + # Set wal_keep_size to prevent WAL segment recycling after enforced # checkpoints in the tests. $node_primary->append_conf( 'postgresql.conf', qq( -wal_keep_segments = 20 +wal_keep_size = 320MB )); return; } diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 5b14334887..456f0d1321 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -107,7 +107,7 @@ extern bool reachedConsistency; extern int wal_segment_size; extern int min_wal_size_mb; extern int max_wal_size_mb; -extern int wal_keep_segments; +extern int wal_keep_size_mb; extern int max_slot_wal_keep_size_mb; extern int XLOGbuffers; extern int XLogArchiveTimeout; @@ -272,7 +272,7 @@ typedef enum WALAvailability WALAVAIL_INVALID_LSN, /* parameter error */ WALAVAIL_RESERVED, /* WAL segment is within max_wal_size */ WALAVAIL_EXTENDED, /* WAL segment is reserved by a slot or - * wal_keep_segments */ + * wal_keep_size */ WALAVAIL_UNRESERVED, /* no longer reserved, but not removed yet */ WALAVAIL_REMOVED /* WAL segment has been removed */ } WALAvailability; diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl index 1fced12fca..20f4a1bbc3 100644 --- a/src/test/recovery/t/019_replslot_limit.pl +++ b/src/test/recovery/t/019_replslot_limit.pl @@ -116,19 +116,19 @@ $start_lsn = $node_primary->lsn('write'); $node_primary->wait_for_catchup($node_standby, 'replay', $start_lsn); $node_standby->stop; -# wal_keep_segments overrides max_slot_wal_keep_size +# wal_keep_size overrides max_slot_wal_keep_size $result = $node_primary->safe_psql('postgres', - "ALTER SYSTEM SET wal_keep_segments to 8; SELECT pg_reload_conf();"); + "ALTER SYSTEM SET wal_keep_size to '8MB'; SELECT pg_reload_conf();"); # Advance WAL again then checkpoint, reducing remain by 6 MB. advance_wal($node_primary, 6); $result = $node_primary->safe_psql('postgres', "SELECT wal_status as remain FROM pg_replication_slots WHERE slot_name = 'rep1'" ); is($result, "extended", - 'check that wal_keep_segments overrides max_slot_wal_keep_size'); -# restore wal_keep_segments + 'check that wal_keep_size overrides max_slot_wal_keep_size'); +# restore wal_keep_size $result = $node_primary->safe_psql('postgres', - "ALTER SYSTEM SET wal_keep_segments to 0; SELECT pg_reload_conf();"); + "ALTER SYSTEM SET wal_keep_size to 0; SELECT pg_reload_conf();"); # The standby can reconnect to primary $node_standby->start;

view thread (24+ messages)  latest in thread

Message-ID: <d8d60d1a-5233-cbcc-099d-ee34033ea619@oss.nttdata.com>
Permalink:  ../d8d60d1a-5233-cbcc-099d-ee34033ea619@oss.nttdata.com/
Also on:    postgresql.org/message-id/d8d60d1a-5233-cbcc-099d-ee34033ea619@oss.nttdata.com

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: masao.fujii@oss.nttdata.com, horikyota.ntt@gmail.com, david@pgmasters.net, alvherre@2ndquadrant.com
  Subject: Re: max_slot_wal_keep_size and wal_keep_segments
  In-Reply-To: <d8d60d1a-5233-cbcc-099d-ee34033ea619@oss.nttdata.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox