On Tue, Mar 31, 2015 at 9:59 AM, Peter Eisentraut <[email protected]> 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
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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
