[prev in list] [next in list] [prev in thread] [next in thread]
List: pgsql-hackers
Subject: Re: [HACKERS] pg_rewind tests
From: Michael Paquier <michael.paquier () gmail ! com>
Date: 2015-03-31 7:37:50
Message-ID: CAB7nPqS9CWD=xDb6uUGCbL7id+sDY0b9f9Q7N-AGZK3zmk010Q () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Tue, Mar 31, 2015 at 9:59 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
> There are some small issues with the pg_rewind tests.
>
> This technique
>
> check: all
> $(prove_check) :: local
> $(prove_check) :: remote
>
> for passing arguments to "prove" does not work with the tools included
> in Perl 5.8.
>
> While sorting out the portability issues in the TAP framework during the
> 9.4 release cycle, we had set 5.8 as the oldest Perl version that is
> supported. (It's the Perl version in RHEL 5.) I suggest using
> environment variables instead, unless we want to change that.
>
That's good to know. And I think that by using environment variables it is
necessary to pass an additional flag to prove_check (see
PG_PROVE_TEST_MODE) in the patch attached because prove_check kicks several
instructions, a command mkdir to begin with.
> Moreover,
>
> if ($test_mode == "local")
> ...
> elsif ($test_mode == "remote")
>
> don't work, because those are numerical comparisons, not string
> comparisons. So the remote branch is never actually run.
>
That's "eq" I guess.
> Finally, RewindTest.pm should use
>
> use strict;
> use warnings;
>
> and the warnings caused by that should be addressed.
>
All those things addressed give more or less the patch attached.
While looking at that I noticed two additional issues:
- In remote mode, the connection string to the promoted standby was
incorrect when running pg_rewind, leading to connection errors
- At least in my environment, a sleep of 1 after the standby promotion was
not sufficient to make the tests work.
Regards,
--
Michael
[Attachment #5 (text/html)]
<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar \
31, 2015 at 9:59 AM, Peter Eisentraut <span dir="ltr"><<a \
href="mailto:peter_e@gmx.net" target="_blank">peter_e@gmx.net</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex">There are some small issues with the pg_rewind \
tests.<br> <br>
This technique<br>
<br>
check: all<br>
$(prove_check) :: local<br>
$(prove_check) :: remote<br>
<br>
for passing arguments to "prove" does not work with the tools included<br>
in Perl 5.8.<br>
<br>
While sorting out the portability issues in the TAP framework during the<br>
9.4 release cycle, we had set 5.8 as the oldest Perl version that is<br>
supported. (It's the Perl version in RHEL 5.) I suggest using<br>
environment variables instead, unless we want to change \
that.<br></blockquote><div><br></div><div>That's good to know. And I think that \
by using environment variables it is necessary to pass an additional flag to \
prove_check (see PG_PROVE_TEST_MODE) in the patch attached because prove_check kicks \
several instructions, a command mkdir to begin with.<br></div><div> </div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Moreover,<br>
<br>
if ($test_mode == "local")<br>
...<br>
elsif ($test_mode == "remote")<br>
<br>
don't work, because those are numerical comparisons, not string<br>
comparisons. So the remote branch is never actually \
run.<br></blockquote><div><br></div><div>That's "eq" I \
guess.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"> Finally, RewindTest.pm should \
use<br> <br>
use strict;<br>
use warnings;<br>
<br>
and the warnings caused by that should be addressed.<span><font \
color="#888888"><br></font></span></blockquote><div> </div></div>All those things \
addressed give more or less the patch attached.<br><br></div><div \
class="gmail_extra">While looking at that I noticed two additional \
issues:<br></div><div class="gmail_extra">- In remote mode, the connection string to \
the promoted standby was incorrect when running pg_rewind, leading to connection \
errors<br></div><div class="gmail_extra">- At least in my environment, a sleep of 1 \
after the standby promotion was not sufficient to make the tests work.<br></div><div \
class="gmail_extra">Regards,<br></div><div class="gmail_extra">-- \
<br><div>Michael<br></div> </div></div>
["20150330_pgrewind_tap_fixes.patch" (text/x-patch)]
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 7c39d82..4108783 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -323,7 +323,7 @@ endef
define prove_check
$(MKDIR_P) tmp_check/log
$(MAKE) -C $(top_builddir) DESTDIR='$(CURDIR)'/tmp_check/install install \
>'$(CURDIR)'/tmp_check/log/install.log 2>&1
-cd $(srcdir) && TESTDIR='$(CURDIR)' \
PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call \
add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) \
top_builddir='$(CURDIR)/$(top_builddir)' PGPORT='6$(DEF_PGPORT)' $(PROVE) \
$(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl +cd $(srcdir) && TESTDIR='$(CURDIR)' \
PG_PROVE_TEST_MODE='$(PG_PROVE_TEST_MODE)' \
PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call \
add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) \
top_builddir='$(CURDIR)/$(top_builddir)' PGPORT='6$(DEF_PGPORT)' $(PROVE) \
$(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl endef
else
diff --git a/src/bin/pg_rewind/Makefile b/src/bin/pg_rewind/Makefile
index efd4988..2bda545 100644
--- a/src/bin/pg_rewind/Makefile
+++ b/src/bin/pg_rewind/Makefile
@@ -47,6 +47,8 @@ clean distclean maintainer-clean:
rm -f pg_rewind$(X) $(OBJS) xlogreader.c
rm -rf tmp_check regress_log
-check: all
- $(prove_check) :: local
- $(prove_check) :: remote
+check:
+ $(eval PG_PROVE_TEST_MODE = local)
+ $(prove_check)
+ $(eval PG_PROVE_TEST_MODE = remote)
+ $(prove_check)
diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm
index 0f8f4ca..f748bd1 100644
--- a/src/bin/pg_rewind/RewindTest.pm
+++ b/src/bin/pg_rewind/RewindTest.pm
@@ -29,6 +29,9 @@ package RewindTest;
# master and standby servers. The data directories are also available
# in paths $test_master_datadir and $test_standby_datadir
+use strict;
+use warnings;
+
use TestLib;
use Test::More;
@@ -58,8 +61,8 @@ our @EXPORT = qw(
# Adjust these paths for your environment
my $testroot = "./tmp_check";
-$test_master_datadir="$testroot/data_master";
-$test_standby_datadir="$testroot/data_standby";
+our $test_master_datadir="$testroot/data_master";
+our $test_standby_datadir="$testroot/data_standby";
mkdir $testroot;
@@ -73,8 +76,8 @@ my $port_standby=$port_master + 1;
my $log_path;
my $tempdir_short;
-$connstr_master="port=$port_master";
-$connstr_standby="port=$port_standby";
+my $connstr_master="port=$port_master";
+my $connstr_standby="port=$port_standby";
$ENV{PGDATABASE} = "postgres";
@@ -127,7 +130,8 @@ sub append_to_file
sub init_rewind_test
{
- ($testname, $test_mode) = @_;
+ my $testname = shift;
+ my $test_mode = shift;
$log_path="regress_log/pg_rewind_log_${testname}_${test_mode}";
@@ -195,11 +199,13 @@ sub promote_standby
# Now promote slave and insert some new data on master, this will put
# the master out-of-sync with the standby.
system_or_bail("pg_ctl -w -D $test_standby_datadir promote >>$log_path 2>&1");
- sleep 1;
+ sleep 2;
}
sub run_pg_rewind
{
+ my $test_mode = shift;
+
# Stop the master and be ready to perform the rewind
system_or_bail("pg_ctl -w -D $test_master_datadir stop -m fast >>$log_path 2>&1");
@@ -212,7 +218,7 @@ sub run_pg_rewind
# overwritten during the rewind.
copy("$test_master_datadir/postgresql.conf", \
"$testroot/master-postgresql.conf.tmp"); # Now run pg_rewind
- if ($test_mode == "local")
+ if ($test_mode eq "local")
{
# Do rewind using a local pgdata as source
# Stop the master and be ready to perform the rewind
@@ -225,12 +231,14 @@ sub run_pg_rewind
'>>', $log_path, '2>&1');
ok ($result, 'pg_rewind local');
}
- elsif ($test_mode == "remote")
+ elsif ($test_mode eq "remote")
{
# Do rewind using a remote connection as source
+ printf "Port standby $port_standby\n";
+ printf "Port standby $test_master_datadir\n";
my $result =
run(['./pg_rewind',
- "--source-server=\"port=$port_standby dbname=postgres\"",
+ "--source-server", "port=$port_standby dbname=postgres",
"--target-pgdata=$test_master_datadir"],
'>>', $log_path, '2>&1');
ok ($result, 'pg_rewind remote');
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index 7198a3a..132892d 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -7,7 +7,7 @@ use RewindTest;
my $testmode = shift;
-RewindTest::init_rewind_test('basic', $testmode);
+RewindTest::init_rewind_test('basic', $ENV{PG_PROVE_TEST_MODE});
RewindTest::setup_cluster();
# Create a test table and insert a row in master.
@@ -57,7 +57,7 @@ master_psql("INSERT INTO trunc_tbl SELECT 'in master, after \
promotion: ' || g FR master_psql("DELETE FROM tail_tbl WHERE id > 10");
master_psql("VACUUM tail_tbl");
-RewindTest::run_pg_rewind();
+RewindTest::run_pg_rewind($ENV{PG_PROVE_TEST_MODE});
check_query('SELECT * FROM tbl1',
qq(in master
diff --git a/src/bin/pg_rewind/t/002_databases.pl \
b/src/bin/pg_rewind/t/002_databases.pl index 709c81e..61a5588 100644
--- a/src/bin/pg_rewind/t/002_databases.pl
+++ b/src/bin/pg_rewind/t/002_databases.pl
@@ -5,9 +5,7 @@ use Test::More tests => 2;
use RewindTest;
-my $testmode = shift;
-
-RewindTest::init_rewind_test('databases', $testmode);
+RewindTest::init_rewind_test('databases', $ENV{PG_PROVE_TEST_MODE});
RewindTest::setup_cluster();
# Create a database in master.
@@ -25,7 +23,7 @@ master_psql('CREATE DATABASE master_afterpromotion');
standby_psql('CREATE DATABASE standby_afterpromotion');
# The clusters are now diverged.
-RewindTest::run_pg_rewind();
+RewindTest::run_pg_rewind($ENV{PG_PROVE_TEST_MODE});
# Check that the correct databases are present after pg_rewind.
check_query('SELECT datname FROM pg_database',
diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl \
b/src/bin/pg_rewind/t/003_extrafiles.pl index dd76fb8..975064b 100644
--- a/src/bin/pg_rewind/t/003_extrafiles.pl
+++ b/src/bin/pg_rewind/t/003_extrafiles.pl
@@ -11,9 +11,11 @@ use RewindTest;
my $testmode = shift;
-RewindTest::init_rewind_test('extrafiles', $testmode);
+RewindTest::init_rewind_test('extrafiles', $ENV{PG_PROVE_TEST_MODE});
RewindTest::setup_cluster();
+my $test_master_datadir = $RewindTest::test_master_datadir;
+
# Create a subdir and files that will be present in both
mkdir "$test_master_datadir/tst_both_dir";
append_to_file "$test_master_datadir/tst_both_dir/both_file1", "in both1";
@@ -38,7 +40,7 @@ mkdir "$test_master_datadir/tst_master_dir/master_subdir/";
append_to_file "$test_master_datadir/tst_master_dir/master_subdir/master_file3", "in \
master3";
RewindTest::promote_standby();
-RewindTest::run_pg_rewind();
+RewindTest::run_pg_rewind($ENV{PG_PROVE_TEST_MODE});
# List files in the data directory after rewind.
my @paths;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic