From: Alexey Kondratov Date: Fri, 4 Oct 2019 17:14:12 +0300 Subject: [PATCH v3 1/2] Improve functionality, docs and tests of -R, --no-ensure-shutdown and --dry-run options Branch: pg-rewind-fixes --- doc/src/sgml/ref/pg_rewind.sgml | 10 +-- src/bin/pg_rewind/pg_rewind.c | 19 +++--- src/bin/pg_rewind/t/001_basic.pl | 2 +- src/bin/pg_rewind/t/002_databases.pl | 2 +- src/bin/pg_rewind/t/003_extrafiles.pl | 2 +- src/bin/pg_rewind/t/004_pg_xlog_symlink.pl | 2 +- src/bin/pg_rewind/t/005_same_timeline.pl | 32 +++++++--- src/bin/pg_rewind/t/RewindTest.pm | 71 +++++++++++++++++----- 8 files changed, 103 insertions(+), 37 deletions(-) diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml index fbf454803b..42d29edd4e 100644 --- a/doc/src/sgml/ref/pg_rewind.sgml +++ b/doc/src/sgml/ref/pg_rewind.sgml @@ -169,12 +169,14 @@ PostgreSQL documentation - pg_rewind verifies that the target server - is cleanly shutdown before rewinding; by default, if it isn't, it - starts the server in single-user mode to complete crash recovery. + pg_rewind requires that the target server + is cleanly shut down before rewinding. By default, if the target server + is not shut down cleanly, pg_rewind starts + the target server in single-user mode to complete crash recovery first, + and stops it. By passing this option, pg_rewind skips this and errors out immediately if the server is not cleanly shut - down. Users are expected to handle the situation themselves in that + down. Users are expected to handle the situation themselves in that case. diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index fe1468b771..875a43b219 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -270,11 +270,12 @@ main(int argc, char **argv) pg_free(buffer); /* - * If the target instance was not cleanly shut down, run a single-user - * postgres session really quickly and reload the control file to get the - * new state. Note if no_ensure_shutdown is specified, pg_rewind won't do - * that automatically. That means users need to do themselves in advance, - * else pg_rewind will soon quit, see sanityChecks(). + * If the target instance was not cleanly shut down, start and stop the + * target cluster once in single-user mode to enforce recovery to finish, + * ensuring that the cluster can be used by pg_rewind. Note that if + * no_ensure_shutdown is specified, pg_rewind ignores this step, and users + * need to make sure by themselves that the target cluster is in a clean + * state. */ if (!no_ensure_shutdown && ControlFile_target.state != DB_SHUTDOWNED && @@ -847,8 +848,12 @@ ensureCleanShutdown(const char *argv0) if (dry_run) return; - /* finally run postgres in single-user mode */ - snprintf(cmd, MAXCMDLEN, "\"%s\" --single -D \"%s\" template1 < \"%s\"", + /* + * Finally run postgres in single-user mode. There is no need to use + * fsync here. This makes the recovery faster, and the target data folder + * is synced at the end anyway. + */ + snprintf(cmd, MAXCMDLEN, "\"%s\" --single -F -D \"%s\" template1 < \"%s\"", exec_path, datadir_target, DEVNULL); if (system(cmd) != 0) diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl index c3293e93df..a1659460ec 100644 --- 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 => 11; +use Test::More tests => 14; use FindBin; use lib $FindBin::RealBin; diff --git a/src/bin/pg_rewind/t/002_databases.pl b/src/bin/pg_rewind/t/002_databases.pl index 1db534c0dc..921c4434f5 100644 --- a/src/bin/pg_rewind/t/002_databases.pl +++ b/src/bin/pg_rewind/t/002_databases.pl @@ -1,7 +1,7 @@ use strict; use warnings; use TestLib; -use Test::More tests => 7; +use Test::More tests => 10; use FindBin; use lib $FindBin::RealBin; diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl b/src/bin/pg_rewind/t/003_extrafiles.pl index f4710440fc..bce5b47148 100644 --- a/src/bin/pg_rewind/t/003_extrafiles.pl +++ b/src/bin/pg_rewind/t/003_extrafiles.pl @@ -3,7 +3,7 @@ use strict; use warnings; use TestLib; -use Test::More tests => 5; +use Test::More tests => 8; use File::Find; diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl index 639eeb9c91..a501be8f78 100644 --- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl +++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl @@ -14,7 +14,7 @@ if ($windows_os) } else { - plan tests => 5; + plan tests => 8; } use FindBin; diff --git a/src/bin/pg_rewind/t/005_same_timeline.pl b/src/bin/pg_rewind/t/005_same_timeline.pl index 40dbc44caa..1d96740a55 100644 --- a/src/bin/pg_rewind/t/005_same_timeline.pl +++ b/src/bin/pg_rewind/t/005_same_timeline.pl @@ -1,19 +1,35 @@ +# +# Test that running pg_rewind with the source and target clusters +# on the same timeline runs successfully. +# use strict; use warnings; use TestLib; -use Test::More tests => 1; +use Test::More tests => 6; use FindBin; use lib $FindBin::RealBin; use RewindTest; -# Test that running pg_rewind if the two clusters are on the same -# timeline runs successfully. +sub run_test +{ + my $test_mode = shift; + + RewindTest::setup_cluster($test_mode, ['-g']); + RewindTest::start_master(); + RewindTest::create_standby($test_mode); + + RewindTest::promote_standby(); + + RewindTest::run_pg_rewind($test_mode); + + RewindTest::clean_rewind_test(); + return; +} + +# Run the test in both modes. +run_test('local'); +run_test('remote'); -RewindTest::setup_cluster(); -RewindTest::start_master(); -RewindTest::create_standby(); -RewindTest::run_pg_rewind('local'); -RewindTest::clean_rewind_test(); exit(0); diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm index c540722420..9292092b1a 100644 --- a/src/bin/pg_rewind/t/RewindTest.pm +++ b/src/bin/pg_rewind/t/RewindTest.pm @@ -227,9 +227,57 @@ sub run_pg_rewind # Append the rewind-specific role to the connection string. $standby_connstr = "$standby_connstr user=rewind_user"; - # Stop the master and be ready to perform the rewind + # Stop the master cleanly to check dry-run mode. $node_master->stop; + # First run pg_rewind in dry-run mode + if ($test_mode eq "local") + { + + # Stop the new master and be ready to perform the rewind + # Do rewind in dry-run mode using a local pgdata as source + $node_standby->stop; + command_ok( + [ + 'pg_rewind', + "--debug", "--no-sync", "--dry-run", + "--source-pgdata=$standby_pgdata", + "--target-pgdata=$master_pgdata", + "--no-ensure-shutdown" + ], + 'pg_rewind local with --dry-run'); + } + elsif ($test_mode eq "remote") + { + + # Do rewind in dry-run mode using a remote connection as source + command_ok( + [ + 'pg_rewind', + "--debug", "--no-sync", "--dry-run", + "--source-server", $standby_connstr, + "--target-pgdata=$master_pgdata", + "--write-recovery-conf", "--progress" + ], + 'pg_rewind remote with --dry-run'); + + # Check that standby.signal hasn't been created. + ok(! -e "$master_pgdata/standby.signal", + 'standby.signal is absent'); + } + else + { + + # Cannot come here normally + croak("Incorrect test mode specified"); + } + + # Start and stop the master and be ready to perform the rewind. + # The cluster needs recovery to finish once, and pg_rewind makes + # sure that it happens automatically. + $node_master->start; + $node_master->stop('immediate'); + # At this point, the rewind processing is ready to run. # We now have a very simple scenario with a few diverged WAL record. # The real testing begins really now with a bifurcation of the possible @@ -246,8 +294,6 @@ sub run_pg_rewind { # Do rewind using a local pgdata as source - # Stop the master and be ready to perform the rewind - $node_standby->stop; command_ok( [ 'pg_rewind', @@ -261,18 +307,21 @@ sub run_pg_rewind elsif ($test_mode eq "remote") { - # Do rewind using a remote connection as source + # Do rewind using a remote connection as source, generating + # recovery configuration automatically. command_ok( [ 'pg_rewind', "--debug", "--source-server", $standby_connstr, - "--target-pgdata=$master_pgdata", "-R", - "--no-sync" + "--target-pgdata=$master_pgdata", "--no-sync", + "--write-recovery-conf" ], 'pg_rewind remote'); - # Check that standby.signal has been created. - ok(-e "$master_pgdata/standby.signal"); + # Check that standby.signal is here as recovery configuration + # was requested. + ok( -e "$master_pgdata/standby.signal", + 'standby.signal created after pg_rewind'); # Now, when pg_rewind apparently succeeded with minimal permissions, # add REPLICATION privilege. So we could test that new standby @@ -280,12 +329,6 @@ sub run_pg_rewind $node_standby->safe_psql('postgres', "ALTER ROLE rewind_user WITH REPLICATION;"); } - else - { - - # Cannot come here normally - croak("Incorrect test mode specified"); - } # Now move back postgresql.conf with old settings move( base-commit: 6837632b758e0470a2692d9f8303e8aebd6fbd8f -- 2.17.1 --------------4FB4DA99007DA85F944316F9 Content-Type: text/x-patch; name="v3-0002-WIP-increase-pg_rewind-test-coverage.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename*0="v3-0002-WIP-increase-pg_rewind-test-coverage.patch"