Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jR9sZ-0006ir-Ky for pgsql-bugs@arkaria.postgresql.org; Wed, 22 Apr 2020 07:33:09 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1jR9sY-0000nQ-70 for pgsql-bugs@arkaria.postgresql.org; Wed, 22 Apr 2020 07:33:06 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jR9sW-0000nJ-E7 for pgsql-bugs@lists.postgresql.org; Wed, 22 Apr 2020 07:33:06 +0000 Received: from out1-smtp.messagingengine.com ([66.111.4.25]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1jR9sM-0005du-7j for pgsql-bugs@lists.postgresql.org; Wed, 22 Apr 2020 07:33:04 +0000 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 28D515C012D; Wed, 22 Apr 2020 03:32:51 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Wed, 22 Apr 2020 03:32:51 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=paquier.xyz; h= date:from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm1; bh=kFgHp0gUfImRqKBQZPMznEkT2rj nSw/nArz4VgNnJg4=; b=ZQRaG7mWDQ/7Q6T+QDU9ByQqvzSUMU8YHuZLTAryEs3 pHPv9t8z/SpIB9gOjKsMRzJQVOoTHAh5X4L8ruTNHvxDCvm4MfBOyhkDKaAv62BI cwXY5oMedDmMrDrA/MZErrWCqkJ2keat8jzmUhOpx+jlDa2fnaqkGv+GA5OvK3Jz VMNTLvsaVfusljlAqQbS+45gbs0N9sI3sYIlXPqNcI53zbCF/+Hrlj8nUwgEdvpb tFY0dSqrm8vZ284AHdjeJblueGONOjnmMsLxzQ4CFNDZ95u6GvPBGl8XMGkSfub/ DZIcUqKcIAnepx11PhkC6rqmN+LY1y43Pv9aCtk2/xw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm2; bh=kFgHp0 gUfImRqKBQZPMznEkT2rjnSw/nArz4VgNnJg4=; b=TOGDKZKXrnO+oGk6nLY1Xt 0k7XupqQKQnzhrcmP3ftX+1Htqim4kPWIAWY1Rr0Ewak8wOuWalzwW2ZnSa/jLwg PKSH7Xtn/It1/afaTASCPMZ2Pc91JbWhRITm0PYMEiszAgtT9huoDNjFVbYAYf5S Mokk6bwXPx2RL70YT1IdLbInyCRcgzIuDN3pxvXAiYy+wSrmyv1gRcLq9WyernrA 3F7a9oVSY2GeDkkFr0hpdJ9G+QHqEC7g0uDzKh1I038YmyOO1l6xlS3xuGJc+HpZ VzCTz4UicKh/27IF5FGNxVTDOJ3AtjxkTBrv8OWBGfL9nrOL5L5Th0/a/DIfRJ8A == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrgeeigdduudelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucgfrhhlucfvnfffucdljedtmdenucfjughrpeffhf fvuffkfhggtggujgesghdtreertddtvdenucfhrhhomhepofhitghhrggvlhcurfgrqhhu ihgvrhcuoehmihgthhgrvghlsehprghquhhivghrrdighiiiqeenucfkphepudduuddrud dtvddrudektddrudekheenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgr ihhlfhhrohhmpehmihgthhgrvghlsehprghquhhivghrrdighiii X-ME-Proxy: Received: from paquier.xyz (ee0822lan1.rev.em-net.ne.jp [111.102.180.185]) by mail.messagingengine.com (Postfix) with ESMTPA id AC05B328006C; Wed, 22 Apr 2020 03:32:32 -0400 (EDT) Date: Wed, 22 Apr 2020 16:32:23 +0900 From: Michael Paquier To: Jehan-Guillaume de Rorthais Cc: Kyotaro Horiguchi , masao.fujii@oss.nttdata.com, pgsql-bugs@lists.postgresql.org Subject: Re: [BUG] non archived WAL removed during production crash recovery Message-ID: <20200422073223.GG33034@paquier.xyz> References: <20200417065043.GC350229@paquier.xyz> <20200417153304.01a226cc@firost> <20200418092611.GD350229@paquier.xyz> <20200420.160231.719176619682386260.horikyota.ntt@gmail.com> <20200420073444.GA77439@paquier.xyz> <20200420142235.1536a73b@firost> <20200421021501.GE77439@paquier.xyz> <20200422004121.516a5a32@firost> <20200422011935.GF33034@paquier.xyz> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="451BZW+OUuJBCAYj" Content-Disposition: inline In-Reply-To: <20200422011935.GF33034@paquier.xyz> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --451BZW+OUuJBCAYj Content-Type: multipart/mixed; boundary="9iyR+p8Z2cn535Lj" Content-Disposition: inline --9iyR+p8Z2cn535Lj Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Apr 22, 2020 at 10:19:35AM +0900, Michael Paquier wrote: > You are right here. The restriction can be removed, and I have > checked that the test from v8 is able to pass on my Windows dev VM. Attached are versions for each branch down to 9.5. While working on the backpatch, I have not found major conflicts except one thing: up to 10, Postgres does WAL segment recycling after two completed checkpoints, and the 8th test of the script relies on the behavior of 11~ of one completed checkpoint (first .ready file present in the cold backup but removed removed from $standby1). I have taken the simplest approach to fix the test by checking that the .ready file actually exists, while the rest of the test remains the same. It is worth noting that for 9.5 and 9.6 the test had compatibility issues with the renaming of pg_xlog to pg_wal, including paths and functions. The calls to poll_query_until() also needed tweaks, but I got the tests to work. -- Michael --9iyR+p8Z2cn535Lj Content-Type: text/x-diff; charset=iso-8859-1 Content-Disposition: attachment; filename="0001-Fix-handling-of-WAL-segments-ready-to-be-arch-master.patch" Content-Transfer-Encoding: quoted-printable =46rom 8734b386e1e077be4d6e5353c08ab5a40b17e60f Mon Sep 17 00:00:00 2001 =46rom: Michael Paquier Date: Wed, 22 Apr 2020 14:48:25 +0900 Subject: [PATCH] Fix handling of WAL segments ready to be archived during crash recovery MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit 78ea8b5 has fixed an issue related to the recycling of WAL segments on standbys depending on archive_mode. However, it has introduced a regression with the handling of WAL segments ready to be archived during crash recovery, causing those files to be recycled without getting archived. This commit fixes the regression by tracking in shared memory if a live cluster is either in crash recovery or archive recovery as the handling of WAL segments ready to be archived is different in both cases (those WAL segments should not be removed during crash recovery), and by using this new shared memory state to decide if a segment can be recycled or not. Previously, it was not possible to know if a cluster was in crash recovery or archive recovery as the shared state was able to track only if recovery was happening or not, leading to the problem. A set of TAP tests is added to close the gap here, making sure that WAL segments ready to be archived are correctly handled when a cluster is in archive or crash recovery with archive_mode set to "on" or "always", for both standby and primary. Reported-by: Beno=EEt Lobr=E9au Author: Jehan-Guillaume de Rorthais Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost Backpatch-through: 9.5 --- src/include/access/xlog.h | 9 + src/backend/access/transam/xlog.c | 57 +++++- src/backend/access/transam/xlogarchive.c | 21 ++- src/test/recovery/t/020_archive_status.pl | 214 ++++++++++++++++++++++ src/tools/pgindent/typedefs.list | 1 + 5 files changed, 286 insertions(+), 16 deletions(-) create mode 100644 src/test/recovery/t/020_archive_status.pl diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index f60ed2d36c..0a12afb59e 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -166,6 +166,14 @@ typedef enum WalLevel WAL_LEVEL_LOGICAL } WalLevel; =20 +/* Recovery states */ +typedef enum RecoveryState +{ + RECOVERY_STATE_CRASH =3D 0, /* crash recovery */ + RECOVERY_STATE_ARCHIVE, /* archive recovery */ + RECOVERY_STATE_DONE /* currently in production */ +} RecoveryState; + extern PGDLLIMPORT int wal_level; =20 /* Is WAL archiving enabled (always or only while server is running normal= ly)? */ @@ -291,6 +299,7 @@ extern const char *xlog_identify(uint8 info); extern void issue_xlog_fsync(int fd, XLogSegNo segno); =20 extern bool RecoveryInProgress(void); +extern RecoveryState GetRecoveryState(void); extern bool HotStandbyActive(void); extern bool HotStandbyActiveInReplay(void); extern bool XLogInsertAllowed(void); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam= /xlog.c index 11e32733c4..009a071276 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -221,8 +221,9 @@ static TimeLineID receiveTLI =3D 0; static bool lastFullPageWrites; =20 /* - * Local copy of SharedRecoveryInProgress variable. True actually means "n= ot - * known, need to check the shared state". + * Local copy of the state tracked by SharedRecoveryState in shared memory, + * It is false if SharedRecoveryState is RECOVERY_STATE_DONE. True actual= ly + * means "not known, need to check the shared state". */ static bool LocalRecoveryInProgress =3D true; =20 @@ -653,10 +654,10 @@ typedef struct XLogCtlData TimeLineID PrevTimeLineID; =20 /* - * SharedRecoveryInProgress indicates if we're still in crash or archive + * SharedRecoveryState indicates if we're still in crash or archive * recovery. Protected by info_lck. */ - bool SharedRecoveryInProgress; + RecoveryState SharedRecoveryState; =20 /* * SharedHotStandbyActive indicates if we allow hot standby queries to be @@ -4434,6 +4435,16 @@ ReadRecord(XLogReaderState *xlogreader, int emode, updateMinRecoveryPoint =3D true; =20 UpdateControlFile(); + + /* + * We update SharedRecoveryState while holding the lock on + * ControlFileLock so both states are consistent in shared + * memory. + */ + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + LWLockRelease(ControlFileLock); =20 CheckRecoveryConsistency(); @@ -5166,7 +5177,7 @@ XLOGShmemInit(void) * in additional info.) */ XLogCtl->XLogCacheBlck =3D XLOGbuffers - 1; - XLogCtl->SharedRecoveryInProgress =3D true; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; XLogCtl->SharedHotStandbyActive =3D false; XLogCtl->SharedPromoteIsTriggered =3D false; XLogCtl->WalWriterSleeping =3D false; @@ -6871,7 +6882,13 @@ StartupXLOG(void) */ dbstate_at_startup =3D ControlFile->state; if (InArchiveRecovery) + { ControlFile->state =3D DB_IN_ARCHIVE_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + } else { ereport(LOG, @@ -6884,6 +6901,10 @@ StartupXLOG(void) ControlFile->checkPointCopy.ThisTimeLineID, recoveryTargetTLI))); ControlFile->state =3D DB_IN_CRASH_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; + SpinLockRelease(&XLogCtl->info_lck); } ControlFile->checkPoint =3D checkPointLoc; ControlFile->checkPointCopy =3D checkPoint; @@ -7911,7 +7932,7 @@ StartupXLOG(void) ControlFile->time =3D (pg_time_t) time(NULL); =20 SpinLockAcquire(&XLogCtl->info_lck); - XLogCtl->SharedRecoveryInProgress =3D false; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_DONE; SpinLockRelease(&XLogCtl->info_lck); =20 UpdateControlFile(); @@ -8057,7 +8078,7 @@ RecoveryInProgress(void) */ volatile XLogCtlData *xlogctl =3D XLogCtl; =20 - LocalRecoveryInProgress =3D xlogctl->SharedRecoveryInProgress; + LocalRecoveryInProgress =3D (xlogctl->SharedRecoveryState !=3D RECOVERY_= STATE_DONE); =20 /* * Initialize TimeLineID and RedoRecPtr when we discover that recovery @@ -8069,8 +8090,8 @@ RecoveryInProgress(void) { /* * If we just exited recovery, make sure we read TimeLineID and - * RedoRecPtr after SharedRecoveryInProgress (for machines with - * weak memory ordering). + * RedoRecPtr after SharedRecoveryState (for machines with weak + * memory ordering). */ pg_memory_barrier(); InitXLOGAccess(); @@ -8086,6 +8107,24 @@ RecoveryInProgress(void) } } =20 +/* + * Returns current recovery state from shared memory. + * + * This returned state is kept consistent with the contents of the control + * file. See details about the possible values of RecoveryState in xlog.h. + */ +RecoveryState +GetRecoveryState(void) +{ + RecoveryState retval; + + SpinLockAcquire(&XLogCtl->info_lck); + retval =3D XLogCtl->SharedRecoveryState; + SpinLockRelease(&XLogCtl->info_lck); + + return retval; +} + /* * Is HotStandby active yet? This is only important in special backends * since normal backends won't ever be able to connect until this returns diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/= transam/xlogarchive.c index d62c12310a..55becd65d4 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -572,18 +572,25 @@ XLogArchiveCheckDone(const char *xlog) { char archiveStatusPath[MAXPGPATH]; struct stat stat_buf; - bool inRecovery =3D RecoveryInProgress(); + + /* The file is always deletable if archive_mode is "off". */ + if (!XLogArchivingActive()) + return true; =20 /* - * The file is always deletable if archive_mode is "off". On standbys - * archiving is disabled if archive_mode is "on", and enabled with - * "always". On a primary, archiving is enabled if archive_mode is "on" - * or "always". + * During archive recovery, the file is deletable if archive_mode is not + * "always". */ - if (!((XLogArchivingActive() && !inRecovery) || - (XLogArchivingAlways() && inRecovery))) + if (!XLogArchivingAlways() && + GetRecoveryState() =3D=3D RECOVERY_STATE_ARCHIVE) return true; =20 + /* + * At this point of the logic, note that we are either a primary with + * archive_mode set to "on" or "always", or a standby with archive_mode + * set to "always". + */ + /* First check for .done --- this means archiver is done with it */ StatusFilePath(archiveStatusPath, xlog, ".done"); if (stat(archiveStatusPath, &stat_buf) =3D=3D 0) diff --git a/src/test/recovery/t/020_archive_status.pl b/src/test/recovery/= t/020_archive_status.pl new file mode 100644 index 0000000000..2369b34712 --- /dev/null +++ b/src/test/recovery/t/020_archive_status.pl @@ -0,0 +1,214 @@ +# +# Tests related to WAL archiving and recovery. +# +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests =3D> 16; +use Config; + +my $primary =3D get_new_node('master'); +$primary->init( + has_archiving =3D> 1, + allows_streaming =3D> 1); +$primary->append_conf('postgresql.conf', 'autovacuum =3D off'); +$primary->start; +my $primary_data =3D $primary->data_dir; + +# Temporarily use an archive_command value to make the archiver fail, +# knowing that archiving is enabled. Note that we cannot use a command +# that does not exist as in this case the archiver process would just exit +# without reporting the failure to pg_stat_archiver. This also cannot +# use a plain "false" as that's unportable on Windows. So, instead, as +# a portable solution, use an archive command based on a command known to +# work but will fail: copy with an incorrect original path. +my $incorrect_command =3D + $TestLib::windows_os + ? qq{copy "%p_does_not_exist" "%f_does_not_exist"} + : qq{cp "%p_does_not_exist" "%f_does_not_exist"}; +$primary->safe_psql( + 'postgres', qq{ + ALTER SYSTEM SET archive_command TO '$incorrect_command'; + SELECT pg_reload_conf(); +}); + +# Save the WAL segment currently in use and switch to a new segment. +# This will be used to track the activity of the archiver. +my $segment_name_1 =3D $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); +my $segment_path_1 =3D "pg_wal/archive_status/$segment_name_1"; +my $segment_path_1_ready =3D "$segment_path_1.ready"; +my $segment_path_1_done =3D "$segment_path_1.done"; +$primary->safe_psql( + 'postgres', q{ + CREATE TABLE mine AS SELECT generate_series(1,10) AS x; + SELECT pg_switch_wal(); + CHECKPOINT; +}); + +# Wait for an archive failure. +$primary->poll_query_until('postgres', + q{SELECT failed_count > 0 FROM pg_stat_archiver}, 't') + or die "Timed out while waiting for archiving to fail"; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file exists for WAL segment $segment_name_1 waiting to be archive= d" +); +ok( !-f "$primary_data/$segment_path_1_done", + ".done file does not exist for WAL segment $segment_name_1 waiting to be = archived" +); + +is( $primary->safe_psql( + 'postgres', q{ + SELECT archived_count, last_failed_wal + FROM pg_stat_archiver + }), + "0|$segment_name_1", + 'pg_stat_archiver failed to archive $segment_name_1'); + +# Crash the cluster for the next test in charge of checking that non-archi= ved +# WAL segments are not removed. +$primary->stop('immediate'); + +# Recovery tests for the archiving with a standby partially check +# the recovery behavior when restoring a backup taken using a +# snapshot with no pg_start/stop_backup. In this situation, +# the recovered standby should enter first crash recovery then +# switch to regular archive recovery. Note that the base backup +# is taken here so as archive_command will fail. This is necessary +# for the assumptions of the tests done with the standbys below. +$primary->backup_fs_cold('backup'); + +$primary->start; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 still exists after crash rec= overy on primary" +); + +# Allow WAL archiving again and wait for a success. +$primary->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); + +$primary->poll_query_until('postgres', + q{SELECT archived_count FROM pg_stat_archiver}, '1') + or die "Timed out while waiting for archiving to finish"; + +ok(!-f "$primary_data/$segment_path_1_ready", + ".ready file for archived WAL segment $segment_name_1 removed"); + +ok(-f "$primary_data/$segment_path_1_done", + ".done file for archived WAL segment $segment_name_1 exists"); + +is( $primary->safe_psql( + 'postgres', q{ SELECT last_archived_wal FROM pg_stat_archiver }), + $segment_name_1, + 'archive success reported in pg_stat_archiver for WAL segment $segment_na= me_1' +); + +# Create some WAL activity and a new checkpoint so as the next standby can +# create a restartpoint. As this standby starts in crash recovery because +# of the cold backup taken previously, it needs a clean restartpoint to de= al +# with existing status files. +my $segment_name_2 =3D $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); +my $segment_path_2 =3D "pg_wal/archive_status/$segment_name_2"; +my $segment_path_2_ready =3D "$segment_path_2.ready"; +my $segment_path_2_done =3D "$segment_path_2.done"; +$primary->safe_psql( + 'postgres', q{ + INSERT INTO mine SELECT generate_series(10,20) AS x; + SELECT pg_switch_wal(); + CHECKPOINT; +}); + +$primary->poll_query_until('postgres', + q{ SELECT last_archived_wal FROM pg_stat_archiver }, + $segment_name_2) + or die "Timed out while waiting for archiving to finish"; + +# Test standby with archive_mode =3D on. +my $standby1 =3D get_new_node('standby'); +$standby1->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby1->append_conf('postgresql.conf', "archive_mode =3D on"); +my $standby1_data =3D $standby1->data_dir; +$standby1->start; +$standby1->safe_psql('postgres', q{CHECKPOINT}); + +# Recovery with archive_mode=3Don does not keep .ready signal files inheri= ted +# from backup. Note that this WAL segment existed in the backup. +ok( !-f "$standby1_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 present in backup got remove= d with archive_mode=3Don on standby" +); + +# Recovery with archive_mode=3Don should not create .ready files. +# Note that this segment did not exist in the backup. +ok( !-f "$standby1_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 not created on standby when = archive_mode=3Don on standby" +); + +# Recovery with archive_mode =3D on creates .done files. +ok( -f "$standby1_data/$segment_path_2_done", + ".done file for WAL segment $segment_name_2 created when archive_mode=3Do= n on standby" +); + +# Test recovery with archive_mode =3D always, which should always keep +# .ready files if archiving is enabled, though here we want the archive +# command to fail to persist the .ready files. Note that this node +# has inherited the archive command of the previous cold backup that +# will cause archiving failures. +my $standby2 =3D get_new_node('standby2'); +$standby2->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby2->append_conf('postgresql.conf', 'archive_mode =3D always'); +my $standby2_data =3D $standby2->data_dir; +$standby2->start; + +$standby2->safe_psql('postgres', q{CHECKPOINT}); + +ok( -f "$standby2_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 existing in backup is kept w= ith archive_mode=3Dalways on standby" +); + +ok( -f "$standby2_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 created with archive_mode=3D= always on standby" +); + +# Reset statistics of the archiver for the next checks. +$standby2->safe_psql('postgres', q{SELECT pg_stat_reset_shared('archiver')= }); + +# Now crash the cluster to check that recovery step does not +# remove non-archived WAL segments on a standby where archiving +# is enabled. +$standby2->stop('immediate'); +$standby2->start; + +ok( -f "$standby2_data/$segment_path_1_ready", + "WAL segment still ready to archive after crash recovery on standby with = archive_mode=3Dalways" +); + +# Allow WAL archiving again, and wait for the segments to be archived. +$standby2->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); +$standby2->poll_query_until('postgres', + q{SELECT last_archived_wal FROM pg_stat_archiver}, + $segment_name_2) + or die "Timed out while waiting for archiving to finish"; + +is( $standby2->safe_psql( + 'postgres', q{SELECT archived_count FROM pg_stat_archiver}), + '2', + 'correct number of WAL segments archived from standby'); + +ok( !-f "$standby2_data/$segment_path_1_ready" + && !-f "$standby2_data/$segment_path_2_ready", + ".ready files removed after archive success with archive_mode=3Dalways on= standby" +); + +ok( -f "$standby2_data/$segment_path_1_done" + && -f "$standby2_data/$segment_path_2_done", + ".done files created after archive success with archive_mode=3Dalways on = standby" +); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs= =2Elist index 525d58e7f0..b319266bb3 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -1983,6 +1983,7 @@ RecordCacheEntry RecordCompareData RecordIOData RecoveryLockListsEntry +RecoveryState RecoveryTargetTimeLineGoal RecoveryTargetType RectBox --=20 2.26.0 --9iyR+p8Z2cn535Lj Content-Type: text/x-diff; charset=iso-8859-1 Content-Disposition: attachment; filename="0001-Fix-handling-of-WAL-segments-ready-to-be-archived-12.patch" Content-Transfer-Encoding: quoted-printable =46rom 94fd9d2a5791a6ffe8bb3ace75a7e263fd91ff1c Mon Sep 17 00:00:00 2001 =46rom: Michael Paquier Date: Wed, 22 Apr 2020 14:48:25 +0900 Subject: [PATCH] Fix handling of WAL segments ready to be archived during crash recovery MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit 78ea8b5 has fixed an issue related to the recycling of WAL segments on standbys depending on archive_mode. However, it has introduced a regression with the handling of WAL segments ready to be archived during crash recovery, causing those files to be recycled without getting archived. This commit fixes the regression by tracking in shared memory if a live cluster is either in crash recovery or archive recovery as the handling of WAL segments ready to be archived is different in both cases (those WAL segments should not be removed during crash recovery), and by using this new shared memory state to decide if a segment can be recycled or not. Previously, it was not possible to know if a cluster was in crash recovery or archive recovery as the shared state was able to track only if recovery was happening or not, leading to the problem. A set of TAP tests is added to close the gap here, making sure that WAL segments ready to be archived are correctly handled when a cluster is in archive or crash recovery with archive_mode set to "on" or "always", for both standby and primary. Reported-by: Beno=EEt Lobr=E9au Author: Jehan-Guillaume de Rorthais Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost Backpatch-through: 9.5 --- src/include/access/xlog.h | 9 + src/backend/access/transam/xlog.c | 57 +++++- src/backend/access/transam/xlogarchive.c | 21 ++- src/test/recovery/t/020_archive_status.pl | 214 ++++++++++++++++++++++ 4 files changed, 285 insertions(+), 16 deletions(-) create mode 100644 src/test/recovery/t/020_archive_status.pl diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index d519252aad..d6ab348cb6 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -164,6 +164,14 @@ typedef enum WalLevel WAL_LEVEL_LOGICAL } WalLevel; =20 +/* Recovery states */ +typedef enum RecoveryState +{ + RECOVERY_STATE_CRASH =3D 0, /* crash recovery */ + RECOVERY_STATE_ARCHIVE, /* archive recovery */ + RECOVERY_STATE_DONE /* currently in production */ +} RecoveryState; + extern PGDLLIMPORT int wal_level; =20 /* Is WAL archiving enabled (always or only while server is running normal= ly)? */ @@ -277,6 +285,7 @@ extern const char *xlog_identify(uint8 info); extern void issue_xlog_fsync(int fd, XLogSegNo segno); =20 extern bool RecoveryInProgress(void); +extern RecoveryState GetRecoveryState(void); extern bool HotStandbyActive(void); extern bool HotStandbyActiveInReplay(void); extern bool XLogInsertAllowed(void); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam= /xlog.c index 05cb6aa022..f181c4d144 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -217,8 +217,9 @@ static TimeLineID receiveTLI =3D 0; static bool lastFullPageWrites; =20 /* - * Local copy of SharedRecoveryInProgress variable. True actually means "n= ot - * known, need to check the shared state". + * Local copy of the state tracked by SharedRecoveryState in shared memory, + * It is false if SharedRecoveryState is RECOVERY_STATE_DONE. True actual= ly + * means "not known, need to check the shared state". */ static bool LocalRecoveryInProgress =3D true; =20 @@ -643,10 +644,10 @@ typedef struct XLogCtlData TimeLineID PrevTimeLineID; =20 /* - * SharedRecoveryInProgress indicates if we're still in crash or archive + * SharedRecoveryState indicates if we're still in crash or archive * recovery. Protected by info_lck. */ - bool SharedRecoveryInProgress; + RecoveryState SharedRecoveryState; =20 /* * SharedHotStandbyActive indicates if we're still in crash or archive @@ -4357,6 +4358,16 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr R= ecPtr, int emode, updateMinRecoveryPoint =3D true; =20 UpdateControlFile(); + + /* + * We update SharedRecoveryState while holding the lock on + * ControlFileLock so both states are consistent in shared + * memory. + */ + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + LWLockRelease(ControlFileLock); =20 CheckRecoveryConsistency(); @@ -5069,7 +5080,7 @@ XLOGShmemInit(void) * in additional info.) */ XLogCtl->XLogCacheBlck =3D XLOGbuffers - 1; - XLogCtl->SharedRecoveryInProgress =3D true; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; XLogCtl->SharedHotStandbyActive =3D false; XLogCtl->WalWriterSleeping =3D false; =20 @@ -6758,7 +6769,13 @@ StartupXLOG(void) */ dbstate_at_startup =3D ControlFile->state; if (InArchiveRecovery) + { ControlFile->state =3D DB_IN_ARCHIVE_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + } else { ereport(LOG, @@ -6771,6 +6788,10 @@ StartupXLOG(void) ControlFile->checkPointCopy.ThisTimeLineID, recoveryTargetTLI))); ControlFile->state =3D DB_IN_CRASH_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; + SpinLockRelease(&XLogCtl->info_lck); } ControlFile->checkPoint =3D checkPointLoc; ControlFile->checkPointCopy =3D checkPoint; @@ -7785,7 +7806,7 @@ StartupXLOG(void) ControlFile->time =3D (pg_time_t) time(NULL); =20 SpinLockAcquire(&XLogCtl->info_lck); - XLogCtl->SharedRecoveryInProgress =3D false; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_DONE; SpinLockRelease(&XLogCtl->info_lck); =20 UpdateControlFile(); @@ -7931,7 +7952,7 @@ RecoveryInProgress(void) */ volatile XLogCtlData *xlogctl =3D XLogCtl; =20 - LocalRecoveryInProgress =3D xlogctl->SharedRecoveryInProgress; + LocalRecoveryInProgress =3D (xlogctl->SharedRecoveryState !=3D RECOVERY_= STATE_DONE); =20 /* * Initialize TimeLineID and RedoRecPtr when we discover that recovery @@ -7943,8 +7964,8 @@ RecoveryInProgress(void) { /* * If we just exited recovery, make sure we read TimeLineID and - * RedoRecPtr after SharedRecoveryInProgress (for machines with - * weak memory ordering). + * RedoRecPtr after SharedRecoveryState (for machines with weak + * memory ordering). */ pg_memory_barrier(); InitXLOGAccess(); @@ -7960,6 +7981,24 @@ RecoveryInProgress(void) } } =20 +/* + * Returns current recovery state from shared memory. + * + * This returned state is kept consistent with the contents of the control + * file. See details about the possible values of RecoveryState in xlog.h. + */ +RecoveryState +GetRecoveryState(void) +{ + RecoveryState retval; + + SpinLockAcquire(&XLogCtl->info_lck); + retval =3D XLogCtl->SharedRecoveryState; + SpinLockRelease(&XLogCtl->info_lck); + + return retval; +} + /* * Is HotStandby active yet? This is only important in special backends * since normal backends won't ever be able to connect until this returns diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/= transam/xlogarchive.c index e14bcf8ea6..b3e5338766 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -617,18 +617,25 @@ XLogArchiveCheckDone(const char *xlog) { char archiveStatusPath[MAXPGPATH]; struct stat stat_buf; - bool inRecovery =3D RecoveryInProgress(); + + /* The file is always deletable if archive_mode is "off". */ + if (!XLogArchivingActive()) + return true; =20 /* - * The file is always deletable if archive_mode is "off". On standbys - * archiving is disabled if archive_mode is "on", and enabled with - * "always". On a primary, archiving is enabled if archive_mode is "on" - * or "always". + * During archive recovery, the file is deletable if archive_mode is not + * "always". */ - if (!((XLogArchivingActive() && !inRecovery) || - (XLogArchivingAlways() && inRecovery))) + if (!XLogArchivingAlways() && + GetRecoveryState() =3D=3D RECOVERY_STATE_ARCHIVE) return true; =20 + /* + * At this point of the logic, note that we are either a primary with + * archive_mode set to "on" or "always", or a standby with archive_mode + * set to "always". + */ + /* First check for .done --- this means archiver is done with it */ StatusFilePath(archiveStatusPath, xlog, ".done"); if (stat(archiveStatusPath, &stat_buf) =3D=3D 0) diff --git a/src/test/recovery/t/020_archive_status.pl b/src/test/recovery/= t/020_archive_status.pl new file mode 100644 index 0000000000..2369b34712 --- /dev/null +++ b/src/test/recovery/t/020_archive_status.pl @@ -0,0 +1,214 @@ +# +# Tests related to WAL archiving and recovery. +# +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests =3D> 16; +use Config; + +my $primary =3D get_new_node('master'); +$primary->init( + has_archiving =3D> 1, + allows_streaming =3D> 1); +$primary->append_conf('postgresql.conf', 'autovacuum =3D off'); +$primary->start; +my $primary_data =3D $primary->data_dir; + +# Temporarily use an archive_command value to make the archiver fail, +# knowing that archiving is enabled. Note that we cannot use a command +# that does not exist as in this case the archiver process would just exit +# without reporting the failure to pg_stat_archiver. This also cannot +# use a plain "false" as that's unportable on Windows. So, instead, as +# a portable solution, use an archive command based on a command known to +# work but will fail: copy with an incorrect original path. +my $incorrect_command =3D + $TestLib::windows_os + ? qq{copy "%p_does_not_exist" "%f_does_not_exist"} + : qq{cp "%p_does_not_exist" "%f_does_not_exist"}; +$primary->safe_psql( + 'postgres', qq{ + ALTER SYSTEM SET archive_command TO '$incorrect_command'; + SELECT pg_reload_conf(); +}); + +# Save the WAL segment currently in use and switch to a new segment. +# This will be used to track the activity of the archiver. +my $segment_name_1 =3D $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); +my $segment_path_1 =3D "pg_wal/archive_status/$segment_name_1"; +my $segment_path_1_ready =3D "$segment_path_1.ready"; +my $segment_path_1_done =3D "$segment_path_1.done"; +$primary->safe_psql( + 'postgres', q{ + CREATE TABLE mine AS SELECT generate_series(1,10) AS x; + SELECT pg_switch_wal(); + CHECKPOINT; +}); + +# Wait for an archive failure. +$primary->poll_query_until('postgres', + q{SELECT failed_count > 0 FROM pg_stat_archiver}, 't') + or die "Timed out while waiting for archiving to fail"; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file exists for WAL segment $segment_name_1 waiting to be archive= d" +); +ok( !-f "$primary_data/$segment_path_1_done", + ".done file does not exist for WAL segment $segment_name_1 waiting to be = archived" +); + +is( $primary->safe_psql( + 'postgres', q{ + SELECT archived_count, last_failed_wal + FROM pg_stat_archiver + }), + "0|$segment_name_1", + 'pg_stat_archiver failed to archive $segment_name_1'); + +# Crash the cluster for the next test in charge of checking that non-archi= ved +# WAL segments are not removed. +$primary->stop('immediate'); + +# Recovery tests for the archiving with a standby partially check +# the recovery behavior when restoring a backup taken using a +# snapshot with no pg_start/stop_backup. In this situation, +# the recovered standby should enter first crash recovery then +# switch to regular archive recovery. Note that the base backup +# is taken here so as archive_command will fail. This is necessary +# for the assumptions of the tests done with the standbys below. +$primary->backup_fs_cold('backup'); + +$primary->start; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 still exists after crash rec= overy on primary" +); + +# Allow WAL archiving again and wait for a success. +$primary->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); + +$primary->poll_query_until('postgres', + q{SELECT archived_count FROM pg_stat_archiver}, '1') + or die "Timed out while waiting for archiving to finish"; + +ok(!-f "$primary_data/$segment_path_1_ready", + ".ready file for archived WAL segment $segment_name_1 removed"); + +ok(-f "$primary_data/$segment_path_1_done", + ".done file for archived WAL segment $segment_name_1 exists"); + +is( $primary->safe_psql( + 'postgres', q{ SELECT last_archived_wal FROM pg_stat_archiver }), + $segment_name_1, + 'archive success reported in pg_stat_archiver for WAL segment $segment_na= me_1' +); + +# Create some WAL activity and a new checkpoint so as the next standby can +# create a restartpoint. As this standby starts in crash recovery because +# of the cold backup taken previously, it needs a clean restartpoint to de= al +# with existing status files. +my $segment_name_2 =3D $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); +my $segment_path_2 =3D "pg_wal/archive_status/$segment_name_2"; +my $segment_path_2_ready =3D "$segment_path_2.ready"; +my $segment_path_2_done =3D "$segment_path_2.done"; +$primary->safe_psql( + 'postgres', q{ + INSERT INTO mine SELECT generate_series(10,20) AS x; + SELECT pg_switch_wal(); + CHECKPOINT; +}); + +$primary->poll_query_until('postgres', + q{ SELECT last_archived_wal FROM pg_stat_archiver }, + $segment_name_2) + or die "Timed out while waiting for archiving to finish"; + +# Test standby with archive_mode =3D on. +my $standby1 =3D get_new_node('standby'); +$standby1->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby1->append_conf('postgresql.conf', "archive_mode =3D on"); +my $standby1_data =3D $standby1->data_dir; +$standby1->start; +$standby1->safe_psql('postgres', q{CHECKPOINT}); + +# Recovery with archive_mode=3Don does not keep .ready signal files inheri= ted +# from backup. Note that this WAL segment existed in the backup. +ok( !-f "$standby1_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 present in backup got remove= d with archive_mode=3Don on standby" +); + +# Recovery with archive_mode=3Don should not create .ready files. +# Note that this segment did not exist in the backup. +ok( !-f "$standby1_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 not created on standby when = archive_mode=3Don on standby" +); + +# Recovery with archive_mode =3D on creates .done files. +ok( -f "$standby1_data/$segment_path_2_done", + ".done file for WAL segment $segment_name_2 created when archive_mode=3Do= n on standby" +); + +# Test recovery with archive_mode =3D always, which should always keep +# .ready files if archiving is enabled, though here we want the archive +# command to fail to persist the .ready files. Note that this node +# has inherited the archive command of the previous cold backup that +# will cause archiving failures. +my $standby2 =3D get_new_node('standby2'); +$standby2->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby2->append_conf('postgresql.conf', 'archive_mode =3D always'); +my $standby2_data =3D $standby2->data_dir; +$standby2->start; + +$standby2->safe_psql('postgres', q{CHECKPOINT}); + +ok( -f "$standby2_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 existing in backup is kept w= ith archive_mode=3Dalways on standby" +); + +ok( -f "$standby2_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 created with archive_mode=3D= always on standby" +); + +# Reset statistics of the archiver for the next checks. +$standby2->safe_psql('postgres', q{SELECT pg_stat_reset_shared('archiver')= }); + +# Now crash the cluster to check that recovery step does not +# remove non-archived WAL segments on a standby where archiving +# is enabled. +$standby2->stop('immediate'); +$standby2->start; + +ok( -f "$standby2_data/$segment_path_1_ready", + "WAL segment still ready to archive after crash recovery on standby with = archive_mode=3Dalways" +); + +# Allow WAL archiving again, and wait for the segments to be archived. +$standby2->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); +$standby2->poll_query_until('postgres', + q{SELECT last_archived_wal FROM pg_stat_archiver}, + $segment_name_2) + or die "Timed out while waiting for archiving to finish"; + +is( $standby2->safe_psql( + 'postgres', q{SELECT archived_count FROM pg_stat_archiver}), + '2', + 'correct number of WAL segments archived from standby'); + +ok( !-f "$standby2_data/$segment_path_1_ready" + && !-f "$standby2_data/$segment_path_2_ready", + ".ready files removed after archive success with archive_mode=3Dalways on= standby" +); + +ok( -f "$standby2_data/$segment_path_1_done" + && -f "$standby2_data/$segment_path_2_done", + ".done files created after archive success with archive_mode=3Dalways on = standby" +); --=20 2.26.0 --9iyR+p8Z2cn535Lj Content-Type: text/x-diff; charset=iso-8859-1 Content-Disposition: attachment; filename="0001-Fix-handling-of-WAL-segments-ready-to-be-archived-11.patch" Content-Transfer-Encoding: quoted-printable =46rom 9fa0d187375b7ce2ed15b527d42fd78ca1c70d42 Mon Sep 17 00:00:00 2001 =46rom: Michael Paquier Date: Wed, 22 Apr 2020 14:48:25 +0900 Subject: [PATCH] Fix handling of WAL segments ready to be archived during crash recovery MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit 78ea8b5 has fixed an issue related to the recycling of WAL segments on standbys depending on archive_mode. However, it has introduced a regression with the handling of WAL segments ready to be archived during crash recovery, causing those files to be recycled without getting archived. This commit fixes the regression by tracking in shared memory if a live cluster is either in crash recovery or archive recovery as the handling of WAL segments ready to be archived is different in both cases (those WAL segments should not be removed during crash recovery), and by using this new shared memory state to decide if a segment can be recycled or not. Previously, it was not possible to know if a cluster was in crash recovery or archive recovery as the shared state was able to track only if recovery was happening or not, leading to the problem. A set of TAP tests is added to close the gap here, making sure that WAL segments ready to be archived are correctly handled when a cluster is in archive or crash recovery with archive_mode set to "on" or "always", for both standby and primary. Reported-by: Beno=EEt Lobr=E9au Author: Jehan-Guillaume de Rorthais Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost Backpatch-through: 9.5 --- src/include/access/xlog.h | 9 + src/backend/access/transam/xlog.c | 57 +++++- src/backend/access/transam/xlogarchive.c | 21 ++- src/test/recovery/t/020_archive_status.pl | 214 ++++++++++++++++++++++ 4 files changed, 285 insertions(+), 16 deletions(-) create mode 100644 src/test/recovery/t/020_archive_status.pl diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index ef6aabde9f..1d9c87cb82 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -129,6 +129,14 @@ typedef enum WalLevel WAL_LEVEL_LOGICAL } WalLevel; =20 +/* Recovery states */ +typedef enum RecoveryState +{ + RECOVERY_STATE_CRASH =3D 0, /* crash recovery */ + RECOVERY_STATE_ARCHIVE, /* archive recovery */ + RECOVERY_STATE_DONE /* currently in production */ +} RecoveryState; + extern PGDLLIMPORT int wal_level; =20 /* Is WAL archiving enabled (always or only while server is running normal= ly)? */ @@ -243,6 +251,7 @@ extern const char *xlog_identify(uint8 info); extern void issue_xlog_fsync(int fd, XLogSegNo segno); =20 extern bool RecoveryInProgress(void); +extern RecoveryState GetRecoveryState(void); extern bool HotStandbyActive(void); extern bool HotStandbyActiveInReplay(void); extern bool XLogInsertAllowed(void); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam= /xlog.c index 62f87ed174..0017bcf85b 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -211,8 +211,9 @@ static TimeLineID receiveTLI =3D 0; static bool lastFullPageWrites; =20 /* - * Local copy of SharedRecoveryInProgress variable. True actually means "n= ot - * known, need to check the shared state". + * Local copy of the state tracked by SharedRecoveryState in shared memory, + * It is false if SharedRecoveryState is RECOVERY_STATE_DONE. True actual= ly + * means "not known, need to check the shared state". */ static bool LocalRecoveryInProgress =3D true; =20 @@ -637,10 +638,10 @@ typedef struct XLogCtlData char archiveCleanupCommand[MAXPGPATH]; =20 /* - * SharedRecoveryInProgress indicates if we're still in crash or archive + * SharedRecoveryState indicates if we're still in crash or archive * recovery. Protected by info_lck. */ - bool SharedRecoveryInProgress; + RecoveryState SharedRecoveryState; =20 /* * SharedHotStandbyActive indicates if we're still in crash or archive @@ -4306,6 +4307,16 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr R= ecPtr, int emode, updateMinRecoveryPoint =3D true; =20 UpdateControlFile(); + + /* + * We update SharedRecoveryState while holding the lock on + * ControlFileLock so both states are consistent in shared + * memory. + */ + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + LWLockRelease(ControlFileLock); =20 CheckRecoveryConsistency(); @@ -5047,7 +5058,7 @@ XLOGShmemInit(void) * in additional info.) */ XLogCtl->XLogCacheBlck =3D XLOGbuffers - 1; - XLogCtl->SharedRecoveryInProgress =3D true; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; XLogCtl->SharedHotStandbyActive =3D false; XLogCtl->WalWriterSleeping =3D false; =20 @@ -6859,7 +6870,13 @@ StartupXLOG(void) */ dbstate_at_startup =3D ControlFile->state; if (InArchiveRecovery) + { ControlFile->state =3D DB_IN_ARCHIVE_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + } else { ereport(LOG, @@ -6872,6 +6889,10 @@ StartupXLOG(void) ControlFile->checkPointCopy.ThisTimeLineID, recoveryTargetTLI))); ControlFile->state =3D DB_IN_CRASH_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; + SpinLockRelease(&XLogCtl->info_lck); } ControlFile->checkPoint =3D checkPointLoc; ControlFile->checkPointCopy =3D checkPoint; @@ -7896,7 +7917,7 @@ StartupXLOG(void) ControlFile->time =3D (pg_time_t) time(NULL); =20 SpinLockAcquire(&XLogCtl->info_lck); - XLogCtl->SharedRecoveryInProgress =3D false; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_DONE; SpinLockRelease(&XLogCtl->info_lck); =20 UpdateControlFile(); @@ -8042,7 +8063,7 @@ RecoveryInProgress(void) */ volatile XLogCtlData *xlogctl =3D XLogCtl; =20 - LocalRecoveryInProgress =3D xlogctl->SharedRecoveryInProgress; + LocalRecoveryInProgress =3D (xlogctl->SharedRecoveryState !=3D RECOVERY_= STATE_DONE); =20 /* * Initialize TimeLineID and RedoRecPtr when we discover that recovery @@ -8054,8 +8075,8 @@ RecoveryInProgress(void) { /* * If we just exited recovery, make sure we read TimeLineID and - * RedoRecPtr after SharedRecoveryInProgress (for machines with - * weak memory ordering). + * RedoRecPtr after SharedRecoveryState (for machines with weak + * memory ordering). */ pg_memory_barrier(); InitXLOGAccess(); @@ -8071,6 +8092,24 @@ RecoveryInProgress(void) } } =20 +/* + * Returns current recovery state from shared memory. + * + * This returned state is kept consistent with the contents of the control + * file. See details about the possible values of RecoveryState in xlog.h. + */ +RecoveryState +GetRecoveryState(void) +{ + RecoveryState retval; + + SpinLockAcquire(&XLogCtl->info_lck); + retval =3D XLogCtl->SharedRecoveryState; + SpinLockRelease(&XLogCtl->info_lck); + + return retval; +} + /* * Is HotStandby active yet? This is only important in special backends * since normal backends won't ever be able to connect until this returns diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/= transam/xlogarchive.c index 2d2812b59c..f8b8504e41 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -610,18 +610,25 @@ XLogArchiveCheckDone(const char *xlog) { char archiveStatusPath[MAXPGPATH]; struct stat stat_buf; - bool inRecovery =3D RecoveryInProgress(); + + /* The file is always deletable if archive_mode is "off". */ + if (!XLogArchivingActive()) + return true; =20 /* - * The file is always deletable if archive_mode is "off". On standbys - * archiving is disabled if archive_mode is "on", and enabled with - * "always". On a primary, archiving is enabled if archive_mode is "on" - * or "always". + * During archive recovery, the file is deletable if archive_mode is not + * "always". */ - if (!((XLogArchivingActive() && !inRecovery) || - (XLogArchivingAlways() && inRecovery))) + if (!XLogArchivingAlways() && + GetRecoveryState() =3D=3D RECOVERY_STATE_ARCHIVE) return true; =20 + /* + * At this point of the logic, note that we are either a primary with + * archive_mode set to "on" or "always", or a standby with archive_mode + * set to "always". + */ + /* First check for .done --- this means archiver is done with it */ StatusFilePath(archiveStatusPath, xlog, ".done"); if (stat(archiveStatusPath, &stat_buf) =3D=3D 0) diff --git a/src/test/recovery/t/020_archive_status.pl b/src/test/recovery/= t/020_archive_status.pl new file mode 100644 index 0000000000..2369b34712 --- /dev/null +++ b/src/test/recovery/t/020_archive_status.pl @@ -0,0 +1,214 @@ +# +# Tests related to WAL archiving and recovery. +# +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests =3D> 16; +use Config; + +my $primary =3D get_new_node('master'); +$primary->init( + has_archiving =3D> 1, + allows_streaming =3D> 1); +$primary->append_conf('postgresql.conf', 'autovacuum =3D off'); +$primary->start; +my $primary_data =3D $primary->data_dir; + +# Temporarily use an archive_command value to make the archiver fail, +# knowing that archiving is enabled. Note that we cannot use a command +# that does not exist as in this case the archiver process would just exit +# without reporting the failure to pg_stat_archiver. This also cannot +# use a plain "false" as that's unportable on Windows. So, instead, as +# a portable solution, use an archive command based on a command known to +# work but will fail: copy with an incorrect original path. +my $incorrect_command =3D + $TestLib::windows_os + ? qq{copy "%p_does_not_exist" "%f_does_not_exist"} + : qq{cp "%p_does_not_exist" "%f_does_not_exist"}; +$primary->safe_psql( + 'postgres', qq{ + ALTER SYSTEM SET archive_command TO '$incorrect_command'; + SELECT pg_reload_conf(); +}); + +# Save the WAL segment currently in use and switch to a new segment. +# This will be used to track the activity of the archiver. +my $segment_name_1 =3D $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); +my $segment_path_1 =3D "pg_wal/archive_status/$segment_name_1"; +my $segment_path_1_ready =3D "$segment_path_1.ready"; +my $segment_path_1_done =3D "$segment_path_1.done"; +$primary->safe_psql( + 'postgres', q{ + CREATE TABLE mine AS SELECT generate_series(1,10) AS x; + SELECT pg_switch_wal(); + CHECKPOINT; +}); + +# Wait for an archive failure. +$primary->poll_query_until('postgres', + q{SELECT failed_count > 0 FROM pg_stat_archiver}, 't') + or die "Timed out while waiting for archiving to fail"; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file exists for WAL segment $segment_name_1 waiting to be archive= d" +); +ok( !-f "$primary_data/$segment_path_1_done", + ".done file does not exist for WAL segment $segment_name_1 waiting to be = archived" +); + +is( $primary->safe_psql( + 'postgres', q{ + SELECT archived_count, last_failed_wal + FROM pg_stat_archiver + }), + "0|$segment_name_1", + 'pg_stat_archiver failed to archive $segment_name_1'); + +# Crash the cluster for the next test in charge of checking that non-archi= ved +# WAL segments are not removed. +$primary->stop('immediate'); + +# Recovery tests for the archiving with a standby partially check +# the recovery behavior when restoring a backup taken using a +# snapshot with no pg_start/stop_backup. In this situation, +# the recovered standby should enter first crash recovery then +# switch to regular archive recovery. Note that the base backup +# is taken here so as archive_command will fail. This is necessary +# for the assumptions of the tests done with the standbys below. +$primary->backup_fs_cold('backup'); + +$primary->start; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 still exists after crash rec= overy on primary" +); + +# Allow WAL archiving again and wait for a success. +$primary->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); + +$primary->poll_query_until('postgres', + q{SELECT archived_count FROM pg_stat_archiver}, '1') + or die "Timed out while waiting for archiving to finish"; + +ok(!-f "$primary_data/$segment_path_1_ready", + ".ready file for archived WAL segment $segment_name_1 removed"); + +ok(-f "$primary_data/$segment_path_1_done", + ".done file for archived WAL segment $segment_name_1 exists"); + +is( $primary->safe_psql( + 'postgres', q{ SELECT last_archived_wal FROM pg_stat_archiver }), + $segment_name_1, + 'archive success reported in pg_stat_archiver for WAL segment $segment_na= me_1' +); + +# Create some WAL activity and a new checkpoint so as the next standby can +# create a restartpoint. As this standby starts in crash recovery because +# of the cold backup taken previously, it needs a clean restartpoint to de= al +# with existing status files. +my $segment_name_2 =3D $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); +my $segment_path_2 =3D "pg_wal/archive_status/$segment_name_2"; +my $segment_path_2_ready =3D "$segment_path_2.ready"; +my $segment_path_2_done =3D "$segment_path_2.done"; +$primary->safe_psql( + 'postgres', q{ + INSERT INTO mine SELECT generate_series(10,20) AS x; + SELECT pg_switch_wal(); + CHECKPOINT; +}); + +$primary->poll_query_until('postgres', + q{ SELECT last_archived_wal FROM pg_stat_archiver }, + $segment_name_2) + or die "Timed out while waiting for archiving to finish"; + +# Test standby with archive_mode =3D on. +my $standby1 =3D get_new_node('standby'); +$standby1->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby1->append_conf('postgresql.conf', "archive_mode =3D on"); +my $standby1_data =3D $standby1->data_dir; +$standby1->start; +$standby1->safe_psql('postgres', q{CHECKPOINT}); + +# Recovery with archive_mode=3Don does not keep .ready signal files inheri= ted +# from backup. Note that this WAL segment existed in the backup. +ok( !-f "$standby1_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 present in backup got remove= d with archive_mode=3Don on standby" +); + +# Recovery with archive_mode=3Don should not create .ready files. +# Note that this segment did not exist in the backup. +ok( !-f "$standby1_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 not created on standby when = archive_mode=3Don on standby" +); + +# Recovery with archive_mode =3D on creates .done files. +ok( -f "$standby1_data/$segment_path_2_done", + ".done file for WAL segment $segment_name_2 created when archive_mode=3Do= n on standby" +); + +# Test recovery with archive_mode =3D always, which should always keep +# .ready files if archiving is enabled, though here we want the archive +# command to fail to persist the .ready files. Note that this node +# has inherited the archive command of the previous cold backup that +# will cause archiving failures. +my $standby2 =3D get_new_node('standby2'); +$standby2->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby2->append_conf('postgresql.conf', 'archive_mode =3D always'); +my $standby2_data =3D $standby2->data_dir; +$standby2->start; + +$standby2->safe_psql('postgres', q{CHECKPOINT}); + +ok( -f "$standby2_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 existing in backup is kept w= ith archive_mode=3Dalways on standby" +); + +ok( -f "$standby2_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 created with archive_mode=3D= always on standby" +); + +# Reset statistics of the archiver for the next checks. +$standby2->safe_psql('postgres', q{SELECT pg_stat_reset_shared('archiver')= }); + +# Now crash the cluster to check that recovery step does not +# remove non-archived WAL segments on a standby where archiving +# is enabled. +$standby2->stop('immediate'); +$standby2->start; + +ok( -f "$standby2_data/$segment_path_1_ready", + "WAL segment still ready to archive after crash recovery on standby with = archive_mode=3Dalways" +); + +# Allow WAL archiving again, and wait for the segments to be archived. +$standby2->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); +$standby2->poll_query_until('postgres', + q{SELECT last_archived_wal FROM pg_stat_archiver}, + $segment_name_2) + or die "Timed out while waiting for archiving to finish"; + +is( $standby2->safe_psql( + 'postgres', q{SELECT archived_count FROM pg_stat_archiver}), + '2', + 'correct number of WAL segments archived from standby'); + +ok( !-f "$standby2_data/$segment_path_1_ready" + && !-f "$standby2_data/$segment_path_2_ready", + ".ready files removed after archive success with archive_mode=3Dalways on= standby" +); + +ok( -f "$standby2_data/$segment_path_1_done" + && -f "$standby2_data/$segment_path_2_done", + ".done files created after archive success with archive_mode=3Dalways on = standby" +); --=20 2.26.0 --9iyR+p8Z2cn535Lj Content-Type: text/x-diff; charset=iso-8859-1 Content-Disposition: attachment; filename="0001-Fix-handling-of-WAL-segments-ready-to-be-archived-10.patch" Content-Transfer-Encoding: quoted-printable =46rom 10809521faf22fa63f70d016f3f317796be5849d Mon Sep 17 00:00:00 2001 =46rom: Michael Paquier Date: Wed, 22 Apr 2020 14:48:25 +0900 Subject: [PATCH] Fix handling of WAL segments ready to be archived during crash recovery MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit 78ea8b5 has fixed an issue related to the recycling of WAL segments on standbys depending on archive_mode. However, it has introduced a regression with the handling of WAL segments ready to be archived during crash recovery, causing those files to be recycled without getting archived. This commit fixes the regression by tracking in shared memory if a live cluster is either in crash recovery or archive recovery as the handling of WAL segments ready to be archived is different in both cases (those WAL segments should not be removed during crash recovery), and by using this new shared memory state to decide if a segment can be recycled or not. Previously, it was not possible to know if a cluster was in crash recovery or archive recovery as the shared state was able to track only if recovery was happening or not, leading to the problem. A set of TAP tests is added to close the gap here, making sure that WAL segments ready to be archived are correctly handled when a cluster is in archive or crash recovery with archive_mode set to "on" or "always", for both standby and primary. Reported-by: Beno=EEt Lobr=E9au Author: Jehan-Guillaume de Rorthais Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost Backpatch-through: 9.5 --- src/include/access/xlog.h | 9 + src/backend/access/transam/xlog.c | 57 +++++- src/backend/access/transam/xlogarchive.c | 21 ++- src/test/recovery/t/020_archive_status.pl | 215 ++++++++++++++++++++++ 4 files changed, 286 insertions(+), 16 deletions(-) create mode 100644 src/test/recovery/t/020_archive_status.pl diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 8751d86cbd..d41fefc185 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -128,6 +128,14 @@ typedef enum WalLevel WAL_LEVEL_LOGICAL } WalLevel; =20 +/* Recovery states */ +typedef enum RecoveryState +{ + RECOVERY_STATE_CRASH =3D 0, /* crash recovery */ + RECOVERY_STATE_ARCHIVE, /* archive recovery */ + RECOVERY_STATE_DONE /* currently in production */ +} RecoveryState; + extern PGDLLIMPORT int wal_level; =20 /* Is WAL archiving enabled (always or only while server is running normal= ly)? */ @@ -242,6 +250,7 @@ extern const char *xlog_identify(uint8 info); extern void issue_xlog_fsync(int fd, XLogSegNo segno); =20 extern bool RecoveryInProgress(void); +extern RecoveryState GetRecoveryState(void); extern bool HotStandbyActive(void); extern bool HotStandbyActiveInReplay(void); extern bool XLogInsertAllowed(void); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam= /xlog.c index ec66b99b32..c10dc889c6 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -209,8 +209,9 @@ static TimeLineID receiveTLI =3D 0; static bool lastFullPageWrites; =20 /* - * Local copy of SharedRecoveryInProgress variable. True actually means "n= ot - * known, need to check the shared state". + * Local copy of the state tracked by SharedRecoveryState in shared memory, + * It is false if SharedRecoveryState is RECOVERY_STATE_DONE. True actual= ly + * means "not known, need to check the shared state". */ static bool LocalRecoveryInProgress =3D true; =20 @@ -635,10 +636,10 @@ typedef struct XLogCtlData char archiveCleanupCommand[MAXPGPATH]; =20 /* - * SharedRecoveryInProgress indicates if we're still in crash or archive + * SharedRecoveryState indicates if we're still in crash or archive * recovery. Protected by info_lck. */ - bool SharedRecoveryInProgress; + RecoveryState SharedRecoveryState; =20 /* * SharedHotStandbyActive indicates if we're still in crash or archive @@ -4295,6 +4296,16 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr R= ecPtr, int emode, updateMinRecoveryPoint =3D true; =20 UpdateControlFile(); + + /* + * We update SharedRecoveryState while holding the lock on + * ControlFileLock so both states are consistent in shared + * memory. + */ + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + LWLockRelease(ControlFileLock); =20 CheckRecoveryConsistency(); @@ -4980,7 +4991,7 @@ XLOGShmemInit(void) * in additional info.) */ XLogCtl->XLogCacheBlck =3D XLOGbuffers - 1; - XLogCtl->SharedRecoveryInProgress =3D true; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; XLogCtl->SharedHotStandbyActive =3D false; XLogCtl->WalWriterSleeping =3D false; =20 @@ -6803,7 +6814,13 @@ StartupXLOG(void) */ dbstate_at_startup =3D ControlFile->state; if (InArchiveRecovery) + { ControlFile->state =3D DB_IN_ARCHIVE_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + } else { ereport(LOG, @@ -6816,6 +6833,10 @@ StartupXLOG(void) ControlFile->checkPointCopy.ThisTimeLineID, recoveryTargetTLI))); ControlFile->state =3D DB_IN_CRASH_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; + SpinLockRelease(&XLogCtl->info_lck); } ControlFile->prevCheckPoint =3D ControlFile->checkPoint; ControlFile->checkPoint =3D checkPointLoc; @@ -7841,7 +7862,7 @@ StartupXLOG(void) ControlFile->time =3D (pg_time_t) time(NULL); =20 SpinLockAcquire(&XLogCtl->info_lck); - XLogCtl->SharedRecoveryInProgress =3D false; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_DONE; SpinLockRelease(&XLogCtl->info_lck); =20 UpdateControlFile(); @@ -7987,7 +8008,7 @@ RecoveryInProgress(void) */ volatile XLogCtlData *xlogctl =3D XLogCtl; =20 - LocalRecoveryInProgress =3D xlogctl->SharedRecoveryInProgress; + LocalRecoveryInProgress =3D (xlogctl->SharedRecoveryState !=3D RECOVERY_= STATE_DONE); =20 /* * Initialize TimeLineID and RedoRecPtr when we discover that recovery @@ -7999,8 +8020,8 @@ RecoveryInProgress(void) { /* * If we just exited recovery, make sure we read TimeLineID and - * RedoRecPtr after SharedRecoveryInProgress (for machines with - * weak memory ordering). + * RedoRecPtr after SharedRecoveryState (for machines with weak + * memory ordering). */ pg_memory_barrier(); InitXLOGAccess(); @@ -8016,6 +8037,24 @@ RecoveryInProgress(void) } } =20 +/* + * Returns current recovery state from shared memory. + * + * This returned state is kept consistent with the contents of the control + * file. See details about the possible values of RecoveryState in xlog.h. + */ +RecoveryState +GetRecoveryState(void) +{ + RecoveryState retval; + + SpinLockAcquire(&XLogCtl->info_lck); + retval =3D XLogCtl->SharedRecoveryState; + SpinLockRelease(&XLogCtl->info_lck); + + return retval; +} + /* * Is HotStandby active yet? This is only important in special backends * since normal backends won't ever be able to connect until this returns diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/= transam/xlogarchive.c index 7825cfe532..13f18ae85d 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -608,18 +608,25 @@ XLogArchiveCheckDone(const char *xlog) { char archiveStatusPath[MAXPGPATH]; struct stat stat_buf; - bool inRecovery =3D RecoveryInProgress(); + + /* The file is always deletable if archive_mode is "off". */ + if (!XLogArchivingActive()) + return true; =20 /* - * The file is always deletable if archive_mode is "off". On standbys - * archiving is disabled if archive_mode is "on", and enabled with - * "always". On a primary, archiving is enabled if archive_mode is "on" - * or "always". + * During archive recovery, the file is deletable if archive_mode is not + * "always". */ - if (!((XLogArchivingActive() && !inRecovery) || - (XLogArchivingAlways() && inRecovery))) + if (!XLogArchivingAlways() && + GetRecoveryState() =3D=3D RECOVERY_STATE_ARCHIVE) return true; =20 + /* + * At this point of the logic, note that we are either a primary with + * archive_mode set to "on" or "always", or a standby with archive_mode + * set to "always". + */ + /* First check for .done --- this means archiver is done with it */ StatusFilePath(archiveStatusPath, xlog, ".done"); if (stat(archiveStatusPath, &stat_buf) =3D=3D 0) diff --git a/src/test/recovery/t/020_archive_status.pl b/src/test/recovery/= t/020_archive_status.pl new file mode 100644 index 0000000000..390b85702b --- /dev/null +++ b/src/test/recovery/t/020_archive_status.pl @@ -0,0 +1,215 @@ +# +# Tests related to WAL archiving and recovery. +# +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests =3D> 16; +use Config; + +my $primary =3D get_new_node('master'); +$primary->init( + has_archiving =3D> 1, + allows_streaming =3D> 1); +$primary->append_conf('postgresql.conf', 'autovacuum =3D off'); +$primary->start; +my $primary_data =3D $primary->data_dir; + +# Temporarily use an archive_command value to make the archiver fail, +# knowing that archiving is enabled. Note that we cannot use a command +# that does not exist as in this case the archiver process would just exit +# without reporting the failure to pg_stat_archiver. This also cannot +# use a plain "false" as that's unportable on Windows. So, instead, as +# a portable solution, use an archive command based on a command known to +# work but will fail: copy with an incorrect original path. +my $incorrect_command =3D + $TestLib::windows_os + ? qq{copy "%p_does_not_exist" "%f_does_not_exist"} + : qq{cp "%p_does_not_exist" "%f_does_not_exist"}; +$primary->safe_psql( + 'postgres', qq{ + ALTER SYSTEM SET archive_command TO '$incorrect_command'; + SELECT pg_reload_conf(); +}); + +# Save the WAL segment currently in use and switch to a new segment. +# This will be used to track the activity of the archiver. +my $segment_name_1 =3D $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); +my $segment_path_1 =3D "pg_wal/archive_status/$segment_name_1"; +my $segment_path_1_ready =3D "$segment_path_1.ready"; +my $segment_path_1_done =3D "$segment_path_1.done"; +$primary->safe_psql( + 'postgres', q{ + CREATE TABLE mine AS SELECT generate_series(1,10) AS x; + SELECT pg_switch_wal(); + CHECKPOINT; +}); + +# Wait for an archive failure. +$primary->poll_query_until('postgres', + q{SELECT failed_count > 0 FROM pg_stat_archiver}, 't') + or die "Timed out while waiting for archiving to fail"; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file exists for WAL segment $segment_name_1 waiting to be archive= d" +); +ok( !-f "$primary_data/$segment_path_1_done", + ".done file does not exist for WAL segment $segment_name_1 waiting to be = archived" +); + +is( $primary->safe_psql( + 'postgres', q{ + SELECT archived_count, last_failed_wal + FROM pg_stat_archiver + }), + "0|$segment_name_1", + 'pg_stat_archiver failed to archive $segment_name_1'); + +# Crash the cluster for the next test in charge of checking that non-archi= ved +# WAL segments are not removed. +$primary->stop('immediate'); + +# Recovery tests for the archiving with a standby partially check +# the recovery behavior when restoring a backup taken using a +# snapshot with no pg_start/stop_backup. In this situation, +# the recovered standby should enter first crash recovery then +# switch to regular archive recovery. Note that the base backup +# is taken here so as archive_command will fail. This is necessary +# for the assumptions of the tests done with the standbys below. +$primary->backup_fs_cold('backup'); + +$primary->start; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 still exists after crash rec= overy on primary" +); + +# Allow WAL archiving again and wait for a success. +$primary->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); + +$primary->poll_query_until('postgres', + q{SELECT archived_count FROM pg_stat_archiver}, '1') + or die "Timed out while waiting for archiving to finish"; + +ok(!-f "$primary_data/$segment_path_1_ready", + ".ready file for archived WAL segment $segment_name_1 removed"); + +ok(-f "$primary_data/$segment_path_1_done", + ".done file for archived WAL segment $segment_name_1 exists"); + +is( $primary->safe_psql( + 'postgres', q{ SELECT last_archived_wal FROM pg_stat_archiver }), + $segment_name_1, + 'archive success reported in pg_stat_archiver for WAL segment $segment_na= me_1' +); + +# Create some WAL activity and a new checkpoint so as the next standby can +# create a restartpoint. As this standby starts in crash recovery because +# of the cold backup taken previously, it needs a clean restartpoint to de= al +# with existing status files. +my $segment_name_2 =3D $primary->safe_psql('postgres', + q{SELECT pg_walfile_name(pg_current_wal_lsn())}); +my $segment_path_2 =3D "pg_wal/archive_status/$segment_name_2"; +my $segment_path_2_ready =3D "$segment_path_2.ready"; +my $segment_path_2_done =3D "$segment_path_2.done"; +$primary->safe_psql( + 'postgres', q{ + INSERT INTO mine SELECT generate_series(10,20) AS x; + SELECT pg_switch_wal(); + CHECKPOINT; +}); + +$primary->poll_query_until('postgres', + q{ SELECT last_archived_wal FROM pg_stat_archiver }, + $segment_name_2) + or die "Timed out while waiting for archiving to finish"; + +# Test standby with archive_mode =3D on. +my $standby1 =3D get_new_node('standby'); +$standby1->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby1->append_conf('postgresql.conf', "archive_mode =3D on"); +my $standby1_data =3D $standby1->data_dir; +$standby1->start; +$standby1->safe_psql('postgres', q{CHECKPOINT}); + +# Recovery with archive_mode=3Don keeps .ready signal files inherited +# from backup after one checkpoint. Note that this WAL segment +# existed in the backup. +ok( -f "$standby1_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 present in backup not remove= d with archive_mode=3Don on standby" +); + +# Recovery with archive_mode=3Don should not create .ready files. +# Note that this segment did not exist in the backup. +ok( !-f "$standby1_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 not created on standby when = archive_mode=3Don on standby" +); + +# Recovery with archive_mode =3D on creates .done files. +ok( -f "$standby1_data/$segment_path_2_done", + ".done file for WAL segment $segment_name_2 created when archive_mode=3Do= n on standby" +); + +# Test recovery with archive_mode =3D always, which should always keep +# .ready files if archiving is enabled, though here we want the archive +# command to fail to persist the .ready files. Note that this node +# has inherited the archive command of the previous cold backup that +# will cause archiving failures. +my $standby2 =3D get_new_node('standby2'); +$standby2->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby2->append_conf('postgresql.conf', 'archive_mode =3D always'); +my $standby2_data =3D $standby2->data_dir; +$standby2->start; + +$standby2->safe_psql('postgres', q{CHECKPOINT}); + +ok( -f "$standby2_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 existing in backup is kept w= ith archive_mode=3Dalways on standby" +); + +ok( -f "$standby2_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 created with archive_mode=3D= always on standby" +); + +# Reset statistics of the archiver for the next checks. +$standby2->safe_psql('postgres', q{SELECT pg_stat_reset_shared('archiver')= }); + +# Now crash the cluster to check that recovery step does not +# remove non-archived WAL segments on a standby where archiving +# is enabled. +$standby2->stop('immediate'); +$standby2->start; + +ok( -f "$standby2_data/$segment_path_1_ready", + "WAL segment still ready to archive after crash recovery on standby with = archive_mode=3Dalways" +); + +# Allow WAL archiving again, and wait for the segments to be archived. +$standby2->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); +$standby2->poll_query_until('postgres', + q{SELECT last_archived_wal FROM pg_stat_archiver}, + $segment_name_2) + or die "Timed out while waiting for archiving to finish"; + +is( $standby2->safe_psql( + 'postgres', q{SELECT archived_count FROM pg_stat_archiver}), + '2', + 'correct number of WAL segments archived from standby'); + +ok( !-f "$standby2_data/$segment_path_1_ready" + && !-f "$standby2_data/$segment_path_2_ready", + ".ready files removed after archive success with archive_mode=3Dalways on= standby" +); + +ok( -f "$standby2_data/$segment_path_1_done" + && -f "$standby2_data/$segment_path_2_done", + ".done files created after archive success with archive_mode=3Dalways on = standby" +); --=20 2.26.0 --9iyR+p8Z2cn535Lj Content-Type: text/x-diff; charset=iso-8859-1 Content-Disposition: attachment; filename="0001-Fix-handling-of-WAL-segments-ready-to-be-archived-96.patch" Content-Transfer-Encoding: quoted-printable =46rom d9baaf1b8f9ce9afec0b2c20610aaf64d05f4c5c Mon Sep 17 00:00:00 2001 =46rom: Michael Paquier Date: Wed, 22 Apr 2020 14:48:25 +0900 Subject: [PATCH] Fix handling of WAL segments ready to be archived during crash recovery MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit 78ea8b5 has fixed an issue related to the recycling of WAL segments on standbys depending on archive_mode. However, it has introduced a regression with the handling of WAL segments ready to be archived during crash recovery, causing those files to be recycled without getting archived. This commit fixes the regression by tracking in shared memory if a live cluster is either in crash recovery or archive recovery as the handling of WAL segments ready to be archived is different in both cases (those WAL segments should not be removed during crash recovery), and by using this new shared memory state to decide if a segment can be recycled or not. Previously, it was not possible to know if a cluster was in crash recovery or archive recovery as the shared state was able to track only if recovery was happening or not, leading to the problem. A set of TAP tests is added to close the gap here, making sure that WAL segments ready to be archived are correctly handled when a cluster is in archive or crash recovery with archive_mode set to "on" or "always", for both standby and primary. Reported-by: Beno=EEt Lobr=E9au Author: Jehan-Guillaume de Rorthais Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost Backpatch-through: 9.5 --- src/include/access/xlog.h | 9 + src/backend/access/transam/xlog.c | 57 +++++- src/backend/access/transam/xlogarchive.c | 21 ++- src/test/recovery/t/020_archive_status.pl | 213 ++++++++++++++++++++++ 4 files changed, 284 insertions(+), 16 deletions(-) create mode 100644 src/test/recovery/t/020_archive_status.pl diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index ab97a61bbb..28f716d4cf 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -125,6 +125,14 @@ typedef enum WalLevel WAL_LEVEL_LOGICAL } WalLevel; =20 +/* Recovery states */ +typedef enum RecoveryState +{ + RECOVERY_STATE_CRASH =3D 0, /* crash recovery */ + RECOVERY_STATE_ARCHIVE, /* archive recovery */ + RECOVERY_STATE_DONE /* currently in production */ +} RecoveryState; + extern PGDLLIMPORT int wal_level; =20 /* Is WAL archiving enabled (always or only while server is running normal= ly)? */ @@ -231,6 +239,7 @@ extern const char *xlog_identify(uint8 info); extern void issue_xlog_fsync(int fd, XLogSegNo segno); =20 extern bool RecoveryInProgress(void); +extern RecoveryState GetRecoveryState(void); extern bool HotStandbyActive(void); extern bool HotStandbyActiveInReplay(void); extern bool XLogInsertAllowed(void); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam= /xlog.c index 823efdbed0..acca98ea28 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -204,8 +204,9 @@ static TimeLineID receiveTLI =3D 0; static bool lastFullPageWrites; =20 /* - * Local copy of SharedRecoveryInProgress variable. True actually means "n= ot - * known, need to check the shared state". + * Local copy of the state tracked by SharedRecoveryState in shared memory, + * It is false if SharedRecoveryState is RECOVERY_STATE_DONE. True actual= ly + * means "not known, need to check the shared state". */ static bool LocalRecoveryInProgress =3D true; =20 @@ -616,10 +617,10 @@ typedef struct XLogCtlData char archiveCleanupCommand[MAXPGPATH]; =20 /* - * SharedRecoveryInProgress indicates if we're still in crash or archive + * SharedRecoveryState indicates if we're still in crash or archive * recovery. Protected by info_lck. */ - bool SharedRecoveryInProgress; + RecoveryState SharedRecoveryState; =20 /* * SharedHotStandbyActive indicates if we're still in crash or archive @@ -4136,6 +4137,16 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr R= ecPtr, int emode, updateMinRecoveryPoint =3D true; =20 UpdateControlFile(); + + /* + * We update SharedRecoveryState while holding the lock on + * ControlFileLock so both states are consistent in shared + * memory. + */ + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + LWLockRelease(ControlFileLock); =20 CheckRecoveryConsistency(); @@ -4819,7 +4830,7 @@ XLOGShmemInit(void) * in additional info.) */ XLogCtl->XLogCacheBlck =3D XLOGbuffers - 1; - XLogCtl->SharedRecoveryInProgress =3D true; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; XLogCtl->SharedHotStandbyActive =3D false; XLogCtl->WalWriterSleeping =3D false; =20 @@ -6541,7 +6552,13 @@ StartupXLOG(void) */ dbstate_at_startup =3D ControlFile->state; if (InArchiveRecovery) + { ControlFile->state =3D DB_IN_ARCHIVE_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + } else { ereport(LOG, @@ -6554,6 +6571,10 @@ StartupXLOG(void) ControlFile->checkPointCopy.ThisTimeLineID, recoveryTargetTLI))); ControlFile->state =3D DB_IN_CRASH_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; + SpinLockRelease(&XLogCtl->info_lck); } ControlFile->prevCheckPoint =3D ControlFile->checkPoint; ControlFile->checkPoint =3D checkPointLoc; @@ -7555,7 +7576,7 @@ StartupXLOG(void) * updates to shared memory.) */ SpinLockAcquire(&XLogCtl->info_lck); - XLogCtl->SharedRecoveryInProgress =3D false; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_DONE; SpinLockRelease(&XLogCtl->info_lck); =20 /* @@ -7698,7 +7719,7 @@ RecoveryInProgress(void) */ volatile XLogCtlData *xlogctl =3D XLogCtl; =20 - LocalRecoveryInProgress =3D xlogctl->SharedRecoveryInProgress; + LocalRecoveryInProgress =3D (xlogctl->SharedRecoveryState !=3D RECOVERY_= STATE_DONE); =20 /* * Initialize TimeLineID and RedoRecPtr when we discover that recovery @@ -7710,8 +7731,8 @@ RecoveryInProgress(void) { /* * If we just exited recovery, make sure we read TimeLineID and - * RedoRecPtr after SharedRecoveryInProgress (for machines with - * weak memory ordering). + * RedoRecPtr after SharedRecoveryState (for machines with weak + * memory ordering). */ pg_memory_barrier(); InitXLOGAccess(); @@ -7727,6 +7748,24 @@ RecoveryInProgress(void) } } =20 +/* + * Returns current recovery state from shared memory. + * + * This returned state is kept consistent with the contents of the control + * file. See details about the possible values of RecoveryState in xlog.h. + */ +RecoveryState +GetRecoveryState(void) +{ + RecoveryState retval; + + SpinLockAcquire(&XLogCtl->info_lck); + retval =3D XLogCtl->SharedRecoveryState; + SpinLockRelease(&XLogCtl->info_lck); + + return retval; +} + /* * Is HotStandby active yet? This is only important in special backends * since normal backends won't ever be able to connect until this returns diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/= transam/xlogarchive.c index 99704c1469..3cc8021cb8 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -619,18 +619,25 @@ XLogArchiveCheckDone(const char *xlog) { char archiveStatusPath[MAXPGPATH]; struct stat stat_buf; - bool inRecovery =3D RecoveryInProgress(); + + /* The file is always deletable if archive_mode is "off". */ + if (!XLogArchivingActive()) + return true; =20 /* - * The file is always deletable if archive_mode is "off". On standbys - * archiving is disabled if archive_mode is "on", and enabled with - * "always". On a primary, archiving is enabled if archive_mode is "on" - * or "always". + * During archive recovery, the file is deletable if archive_mode is not + * "always". */ - if (!((XLogArchivingActive() && !inRecovery) || - (XLogArchivingAlways() && inRecovery))) + if (!XLogArchivingAlways() && + GetRecoveryState() =3D=3D RECOVERY_STATE_ARCHIVE) return true; =20 + /* + * At this point of the logic, note that we are either a primary with + * archive_mode set to "on" or "always", or a standby with archive_mode + * set to "always". + */ + /* First check for .done --- this means archiver is done with it */ StatusFilePath(archiveStatusPath, xlog, ".done"); if (stat(archiveStatusPath, &stat_buf) =3D=3D 0) diff --git a/src/test/recovery/t/020_archive_status.pl b/src/test/recovery/= t/020_archive_status.pl new file mode 100644 index 0000000000..7cf367d8bf --- /dev/null +++ b/src/test/recovery/t/020_archive_status.pl @@ -0,0 +1,213 @@ +# +# Tests related to WAL archiving and recovery. +# +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests =3D> 16; +use Config; + +my $primary =3D get_new_node('master'); +$primary->init( + has_archiving =3D> 1, + allows_streaming =3D> 1); +$primary->append_conf('postgresql.conf', 'autovacuum =3D off'); +$primary->start; +my $primary_data =3D $primary->data_dir; + +# Temporarily use an archive_command value to make the archiver fail, +# knowing that archiving is enabled. Note that we cannot use a command +# that does not exist as in this case the archiver process would just exit +# without reporting the failure to pg_stat_archiver. This also cannot +# use a plain "false" as that's unportable on Windows. So, instead, as +# a portable solution, use an archive command based on a command known to +# work but will fail: copy with an incorrect original path. +my $incorrect_command =3D + $TestLib::windows_os + ? qq{copy "%p_does_not_exist" "%f_does_not_exist"} + : qq{cp "%p_does_not_exist" "%f_does_not_exist"}; +$primary->safe_psql( + 'postgres', qq{ + ALTER SYSTEM SET archive_command TO '$incorrect_command'; + SELECT pg_reload_conf(); +}); + +# Save the WAL segment currently in use and switch to a new segment. +# This will be used to track the activity of the archiver. +my $segment_name_1 =3D $primary->safe_psql('postgres', + q{SELECT pg_xlogfile_name(pg_current_xlog_location())}); +my $segment_path_1 =3D "pg_xlog/archive_status/$segment_name_1"; +my $segment_path_1_ready =3D "$segment_path_1.ready"; +my $segment_path_1_done =3D "$segment_path_1.done"; +$primary->safe_psql( + 'postgres', q{ + CREATE TABLE mine AS SELECT generate_series(1,10) AS x; + SELECT pg_switch_xlog(); + CHECKPOINT; +}); + +# Wait for an archive failure. +$primary->poll_query_until('postgres', + q{SELECT failed_count > 0 FROM pg_stat_archiver}, 't') + or die "Timed out while waiting for archiving to fail"; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file exists for WAL segment $segment_name_1 waiting to be archive= d" +); +ok( !-f "$primary_data/$segment_path_1_done", + ".done file does not exist for WAL segment $segment_name_1 waiting to be = archived" +); + +is( $primary->safe_psql( + 'postgres', q{ + SELECT archived_count, last_failed_wal + FROM pg_stat_archiver + }), + "0|$segment_name_1", + 'pg_stat_archiver failed to archive $segment_name_1'); + +# Crash the cluster for the next test in charge of checking that non-archi= ved +# WAL segments are not removed. +$primary->stop('immediate'); + +# Recovery tests for the archiving with a standby partially check +# the recovery behavior when restoring a backup taken using a +# snapshot with no pg_start/stop_backup. In this situation, +# the recovered standby should enter first crash recovery then +# switch to regular archive recovery. Note that the base backup +# is taken here so as archive_command will fail. This is necessary +# for the assumptions of the tests done with the standbys below. +$primary->backup_fs_cold('backup'); + +$primary->start; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 still exists after crash rec= overy on primary" +); + +# Allow WAL archiving again and wait for a success. +$primary->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); + +$primary->poll_query_until('postgres', + q{SELECT archived_count =3D 1 FROM pg_stat_archiver}) + or die "Timed out while waiting for archiving to finish"; + +ok(!-f "$primary_data/$segment_path_1_ready", + ".ready file for archived WAL segment $segment_name_1 removed"); + +ok(-f "$primary_data/$segment_path_1_done", + ".done file for archived WAL segment $segment_name_1 exists"); + +is( $primary->safe_psql( + 'postgres', q{ SELECT last_archived_wal FROM pg_stat_archiver }), + $segment_name_1, + 'archive success reported in pg_stat_archiver for WAL segment $segment_na= me_1' +); + +# Create some WAL activity and a new checkpoint so as the next standby can +# create a restartpoint. As this standby starts in crash recovery because +# of the cold backup taken previously, it needs a clean restartpoint to de= al +# with existing status files. +my $segment_name_2 =3D $primary->safe_psql('postgres', + q{SELECT pg_xlogfile_name(pg_current_xlog_location())}); +my $segment_path_2 =3D "pg_xlog/archive_status/$segment_name_2"; +my $segment_path_2_ready =3D "$segment_path_2.ready"; +my $segment_path_2_done =3D "$segment_path_2.done"; +$primary->safe_psql( + 'postgres', q{ + INSERT INTO mine SELECT generate_series(10,20) AS x; + SELECT pg_switch_xlog(); + CHECKPOINT; +}); + +$primary->poll_query_until('postgres', + qq{ SELECT last_archived_wal =3D '$segment_name_2' FROM pg_stat_archiver = }) + or die "Timed out while waiting for archiving to finish"; + +# Test standby with archive_mode =3D on. +my $standby1 =3D get_new_node('standby'); +$standby1->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby1->append_conf('postgresql.conf', "archive_mode =3D on"); +my $standby1_data =3D $standby1->data_dir; +$standby1->start; +$standby1->safe_psql('postgres', q{CHECKPOINT}); + +# Recovery with archive_mode=3Don keeps .ready signal files inherited +# from backup after one checkpoint. Note that this WAL segment +# existed in the backup. +ok( -f "$standby1_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 present in backup not remove= d with archive_mode=3Don on standby" +); + +# Recovery with archive_mode=3Don should not create .ready files. +# Note that this segment did not exist in the backup. +ok( !-f "$standby1_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 not created on standby when = archive_mode=3Don on standby" +); + +# Recovery with archive_mode =3D on creates .done files. +ok( -f "$standby1_data/$segment_path_2_done", + ".done file for WAL segment $segment_name_2 created when archive_mode=3Do= n on standby" +); + +# Test recovery with archive_mode =3D always, which should always keep +# .ready files if archiving is enabled, though here we want the archive +# command to fail to persist the .ready files. Note that this node +# has inherited the archive command of the previous cold backup that +# will cause archiving failures. +my $standby2 =3D get_new_node('standby2'); +$standby2->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby2->append_conf('postgresql.conf', 'archive_mode =3D always'); +my $standby2_data =3D $standby2->data_dir; +$standby2->start; + +$standby2->safe_psql('postgres', q{CHECKPOINT}); + +ok( -f "$standby2_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 existing in backup is kept w= ith archive_mode=3Dalways on standby" +); + +ok( -f "$standby2_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 created with archive_mode=3D= always on standby" +); + +# Reset statistics of the archiver for the next checks. +$standby2->safe_psql('postgres', q{SELECT pg_stat_reset_shared('archiver')= }); + +# Now crash the cluster to check that recovery step does not +# remove non-archived WAL segments on a standby where archiving +# is enabled. +$standby2->stop('immediate'); +$standby2->start; + +ok( -f "$standby2_data/$segment_path_1_ready", + "WAL segment still ready to archive after crash recovery on standby with = archive_mode=3Dalways" +); + +# Allow WAL archiving again, and wait for the segments to be archived. +$standby2->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); +$standby2->poll_query_until('postgres', + qq{SELECT last_archived_wal =3D '$segment_name_2' FROM pg_stat_archiver}) + or die "Timed out while waiting for archiving to finish"; + +is( $standby2->safe_psql( + 'postgres', q{SELECT archived_count FROM pg_stat_archiver}), + '2', + 'correct number of WAL segments archived from standby'); + +ok( !-f "$standby2_data/$segment_path_1_ready" + && !-f "$standby2_data/$segment_path_2_ready", + ".ready files removed after archive success with archive_mode=3Dalways on= standby" +); + +ok( -f "$standby2_data/$segment_path_1_done" + && -f "$standby2_data/$segment_path_2_done", + ".done files created after archive success with archive_mode=3Dalways on = standby" +); --=20 2.26.0 --9iyR+p8Z2cn535Lj Content-Type: text/x-diff; charset=iso-8859-1 Content-Disposition: attachment; filename="0001-Fix-handling-of-WAL-segments-ready-to-be-archived-95.patch" Content-Transfer-Encoding: quoted-printable =46rom 3dd404655cdc1667f5b2274cfc30c4d24ff988de Mon Sep 17 00:00:00 2001 =46rom: Michael Paquier Date: Wed, 22 Apr 2020 14:48:25 +0900 Subject: [PATCH] Fix handling of WAL segments ready to be archived during crash recovery MIME-Version: 1.0 Content-Type: text/plain; charset=3DUTF-8 Content-Transfer-Encoding: 8bit 78ea8b5 has fixed an issue related to the recycling of WAL segments on standbys depending on archive_mode. However, it has introduced a regression with the handling of WAL segments ready to be archived during crash recovery, causing those files to be recycled without getting archived. This commit fixes the regression by tracking in shared memory if a live cluster is either in crash recovery or archive recovery as the handling of WAL segments ready to be archived is different in both cases (those WAL segments should not be removed during crash recovery), and by using this new shared memory state to decide if a segment can be recycled or not. Previously, it was not possible to know if a cluster was in crash recovery or archive recovery as the shared state was able to track only if recovery was happening or not, leading to the problem. A set of TAP tests is added to close the gap here, making sure that WAL segments ready to be archived are correctly handled when a cluster is in archive or crash recovery with archive_mode set to "on" or "always", for both standby and primary. Reported-by: Beno=EEt Lobr=E9au Author: Jehan-Guillaume de Rorthais Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/20200331172229.40ee00dc@firost Backpatch-through: 9.5 --- src/include/access/xlog.h | 9 + src/backend/access/transam/xlog.c | 57 +++++- src/backend/access/transam/xlogarchive.c | 21 ++- src/test/recovery/t/020_archive_status.pl | 213 ++++++++++++++++++++++ 4 files changed, 284 insertions(+), 16 deletions(-) create mode 100644 src/test/recovery/t/020_archive_status.pl diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 59ebccb19e..d051ee02bc 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -126,6 +126,14 @@ typedef enum WalLevel } WalLevel; extern int wal_level; =20 +/* Recovery states */ +typedef enum RecoveryState +{ + RECOVERY_STATE_CRASH =3D 0, /* crash recovery */ + RECOVERY_STATE_ARCHIVE, /* archive recovery */ + RECOVERY_STATE_DONE /* currently in production */ +} RecoveryState; + /* Is WAL archiving enabled (always or only while server is running normal= ly)? */ #define XLogArchivingActive() \ (XLogArchiveMode > ARCHIVE_MODE_OFF && wal_level >=3D WAL_LEVEL_ARCHIVE) @@ -230,6 +238,7 @@ extern const char *xlog_identify(uint8 info); extern void issue_xlog_fsync(int fd, XLogSegNo segno); =20 extern bool RecoveryInProgress(void); +extern RecoveryState GetRecoveryState(void); extern bool HotStandbyActive(void); extern bool HotStandbyActiveInReplay(void); extern bool XLogInsertAllowed(void); diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam= /xlog.c index a378df9d76..a6cdb0024f 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -203,8 +203,9 @@ static TimeLineID receiveTLI =3D 0; static bool lastFullPageWrites; =20 /* - * Local copy of SharedRecoveryInProgress variable. True actually means "n= ot - * known, need to check the shared state". + * Local copy of the state tracked by SharedRecoveryState in shared memory, + * It is false if SharedRecoveryState is RECOVERY_STATE_DONE. True actual= ly + * means "not known, need to check the shared state". */ static bool LocalRecoveryInProgress =3D true; =20 @@ -608,10 +609,10 @@ typedef struct XLogCtlData char archiveCleanupCommand[MAXPGPATH]; =20 /* - * SharedRecoveryInProgress indicates if we're still in crash or archive + * SharedRecoveryState indicates if we're still in crash or archive * recovery. Protected by info_lck. */ - bool SharedRecoveryInProgress; + RecoveryState SharedRecoveryState; =20 /* * SharedHotStandbyActive indicates if we're still in crash or archive @@ -4083,6 +4084,16 @@ ReadRecord(XLogReaderState *xlogreader, XLogRecPtr R= ecPtr, int emode, updateMinRecoveryPoint =3D true; =20 UpdateControlFile(); + + /* + * We update SharedRecoveryState while holding the lock on + * ControlFileLock so both states are consistent in shared + * memory. + */ + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + LWLockRelease(ControlFileLock); =20 CheckRecoveryConsistency(); @@ -4771,7 +4782,7 @@ XLOGShmemInit(void) * in additional info.) */ XLogCtl->XLogCacheBlck =3D XLOGbuffers - 1; - XLogCtl->SharedRecoveryInProgress =3D true; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; XLogCtl->SharedHotStandbyActive =3D false; XLogCtl->WalWriterSleeping =3D false; =20 @@ -6493,7 +6504,13 @@ StartupXLOG(void) */ dbstate_at_startup =3D ControlFile->state; if (InArchiveRecovery) + { ControlFile->state =3D DB_IN_ARCHIVE_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_ARCHIVE; + SpinLockRelease(&XLogCtl->info_lck); + } else { ereport(LOG, @@ -6506,6 +6523,10 @@ StartupXLOG(void) ControlFile->checkPointCopy.ThisTimeLineID, recoveryTargetTLI))); ControlFile->state =3D DB_IN_CRASH_RECOVERY; + + SpinLockAcquire(&XLogCtl->info_lck); + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_CRASH; + SpinLockRelease(&XLogCtl->info_lck); } ControlFile->prevCheckPoint =3D ControlFile->checkPoint; ControlFile->checkPoint =3D checkPointLoc; @@ -7496,7 +7517,7 @@ StartupXLOG(void) * updates to shared memory.) */ SpinLockAcquire(&XLogCtl->info_lck); - XLogCtl->SharedRecoveryInProgress =3D false; + XLogCtl->SharedRecoveryState =3D RECOVERY_STATE_DONE; SpinLockRelease(&XLogCtl->info_lck); =20 /* @@ -7639,7 +7660,7 @@ RecoveryInProgress(void) */ volatile XLogCtlData *xlogctl =3D XLogCtl; =20 - LocalRecoveryInProgress =3D xlogctl->SharedRecoveryInProgress; + LocalRecoveryInProgress =3D (xlogctl->SharedRecoveryState !=3D RECOVERY_= STATE_DONE); =20 /* * Initialize TimeLineID and RedoRecPtr when we discover that recovery @@ -7651,8 +7672,8 @@ RecoveryInProgress(void) { /* * If we just exited recovery, make sure we read TimeLineID and - * RedoRecPtr after SharedRecoveryInProgress (for machines with - * weak memory ordering). + * RedoRecPtr after SharedRecoveryState (for machines with weak + * memory ordering). */ pg_memory_barrier(); InitXLOGAccess(); @@ -7668,6 +7689,24 @@ RecoveryInProgress(void) } } =20 +/* + * Returns current recovery state from shared memory. + * + * This returned state is kept consistent with the contents of the control + * file. See details about the possible values of RecoveryState in xlog.h. + */ +RecoveryState +GetRecoveryState(void) +{ + RecoveryState retval; + + SpinLockAcquire(&XLogCtl->info_lck); + retval =3D XLogCtl->SharedRecoveryState; + SpinLockRelease(&XLogCtl->info_lck); + + return retval; +} + /* * Is HotStandby active yet? This is only important in special backends * since normal backends won't ever be able to connect until this returns diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/= transam/xlogarchive.c index c899594fb6..e36a85ccab 100644 --- a/src/backend/access/transam/xlogarchive.c +++ b/src/backend/access/transam/xlogarchive.c @@ -619,18 +619,25 @@ XLogArchiveCheckDone(const char *xlog) { char archiveStatusPath[MAXPGPATH]; struct stat stat_buf; - bool inRecovery =3D RecoveryInProgress(); + + /* The file is always deletable if archive_mode is "off". */ + if (!XLogArchivingActive()) + return true; =20 /* - * The file is always deletable if archive_mode is "off". On standbys - * archiving is disabled if archive_mode is "on", and enabled with - * "always". On a primary, archiving is enabled if archive_mode is "on" - * or "always". + * During archive recovery, the file is deletable if archive_mode is not + * "always". */ - if (!((XLogArchivingActive() && !inRecovery) || - (XLogArchivingAlways() && inRecovery))) + if (!XLogArchivingAlways() && + GetRecoveryState() =3D=3D RECOVERY_STATE_ARCHIVE) return true; =20 + /* + * At this point of the logic, note that we are either a primary with + * archive_mode set to "on" or "always", or a standby with archive_mode + * set to "always". + */ + /* First check for .done --- this means archiver is done with it */ StatusFilePath(archiveStatusPath, xlog, ".done"); if (stat(archiveStatusPath, &stat_buf) =3D=3D 0) diff --git a/src/test/recovery/t/020_archive_status.pl b/src/test/recovery/= t/020_archive_status.pl new file mode 100644 index 0000000000..7cf367d8bf --- /dev/null +++ b/src/test/recovery/t/020_archive_status.pl @@ -0,0 +1,213 @@ +# +# Tests related to WAL archiving and recovery. +# +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests =3D> 16; +use Config; + +my $primary =3D get_new_node('master'); +$primary->init( + has_archiving =3D> 1, + allows_streaming =3D> 1); +$primary->append_conf('postgresql.conf', 'autovacuum =3D off'); +$primary->start; +my $primary_data =3D $primary->data_dir; + +# Temporarily use an archive_command value to make the archiver fail, +# knowing that archiving is enabled. Note that we cannot use a command +# that does not exist as in this case the archiver process would just exit +# without reporting the failure to pg_stat_archiver. This also cannot +# use a plain "false" as that's unportable on Windows. So, instead, as +# a portable solution, use an archive command based on a command known to +# work but will fail: copy with an incorrect original path. +my $incorrect_command =3D + $TestLib::windows_os + ? qq{copy "%p_does_not_exist" "%f_does_not_exist"} + : qq{cp "%p_does_not_exist" "%f_does_not_exist"}; +$primary->safe_psql( + 'postgres', qq{ + ALTER SYSTEM SET archive_command TO '$incorrect_command'; + SELECT pg_reload_conf(); +}); + +# Save the WAL segment currently in use and switch to a new segment. +# This will be used to track the activity of the archiver. +my $segment_name_1 =3D $primary->safe_psql('postgres', + q{SELECT pg_xlogfile_name(pg_current_xlog_location())}); +my $segment_path_1 =3D "pg_xlog/archive_status/$segment_name_1"; +my $segment_path_1_ready =3D "$segment_path_1.ready"; +my $segment_path_1_done =3D "$segment_path_1.done"; +$primary->safe_psql( + 'postgres', q{ + CREATE TABLE mine AS SELECT generate_series(1,10) AS x; + SELECT pg_switch_xlog(); + CHECKPOINT; +}); + +# Wait for an archive failure. +$primary->poll_query_until('postgres', + q{SELECT failed_count > 0 FROM pg_stat_archiver}, 't') + or die "Timed out while waiting for archiving to fail"; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file exists for WAL segment $segment_name_1 waiting to be archive= d" +); +ok( !-f "$primary_data/$segment_path_1_done", + ".done file does not exist for WAL segment $segment_name_1 waiting to be = archived" +); + +is( $primary->safe_psql( + 'postgres', q{ + SELECT archived_count, last_failed_wal + FROM pg_stat_archiver + }), + "0|$segment_name_1", + 'pg_stat_archiver failed to archive $segment_name_1'); + +# Crash the cluster for the next test in charge of checking that non-archi= ved +# WAL segments are not removed. +$primary->stop('immediate'); + +# Recovery tests for the archiving with a standby partially check +# the recovery behavior when restoring a backup taken using a +# snapshot with no pg_start/stop_backup. In this situation, +# the recovered standby should enter first crash recovery then +# switch to regular archive recovery. Note that the base backup +# is taken here so as archive_command will fail. This is necessary +# for the assumptions of the tests done with the standbys below. +$primary->backup_fs_cold('backup'); + +$primary->start; +ok( -f "$primary_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 still exists after crash rec= overy on primary" +); + +# Allow WAL archiving again and wait for a success. +$primary->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); + +$primary->poll_query_until('postgres', + q{SELECT archived_count =3D 1 FROM pg_stat_archiver}) + or die "Timed out while waiting for archiving to finish"; + +ok(!-f "$primary_data/$segment_path_1_ready", + ".ready file for archived WAL segment $segment_name_1 removed"); + +ok(-f "$primary_data/$segment_path_1_done", + ".done file for archived WAL segment $segment_name_1 exists"); + +is( $primary->safe_psql( + 'postgres', q{ SELECT last_archived_wal FROM pg_stat_archiver }), + $segment_name_1, + 'archive success reported in pg_stat_archiver for WAL segment $segment_na= me_1' +); + +# Create some WAL activity and a new checkpoint so as the next standby can +# create a restartpoint. As this standby starts in crash recovery because +# of the cold backup taken previously, it needs a clean restartpoint to de= al +# with existing status files. +my $segment_name_2 =3D $primary->safe_psql('postgres', + q{SELECT pg_xlogfile_name(pg_current_xlog_location())}); +my $segment_path_2 =3D "pg_xlog/archive_status/$segment_name_2"; +my $segment_path_2_ready =3D "$segment_path_2.ready"; +my $segment_path_2_done =3D "$segment_path_2.done"; +$primary->safe_psql( + 'postgres', q{ + INSERT INTO mine SELECT generate_series(10,20) AS x; + SELECT pg_switch_xlog(); + CHECKPOINT; +}); + +$primary->poll_query_until('postgres', + qq{ SELECT last_archived_wal =3D '$segment_name_2' FROM pg_stat_archiver = }) + or die "Timed out while waiting for archiving to finish"; + +# Test standby with archive_mode =3D on. +my $standby1 =3D get_new_node('standby'); +$standby1->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby1->append_conf('postgresql.conf', "archive_mode =3D on"); +my $standby1_data =3D $standby1->data_dir; +$standby1->start; +$standby1->safe_psql('postgres', q{CHECKPOINT}); + +# Recovery with archive_mode=3Don keeps .ready signal files inherited +# from backup after one checkpoint. Note that this WAL segment +# existed in the backup. +ok( -f "$standby1_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 present in backup not remove= d with archive_mode=3Don on standby" +); + +# Recovery with archive_mode=3Don should not create .ready files. +# Note that this segment did not exist in the backup. +ok( !-f "$standby1_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 not created on standby when = archive_mode=3Don on standby" +); + +# Recovery with archive_mode =3D on creates .done files. +ok( -f "$standby1_data/$segment_path_2_done", + ".done file for WAL segment $segment_name_2 created when archive_mode=3Do= n on standby" +); + +# Test recovery with archive_mode =3D always, which should always keep +# .ready files if archiving is enabled, though here we want the archive +# command to fail to persist the .ready files. Note that this node +# has inherited the archive command of the previous cold backup that +# will cause archiving failures. +my $standby2 =3D get_new_node('standby2'); +$standby2->init_from_backup($primary, 'backup', has_restoring =3D> 1); +$standby2->append_conf('postgresql.conf', 'archive_mode =3D always'); +my $standby2_data =3D $standby2->data_dir; +$standby2->start; + +$standby2->safe_psql('postgres', q{CHECKPOINT}); + +ok( -f "$standby2_data/$segment_path_1_ready", + ".ready file for WAL segment $segment_name_1 existing in backup is kept w= ith archive_mode=3Dalways on standby" +); + +ok( -f "$standby2_data/$segment_path_2_ready", + ".ready file for WAL segment $segment_name_2 created with archive_mode=3D= always on standby" +); + +# Reset statistics of the archiver for the next checks. +$standby2->safe_psql('postgres', q{SELECT pg_stat_reset_shared('archiver')= }); + +# Now crash the cluster to check that recovery step does not +# remove non-archived WAL segments on a standby where archiving +# is enabled. +$standby2->stop('immediate'); +$standby2->start; + +ok( -f "$standby2_data/$segment_path_1_ready", + "WAL segment still ready to archive after crash recovery on standby with = archive_mode=3Dalways" +); + +# Allow WAL archiving again, and wait for the segments to be archived. +$standby2->safe_psql( + 'postgres', q{ + ALTER SYSTEM RESET archive_command; + SELECT pg_reload_conf(); +}); +$standby2->poll_query_until('postgres', + qq{SELECT last_archived_wal =3D '$segment_name_2' FROM pg_stat_archiver}) + or die "Timed out while waiting for archiving to finish"; + +is( $standby2->safe_psql( + 'postgres', q{SELECT archived_count FROM pg_stat_archiver}), + '2', + 'correct number of WAL segments archived from standby'); + +ok( !-f "$standby2_data/$segment_path_1_ready" + && !-f "$standby2_data/$segment_path_2_ready", + ".ready files removed after archive success with archive_mode=3Dalways on= standby" +); + +ok( -f "$standby2_data/$segment_path_1_done" + && -f "$standby2_data/$segment_path_2_done", + ".done files created after archive success with archive_mode=3Dalways on = standby" +); --=20 2.26.0 --9iyR+p8Z2cn535Lj-- --451BZW+OUuJBCAYj Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAl6f8wYACgkQnvQgOdby QH3fuw/+M5pcfx/L/4vIJX0EjNhwglpKgmATqUIg5T7LxG9N8vihB+ly0WQlaWZ6 y6azF+xCcgEm+niaaHrs8mbCH2l/GlNW51JtAefTar1b1I4VH8rq+zPScL3X41EO HUEJe0hb6OTpBKkCj1FQ8YNgmaTztFqDtMHK1pvEywpjSUvwPzYkJ/pdSmjOJvDr l1QP0TnDKdRO7F6oUAlZwT2YFils6ZsSpqD+bGpeu5hETaKCmKDsTTGa4zGZApls Sl5YXyJvw2weTltwzUDTKbbDVrkKsSwLprw5quzCdm92IViJTasWNt85NJUZH4hh EGag3tH1yGm7sXWm4ydhqgbgxhrtvBN9YVtuP1eSkrO4XvY4f3x9sXHsVXu+LuGt h3GNzqlSJn+2gGu6D87DaezLnGJkaydB62hV56og1Q5WidEvhL1dSs3jjvplep+f 5ysodUw/yHzK6jhyUq1hz78RzYgnF6waxn/fJE/CnHDZ6D+Zar96YbztHQdRy8az xIJn2+YwWZnSUxBqgSCmSqrVrPVlG1+F7zObvwH7/s0GAjUqoDQqAA5LbnSeMynP 64mf1mUkOgrOaJoFfOM+3lHjl6VZIe1QWSUg/exKMMamvFfOieP3pXUqaDjUYgVD hj7hek9CW1NA3K+f2/hM1F6cor/mrspPrzE0yBP4DVJHy8nRi7o= =sQxI -----END PGP SIGNATURE----- --451BZW+OUuJBCAYj--