Re: pg_wal/RECOVERYHISTORY file remains after archive recovery

[Michael Paquier]

On Mon, Sep 30, 2019 at 12:53:58PM +0900, Masahiko Sawada wrote:
> I think that the above checks are always true because isnt() function
> checks if the 1st argument and 2nd argument are not the same.

Dammit.  I overlooked this part of the module's doc.

> I've attached the updated version patch including the tests. Please
> review it.

Thanks, your test allows to reproduce the original problem, so that's
nice.  I don't have much to say, except some improvements to the
comments of the test as per the attached.  What do you think?
--
Michael

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 61ba6b852e..790e2c8714 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5461,7 +5461,6 @@ validateRecoveryParameters(void)
 static void
 exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
 {
-	char		recoveryPath[MAXPGPATH];
 	char		xlogfname[MAXFNAMELEN];
 	XLogSegNo	endLogSegNo;
 	XLogSegNo	startLogSegNo;
@@ -5541,17 +5540,6 @@ exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
 	XLogFileName(xlogfname, ThisTimeLineID, startLogSegNo, wal_segment_size);
 	XLogArchiveCleanup(xlogfname);
 
-	/*
-	 * Since there might be a partial WAL segment named RECOVERYXLOG, get rid
-	 * of it.
-	 */
-	snprintf(recoveryPath, MAXPGPATH, XLOGDIR "/RECOVERYXLOG");
-	unlink(recoveryPath);		/* ignore any error */
-
-	/* Get rid of any remaining recovered timeline-history file, too */
-	snprintf(recoveryPath, MAXPGPATH, XLOGDIR "/RECOVERYHISTORY");
-	unlink(recoveryPath);		/* ignore any error */
-
 	/*
 	 * Remove the signal files out of the way, so that we don't accidentally
 	 * re-enter archive recovery mode in a subsequent crash.
@@ -7433,6 +7421,7 @@ StartupXLOG(void)
 	if (ArchiveRecoveryRequested)
 	{
 		char		reason[200];
+		char		recoveryPath[MAXPGPATH];
 
 		Assert(InArchiveRecovery);
 
@@ -7489,6 +7478,17 @@ StartupXLOG(void)
 		 */
 		writeTimeLineHistory(ThisTimeLineID, recoveryTargetTLI,
 							 EndRecPtr, reason);
+
+		/*
+		 * Since there might be a partial WAL segment named RECOVERYXLOG, get
+		 * rid of it.
+		 */
+		snprintf(recoveryPath, MAXPGPATH, XLOGDIR "/RECOVERYXLOG");
+		unlink(recoveryPath);	/* ignore any error */
+
+		/* Get rid of any remaining recovered timeline-history file, too */
+		snprintf(recoveryPath, MAXPGPATH, XLOGDIR "/RECOVERYHISTORY");
+		unlink(recoveryPath);	/* ignore any error */
 	}
 
 	/* Save the selected TimeLineID in shared memory, too */
diff --git a/src/test/recovery/t/002_archiving.pl b/src/test/recovery/t/002_archiving.pl
index e1bd3c95cc..b1c4694cdf 100644
--- a/src/test/recovery/t/002_archiving.pl
+++ b/src/test/recovery/t/002_archiving.pl
@@ -3,7 +3,7 @@ use strict;
 use warnings;
 use PostgresNode;
 use TestLib;
-use Test::More tests => 1;
+use Test::More tests => 3;
 use File::Copy;
 
 # Initialize master node, doing archives
@@ -49,3 +49,28 @@ $node_standby->poll_query_until('postgres', $caughtup_query)
 my $result =
   $node_standby->safe_psql('postgres', "SELECT count(*) FROM tab_int");
 is($result, qq(1000), 'check content from archives');
+
+# Check the presence of temporary files specifically generated during
+# archive recovery.  To ensure the presence of the temporary history
+# file, switch to a timeline large enough to allow a standby to recover
+# a history file from an archive.  As this requires at least two timeline
+# switches, promote the existing standby first.  Then create a second
+# standby based on the promoted one.  Finally, the second standby is
+# promoted.
+$node_standby->promote;
+
+my $node_standby2 = get_new_node('standby2');
+$node_standby2->init_from_backup($node_master, $backup_name,
+				 has_restoring => 1);
+$node_standby2->start;
+
+# Now promote standby2, and check that files specifically generated during
+# archive recovery are removed by the end of recovery.
+$node_standby2->promote;
+my $node_standby2_data = $node_standby2->data_dir;
+ok(
+	! -f "$node_standby2_data/pg_wal/RECOVERYHISTORY",
+	"RECOVERYHISTORY removed after promotion");
+ok(
+	! -f "$node_standby2_data/pg_wal/RECOVERYXLOG",
+	"RECOVERYXLOG removed after promotion");

signature.asc
Description: PGP signature