Mark commit and abort WAL records with XLR_SPECIAL_REL_UPDATE.

hlinnaka · hlinnaka · commit a28d731a1187 · 2020-08-17T10:52:58.000+03:00
If a commit or abort record includes "dropped relfilenodes", then replaying the record will remove data files. That is surely a "special rel update", but the records were not marked as such. Fix that, teach pg_rewind to expect and ignore them, and add a test case to cover it. It's always been like this, but no backporting for fear of breaking existing applications. If an application parsed the WAL but was not handling commit/abort records, it would stop working. That might be a good thing if it really needed to handle the dropped rels, but it will be caught when the application is updated to work with PostgreSQL v14 anyway. Discussion: https://www.postgresql.org/message-id/07b33e2c-46a6-86a1-5f9e-a7da73fddb95%40iki.fi Reviewed-by: Amit Kapila, Michael Paquier
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
@@ -5565,6 +5565,7 @@ XactLogCommitRecord(TimestampTz commit_time,
 	{
 		xl_xinfo.xinfo |= XACT_XINFO_HAS_RELFILENODES;
 		xl_relfilenodes.nrels = nrels;
+		info |= XLR_SPECIAL_REL_UPDATE;
 	}
 
 	if (nmsgs > 0)
@@ -5697,6 +5698,7 @@ XactLogAbortRecord(TimestampTz abort_time,
 	{
 		xl_xinfo.xinfo |= XACT_XINFO_HAS_RELFILENODES;
 		xl_relfilenodes.nrels = nrels;
+		info |= XLR_SPECIAL_REL_UPDATE;
 	}
 
 	if (TransactionIdIsValid(twophase_xid))
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
@@ -14,6 +14,7 @@
 #include <unistd.h>
 
 #include "access/rmgr.h"
+#include "access/xact.h"
 #include "access/xlog_internal.h"
 #include "access/xlogreader.h"
 #include "catalog/pg_control.h"
@@ -397,6 +398,18 @@ extractPageInfo(XLogReaderState *record)
 		 * source system.
 		 */
 	}
+	else if (rmid == RM_XACT_ID &&
+			 ((rminfo & XLOG_XACT_OPMASK) == XLOG_XACT_COMMIT ||
+			  (rminfo & XLOG_XACT_OPMASK) == XLOG_XACT_COMMIT_PREPARED ||
+			  (rminfo & XLOG_XACT_OPMASK) == XLOG_XACT_ABORT ||
+			  (rminfo & XLOG_XACT_OPMASK) == XLOG_XACT_ABORT_PREPARED))
+	{
+		/*
+		 * These records can include "dropped rels". We can safely ignore
+		 * them, we will see that they are missing and copy them from the
+		 * source.
+		 */
+	}
 	else if (info & XLR_SPECIAL_REL_UPDATE)
 	{
 		/*
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
@@ -1,7 +1,7 @@
 use strict;
 use warnings;
 use TestLib;
-use Test::More tests => 20;
+use Test::More tests => 23;
 
 use FindBin;
 use lib $FindBin::RealBin;
@@ -29,6 +29,10 @@ sub run_test
 	primary_psql("CREATE TABLE tail_tbl (id integer, d text)");
 	primary_psql("INSERT INTO tail_tbl VALUES (0, 'in primary')");
 
+	# This test table is dropped in the old primary after promotion.
+	primary_psql("CREATE TABLE drop_tbl (d text)");
+	primary_psql("INSERT INTO drop_tbl VALUES ('in primary')");
+
 	primary_psql("CHECKPOINT");
 
 	RewindTest::create_standby($test_mode);
@@ -66,6 +70,9 @@ sub run_test
 	primary_psql("DELETE FROM tail_tbl WHERE id > 10");
 	primary_psql("VACUUM tail_tbl");
 
+	# Drop drop_tbl. pg_rewind should copy it back.
+	primary_psql("DROP TABLE drop_tbl");
+
 	# Before running pg_rewind, do a couple of extra tests with several
 	# option combinations.  As the code paths taken by those tests
 	# do not change for the "local" and "remote" modes, just run them
@@ -154,6 +161,12 @@ sub run_test
 ),
 		'tail-copy');
 
+	check_query(
+		'SELECT * FROM drop_tbl',
+		qq(in primary
+),
+		'drop');
+
 	# Permissions on PGDATA should be default
   SKIP:
 	{

Original file line number	Diff line number	Diff line change
`@@ -5565,6 +5565,7 @@ XactLogCommitRecord(TimestampTz commit_time,`
`5565`	`5565`	`{`
`5566`	`5566`	`xl_xinfo.xinfo \|= XACT_XINFO_HAS_RELFILENODES;`
`5567`	`5567`	`xl_relfilenodes.nrels = nrels;`
	`5568`	`+ info \|= XLR_SPECIAL_REL_UPDATE;`
`5568`	`5569`	`}`
`5569`	`5570`
`5570`	`5571`	`if (nmsgs > 0)`
`@@ -5697,6 +5698,7 @@ XactLogAbortRecord(TimestampTz abort_time,`
`5697`	`5698`	`{`
`5698`	`5699`	`xl_xinfo.xinfo \|= XACT_XINFO_HAS_RELFILENODES;`
`5699`	`5700`	`xl_relfilenodes.nrels = nrels;`
	`5701`	`+ info \|= XLR_SPECIAL_REL_UPDATE;`
`5700`	`5702`	`}`
`5701`	`5703`
`5702`	`5704`	`if (TransactionIdIsValid(twophase_xid))`