Re: pg_rewind : feature to rewind promoted standby is broken!

[Michael Paquier]

On Tue, Mar 12, 2019 at 06:23:01PM +0900, Michael Paquier wrote:
> And you are pointing out to the correct commit.  The issue is that
> process_target_file() has added a call to check_file_excluded(), and
> this skips all the folders which it thinks can be skipped.  One
> problem though is that we also filter out pg_internal.init, which is
> present in each database folder, and remains in the target directory
> marked for deletion.  Then, when the deletion happens, the failure
> happens as the directory is not fully empty.

Okay, here is a refined patch with better comments, the addition of a
test case (creating tables in the new databases in 002_databases.pl is
enough to trigger the problem).

Could you check that it fixes the issue on your side?
--
Michael

diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index bb4ffd0e38..57b77e1840 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -334,12 +334,13 @@ process_target_file(const char *path, file_type_t type, size_t oldsize,
 	file_entry_t *entry;
 
 	/*
-	 * Ignore any path matching the exclusion filters.  This is not actually
-	 * mandatory for target files, but this does not hurt and let's be
-	 * consistent with the source processing.
+	 * Do not apply any exclusion filters here.  The folders marked for
+	 * deletion on the target should be entirely empty, and we also
+	 * check after pg_internal.init which is present in each database
+	 * folder.  Not applying any exclusion filters here also has the
+	 * advantage to remove any files on the target which have been filtered
+	 * from the source.
 	 */
-	if (check_file_excluded(path, false))
-		return;
 
 	snprintf(localpath, sizeof(localpath), "%s/%s", datadir_target, path);
 	if (lstat(localpath, &statbuf) < 0)
diff --git a/src/bin/pg_rewind/t/002_databases.pl b/src/bin/pg_rewind/t/002_databases.pl
index 6dc05720a1..0562c21549 100644
--- a/src/bin/pg_rewind/t/002_databases.pl
+++ b/src/bin/pg_rewind/t/002_databases.pl
@@ -15,19 +15,27 @@ sub run_test
 	RewindTest::setup_cluster($test_mode, ['-g']);
 	RewindTest::start_master();
 
-	# Create a database in master.
+	# Create a database in master with a table.
 	master_psql('CREATE DATABASE inmaster');
+	master_psql('CREATE TABLE inmaster_tab (a int)', 'inmaster');
 
 	RewindTest::create_standby($test_mode);
 
-	# Create another database, the creation is replicated to the standby
+	# Create another database with another table, the creation is
+	# replicated to the standby.
 	master_psql('CREATE DATABASE beforepromotion');
+	master_psql('CREATE TABLE beforepromotion_tab (a int)',
+		    'beforepromotion');
 
 	RewindTest::promote_standby();
 
 	# Create databases in the old master and the new promoted standby.
 	master_psql('CREATE DATABASE master_afterpromotion');
+	master_psql('CREATE TABLE master_promotion_tab (a int)',
+		    'master_afterpromotion');
 	standby_psql('CREATE DATABASE standby_afterpromotion');
+	standby_psql('CREATE TABLE standby_promotion_tab (a int)',
+		    'standby_afterpromotion');
 
 	# The clusters are now diverged.
 
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
index 85cae7e47b..2c98fa5712 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -68,18 +68,20 @@ our $node_standby;
 sub master_psql
 {
 	my $cmd = shift;
+	my $dbname = shift || 'postgres';
 
 	system_or_bail 'psql', '-q', '--no-psqlrc', '-d',
-	  $node_master->connstr('postgres'), '-c', "$cmd";
+	  $node_master->connstr($dbname), '-c', "$cmd";
 	return;
 }
 
 sub standby_psql
 {
 	my $cmd = shift;
+	my $dbname = shift || 'postgres';
 
 	system_or_bail 'psql', '-q', '--no-psqlrc', '-d',
-	  $node_standby->connstr('postgres'), '-c', "$cmd";
+	  $node_standby->connstr($dbname), '-c', "$cmd";
 	return;
 }
 

signature.asc
Description: PGP signature