public inbox for [email protected]
help / color / mirror / Atom feedFrom: Alexey Kondratov <[email protected]>
Subject: [PATCH v3 1/2] Improve functionality, docs and tests of -R, --no-ensure-shutdown and --dry-run options
Date: Fri, 4 Oct 2019 17:14:12 +0300
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
<term><option>--no-ensure-shutdown</option></term>
<listitem>
<para>
- <application>pg_rewind</application> 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.
+ <application>pg_rewind</application> requires that the target server
+ is cleanly shut down before rewinding. By default, if the target server
+ is not shut down cleanly, <application>pg_rewind</application> starts
+ the target server in single-user mode to complete crash recovery first,
+ and stops it.
By passing this option, <application>pg_rewind</application> 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.
</para>
</listitem>
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"
view thread (15+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH v3 1/2] Improve functionality, docs and tests of -R, --no-ensure-shutdown and --dry-run options
In-Reply-To: <no-message-id-1882286@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox