pg_rewind: Fix thinko in parsing target WAL.

hlinnaka · hlinnaka · commit 72d172743e52 · 2020-11-10T19:25:46.000+02:00
It's entirely possible to see WAL for a relation that doesn't exist in the target anymore. That happens when the relation was dropped later. The refactoring in commit eb00f1d broke that case, by sanity-checking the file type in the target before checking the flag forwhether it exists there at all. I noticed this during manual testing. Modify the 001_basic.pl test so that it covers this case.
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
@@ -324,17 +324,20 @@ process_target_wal_block_change(ForkNumber forknum, RelFileNode rnode,
 	{
 		Assert(entry->isrelfile);
 
-		if (entry->target_type != FILE_TYPE_REGULAR)
-			pg_fatal("unexpected page modification for non-regular file \"%s\"",
-					 entry->path);
-
-		if (entry->target_exists && entry->source_exists)
+		if (entry->target_exists)
 		{
-			off_t		end_offset;
+			if (entry->target_type != FILE_TYPE_REGULAR)
+				pg_fatal("unexpected page modification for non-regular file \"%s\"",
+						 entry->path);
 
-			end_offset = (blkno_inseg + 1) * BLCKSZ;
-			if (end_offset <= entry->source_size && end_offset <= entry->target_size)
-				datapagemap_add(&entry->target_pages_to_overwrite, blkno_inseg);
+			if (entry->source_exists)
+			{
+				off_t		end_offset;
+
+				end_offset = (blkno_inseg + 1) * BLCKSZ;
+				if (end_offset <= entry->source_size && end_offset <= entry->target_size)
+					datapagemap_add(&entry->target_pages_to_overwrite, blkno_inseg);
+			}
 		}
 	}
 }
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
@@ -71,6 +71,7 @@ sub run_test
 	primary_psql("VACUUM tail_tbl");
 
 	# Drop drop_tbl. pg_rewind should copy it back.
+	primary_psql("insert into drop_tbl values ('in primary, after promotion')");
 	primary_psql("DROP TABLE drop_tbl");
 
 	# Before running pg_rewind, do a couple of extra tests with several

Original file line number	Diff line number	Diff line change
`@@ -324,17 +324,20 @@ process_target_wal_block_change(ForkNumber forknum, RelFileNode rnode,`
`324`	`324`	`{`
`325`	`325`	`Assert(entry->isrelfile);`
`326`	`326`
`327`		`- if (entry->target_type != FILE_TYPE_REGULAR)`
`328`		`- pg_fatal("unexpected page modification for non-regular file \"%s\"",`
`329`		`- entry->path);`
`330`		`-`
`331`		`- if (entry->target_exists && entry->source_exists)`
	`327`	`+ if (entry->target_exists)`
`332`	`328`	`{`
`333`		`- off_t end_offset;`
	`329`	`+ if (entry->target_type != FILE_TYPE_REGULAR)`
	`330`	`+ pg_fatal("unexpected page modification for non-regular file \"%s\"",`
	`331`	`+ entry->path);`
`334`	`332`
`335`		`- end_offset = (blkno_inseg + 1) * BLCKSZ;`
`336`		`- if (end_offset <= entry->source_size && end_offset <= entry->target_size)`
`337`		`- datapagemap_add(&entry->target_pages_to_overwrite, blkno_inseg);`
	`333`	`+ if (entry->source_exists)`
	`334`	`+ {`
	`335`	`+ off_t end_offset;`
	`336`	`+`
	`337`	`+ end_offset = (blkno_inseg + 1) * BLCKSZ;`
	`338`	`+ if (end_offset <= entry->source_size && end_offset <= entry->target_size)`
	`339`	`+ datapagemap_add(&entry->target_pages_to_overwrite, blkno_inseg);`
	`340`	`+ }`
`338`	`341`	`}`
`339`	`342`	`}`
`340`	`343`	`}`