Hi,
Rebased and updated based on feedback. Responses to multiple emails below:
On Thu, Dec 16, 2021 at 1:22 PM Michael Paquier <mich...@paquier.xyz> wrote:
> On Wed, Dec 15, 2021 at 05:50:45PM +0900, Michael Paquier wrote:
> > Hmm. FWIW, I am working on doing similar for pg_upgrade to switch to
> > TAP there, and we share a lot in terms of running pg_regress on an
> > exising cluster. Wouldn't it be better to move this definition to
> > src/Makefile.global.in rather than src/test/recovery/?
> >
> > My pg_regress command is actually very similar to yours, so I am
> > wondering if this would be better if somehow centralized, perhaps in
> > Cluster.pm.
Thanks for looking. Right, it sounds like you'll have the same
problems I ran into. I haven't updated this patch for that yet, as
I'm not sure exactly what you need and we could easily move it in a
later commit. Does that seem reasonable?
> By the way, while I was sorting out my things, I have noticed that v4
> does not handle EXTRA_REGRESS_OPT. Is that wanted? You could just
> add that into your patch set and push the extra options to the
> pg_regress command:
> my $extra_opts_val = $ENV{EXTRA_REGRESS_OPT} || "";
> my @extra_opts = split(/\s+/, $extra_opts_val);
Seems like a good idea for consistency, but isn't that a variable
that's supposed to be expanded by a shell, not naively split on
whitespace? Perhaps we should use the single-argument variant of
system(), so the whole escaped enchilada is passed to a shell? Tried
like that in this version (though now I'm wondering what the correct
perl incantation is to shell-escape $outputdir and $dlpath...)
On Sat, Dec 11, 2021 at 1:17 PM Andres Freund <and...@anarazel.de> wrote:
> On 2021-12-10 12:58:01 +1300, Thomas Munro wrote:
> > +$node_primary->adjust_conf('postgresql.conf', 'max_connections', '25', 1);
>
> Possible that this will cause problem on some *BSD platform with a limited
> count of semaphores. But we can deal with that if / when it happens.
Right, those systems don't work out of the box for us already without
sysctl tweaks, so it doesn't matter if animals have to be adjusted
further.
> > @@ -590,16 +595,35 @@ create_tablespace_directories(const char *location,
> > const Oid tablespaceoid)
> > char *linkloc;
> > char *location_with_version_dir;
> > struct stat st;
> > + bool in_place;
> >
> > linkloc = psprintf("pg_tblspc/%u", tablespaceoid);
> > - location_with_version_dir = psprintf("%s/%s", location,
> > +
> > + /*
> > + * If we're asked to make an 'in place' tablespace, create the
> > directory
> > + * directly where the symlink would normally go. This is a
> > developer-only
> > + * option for now, to facilitate regression testing.
> > + */
> > + in_place =
> > + (allow_in_place_tablespaces || InRecovery) &&
> > strlen(location) == 0;
>
> Why is in_place set to true by InRecovery?
Well the real condition is strlen(location) == 0, and the other part
is a sort of bit belt-and-braces check, but yeah, I should just remove
that part. Done.
> ISTM that allow_in_place_tablespaces should be checked in CreateTableSpace(),
> and create_tablespace_directories() should just do whatever it's told?
> Otherwise it seems there's ample potential for confusion, e.g. because of the
> GUC differing between primary and replica, or between crash and a new start.
Agreed, that was the effect but the extra unnecessary check was a bit confusing.
> > /*
> > * Attempt to coerce target directory to safe permissions. If this
> > fails,
> > * it doesn't exist or has the wrong owner.
> > */
> > - if (chmod(location, pg_dir_create_mode) != 0)
> > + if (!in_place && chmod(location, pg_dir_create_mode) != 0)
> > {
> > if (errno == ENOENT)
> > ereport(ERROR,
>
> Maybe add a coment saying that we don't need to chmod here because
> MakePGDirectory() takes care of that?
Done.
> > @@ -648,13 +672,13 @@ create_tablespace_directories(const char *location,
> > const Oid tablespaceoid)
> > /*
> > * In recovery, remove old symlink, in case it points to the wrong
> > place.
> > */
> > - if (InRecovery)
> > + if (!in_place && InRecovery)
> > remove_tablespace_symlink(linkloc);
>
> I don't think it's right to check !in_place as you do here, given that it
> currently depends on a GUC setting that's possibly differs between WAL
> generation and replay time?
I have to, because otherwise we'll remove the directory we just
created at the top of the function. It doesn't really depend on a GUC
(clearer after previous change).
> Perhaps also add a test that we catch "in-place" tablespace creation with
> allow_in_place_tablespaces = false? Although perhaps that's better done in the
> previous commit...
There was a test for that already, see this bit:
+-- empty tablespace locations are not usually allowed
+CREATE TABLESPACE regress_tblspace LOCATION ''; -- fail
+ERROR: tablespace location must be an absolute path
> > +++ b/src/test/modules/test_misc/t/002_tablespace.pl
>
> Two minor things that I think would be worth testing here:
> 1) moving between two "absolute" tablespaces. That could conceivably break
> differently
> between in-place and relative tablespaces.
> 2) Moving between absolute and relative tablespace.
Done.
> > +# required for 027_stream_regress.pl
> > +REGRESS_OUTPUTDIR=$(abs_top_builddir)/src/test/recovery
> > +export REGRESS_OUTPUTDIR
>
> Why do we need this?
The Make macro "prove_check" (src/Makefile.global.in) always changes
to the source directory to run TAP tests. Without an explicit
directive to control where regression test output goes, it got
splattered all over the source tree in VPATH builds. I didn't see an
existing way to adjust that (did I miss something?). Hence desire to
pass down a path in the build tree. Better ideas welcome.
> > +# Initialize primary node
> > +my $node_primary = PostgreSQL::Test::Cluster->new('primary');
> > +$node_primary->init(allows_streaming => 1);
> > +$node_primary->adjust_conf('postgresql.conf', 'max_connections', '25', 1);
>
> Probably should set at least max_prepared_transactions > 0, so the tests
> requiring prepared xacts can work. They have nontrivial replay routines, so
> that seems worthwhile?
Good idea. Done.
> > +# Perform a logical dump of primary and standby, and check that they match
> > +command_ok(
> > + [ 'pg_dump', '-f', $outputdir . '/primary.dump', '--no-sync',
> > + '-p', $node_primary->port, 'regression' ],
> > + 'dump primary server');
> > +command_ok(
> > + [ 'pg_dump', '-f', $outputdir . '/standby.dump', '--no-sync',
> > + '-p', $node_standby_1->port, 'regression' ],
> > + 'dump standby server');
> > +command_ok(
> > + [ 'diff', $outputdir . '/primary.dump', $outputdir . '/standby.dump'
> > ],
> > + 'compare primary and standby dumps');
> > +
> > +$node_standby_1->stop;
> > +$node_primary->stop;
>
> This doesn't verify if global objects are replayed correctly. Perhaps it'd be
> better to use pg_dumpall?
Good idea. Done.
From 6a05a305f3e4c4e5b9bb368f3e18d502e779a3b8 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Fri, 10 Dec 2021 11:45:24 +1300
Subject: [PATCH v10 1/6] Allow "in place" tablespaces.
Provide a developer-only GUC allow_in_place_tablespaces, disabled by
default. When enabled, tablespaces can be created with an empty
LOCATION string, meaning that they should be created as a directory
directly beneath pg_tblspc. This can be used for new testing scenarios,
in a follow-up patch. Not intended for end-user usage, since it might
confuse backup tools that expect symlinks.
Reviewed-by: Andres Freund <and...@anarazel.de>
Reviewed-by: Michael Paquier <mich...@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
---
src/backend/commands/tablespace.c | 39 +++++++++++++++++++++++++------
src/backend/utils/misc/guc.c | 12 ++++++++++
src/include/commands/tablespace.h | 2 ++
3 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c
index 4b96eec9df..c198501cbf 100644
--- a/src/backend/commands/tablespace.c
+++ b/src/backend/commands/tablespace.c
@@ -87,6 +87,7 @@
/* GUC variables */
char *default_tablespace = NULL;
char *temp_tablespaces = NULL;
+bool allow_in_place_tablespaces = false;
static void create_tablespace_directories(const char *location,
@@ -241,6 +242,7 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
char *location;
Oid ownerId;
Datum newOptions;
+ bool in_place;
/* Must be superuser */
if (!superuser())
@@ -266,12 +268,15 @@ CreateTableSpace(CreateTableSpaceStmt *stmt)
(errcode(ERRCODE_INVALID_NAME),
errmsg("tablespace location cannot contain single quotes")));
+ in_place = allow_in_place_tablespaces && strlen(location) == 0;
+
/*
* Allowing relative paths seems risky
*
- * this also helps us ensure that location is not empty or whitespace
+ * This also helps us ensure that location is not empty or whitespace,
+ * unless specifying a developer-only in-place tablespace.
*/
- if (!is_absolute_path(location))
+ if (!in_place && !is_absolute_path(location))
ereport(ERROR,
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
errmsg("tablespace location must be an absolute path")));
@@ -590,16 +595,36 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
char *linkloc;
char *location_with_version_dir;
struct stat st;
+ bool in_place;
linkloc = psprintf("pg_tblspc/%u", tablespaceoid);
- location_with_version_dir = psprintf("%s/%s", location,
+
+ /*
+ * If we're asked to make an 'in place' tablespace, create the directory
+ * directly where the symlink would normally go. This is a developer-only
+ * option for now, to facilitate regression testing.
+ */
+ in_place = strlen(location) == 0;
+
+ if (in_place)
+ {
+ if (MakePGDirectory(linkloc) < 0 && errno != EEXIST)
+ ereport(ERROR,
+ (errcode_for_file_access(),
+ errmsg("could not create directory \"%s\": %m",
+ linkloc)));
+ }
+
+ location_with_version_dir = psprintf("%s/%s", in_place ? linkloc : location,
TABLESPACE_VERSION_DIRECTORY);
/*
* Attempt to coerce target directory to safe permissions. If this fails,
- * it doesn't exist or has the wrong owner.
+ * it doesn't exist or has the wrong owner. Not needed for in-place mode,
+ * because in that case we created the directory with the desired
+ * permissions.
*/
- if (chmod(location, pg_dir_create_mode) != 0)
+ if (!in_place && chmod(location, pg_dir_create_mode) != 0)
{
if (errno == ENOENT)
ereport(ERROR,
@@ -648,13 +673,13 @@ create_tablespace_directories(const char *location, const Oid tablespaceoid)
/*
* In recovery, remove old symlink, in case it points to the wrong place.
*/
- if (InRecovery)
+ if (!in_place && InRecovery)
remove_tablespace_symlink(linkloc);
/*
* Create the symlink under PGDATA
*/
- if (symlink(location, linkloc) < 0)
+ if (!in_place && symlink(location, linkloc) < 0)
ereport(ERROR,
(errcode_for_file_access(),
errmsg("could not create symbolic link \"%s\": %m",
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index bff949a40b..ea655bd98e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -46,6 +46,7 @@
#include "catalog/storage.h"
#include "commands/async.h"
#include "commands/prepare.h"
+#include "commands/tablespace.h"
#include "commands/trigger.h"
#include "commands/user.h"
#include "commands/vacuum.h"
@@ -1963,6 +1964,17 @@ static struct config_bool ConfigureNamesBool[] =
NULL, NULL, NULL
},
+ {
+ {"allow_in_place_tablespaces", PGC_SUSET, DEVELOPER_OPTIONS,
+ gettext_noop("Allows tablespaces directly inside pg_tblspc, for testing."),
+ NULL,
+ GUC_NOT_IN_SAMPLE
+ },
+ &allow_in_place_tablespaces,
+ false,
+ NULL, NULL, NULL
+ },
+
{
{"lo_compat_privileges", PGC_SUSET, COMPAT_OPTIONS_PREVIOUS,
gettext_noop("Enables backward compatibility mode for privilege checks on large objects."),
diff --git a/src/include/commands/tablespace.h b/src/include/commands/tablespace.h
index 49cfc8479a..f33ba51bd6 100644
--- a/src/include/commands/tablespace.h
+++ b/src/include/commands/tablespace.h
@@ -19,6 +19,8 @@
#include "lib/stringinfo.h"
#include "nodes/parsenodes.h"
+extern bool allow_in_place_tablespaces;
+
/* XLOG stuff */
#define XLOG_TBLSPC_CREATE 0x00
#define XLOG_TBLSPC_DROP 0x10
--
2.30.2
From b84478cd1db090cd308514a8db322ea0dc54cbb4 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Wed, 4 Aug 2021 22:17:54 +1200
Subject: [PATCH v10 2/6] Use in-place tablespaces in regression test.
Remove the machinery from pg_regress that manages the testtablespace
directory. Instead, use "in-place" tablespaces, because they work
correctly when there is a streaming replica running on the same host.
Reviewed-by: Andres Freund <and...@anarazel.de>
Reviewed-by: Michael Paquier <mich...@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
---
src/test/regress/GNUmakefile | 2 +-
src/test/regress/expected/tablespace.out | 20 ++++++++++----
src/test/regress/pg_regress.c | 35 ------------------------
src/test/regress/sql/tablespace.sql | 19 +++++++++----
src/tools/msvc/vcregress.pl | 2 --
5 files changed, 28 insertions(+), 50 deletions(-)
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 330eca2b83..31b1c49100 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -112,7 +112,7 @@ submake-contrib-spi: | submake-libpgport submake-generated-headers
## Run tests
##
-REGRESS_OPTS = --dlpath=. --max-concurrent-tests=20 --make-testtablespace-dir \
+REGRESS_OPTS = --dlpath=. --max-concurrent-tests=20 \
$(EXTRA_REGRESS_OPTS)
check: all
diff --git a/src/test/regress/expected/tablespace.out b/src/test/regress/expected/tablespace.out
index 864f4b6e20..2dfbcfdebe 100644
--- a/src/test/regress/expected/tablespace.out
+++ b/src/test/regress/expected/tablespace.out
@@ -1,10 +1,18 @@
--- directory paths are passed to us in environment variables
-\getenv abs_builddir PG_ABS_BUILDDIR
-\set testtablespace :abs_builddir '/testtablespace'
+-- relative tablespace locations are not allowed
+CREATE TABLESPACE regress_tblspace LOCATION 'relative'; -- fail
+ERROR: tablespace location must be an absolute path
+-- empty tablespace locations are not usually allowed
+CREATE TABLESPACE regress_tblspace LOCATION ''; -- fail
+ERROR: tablespace location must be an absolute path
+-- as a special developer-only option to allow us to use tablespaces
+-- with streaming replication on the same server, an empty location
+-- can be allowed as a way to say that the tablespace should be created
+-- as a directory in pg_tblspc, rather than being a symlink
+SET allow_in_place_tablespaces = true;
-- create a tablespace using WITH clause
-CREATE TABLESPACE regress_tblspacewith LOCATION :'testtablespace' WITH (some_nonexistent_parameter = true); -- fail
+CREATE TABLESPACE regress_tblspacewith LOCATION '' WITH (some_nonexistent_parameter = true); -- fail
ERROR: unrecognized parameter "some_nonexistent_parameter"
-CREATE TABLESPACE regress_tblspacewith LOCATION :'testtablespace' WITH (random_page_cost = 3.0); -- ok
+CREATE TABLESPACE regress_tblspacewith LOCATION '' WITH (random_page_cost = 3.0); -- ok
-- check to see the parameter was used
SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
spcoptions
@@ -15,7 +23,7 @@ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
-- drop the tablespace so we can re-use the location
DROP TABLESPACE regress_tblspacewith;
-- create a tablespace we can use
-CREATE TABLESPACE regress_tblspace LOCATION :'testtablespace';
+CREATE TABLESPACE regress_tblspace LOCATION '';
-- try setting and resetting some properties for the new tablespace
ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1);
ALTER TABLESPACE regress_tblspace SET (some_nonexistent_parameter = true); -- fail
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 589357ba59..d64b02d4ad 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -438,32 +438,6 @@ string_matches_pattern(const char *str, const char *pattern)
return false;
}
-/*
- * Clean out the test tablespace dir, or create it if it doesn't exist.
- *
- * On Windows, doing this cleanup here makes it possible to run the
- * regression tests under a Windows administrative user account with the
- * restricted token obtained when starting pg_regress.
- */
-static void
-prepare_testtablespace_dir(void)
-{
- char testtablespace[MAXPGPATH];
-
- snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-
- if (directory_exists(testtablespace))
- {
- if (!rmtree(testtablespace, true))
- {
- fprintf(stderr, _("\n%s: could not remove test tablespace \"%s\"\n"),
- progname, testtablespace);
- exit(2);
- }
- }
- make_directory(testtablespace);
-}
-
/*
* Scan resultmap file to find which platform-specific expected files to use.
*
@@ -2010,7 +1984,6 @@ help(void)
printf(_(" --launcher=CMD use CMD as launcher of psql\n"));
printf(_(" --load-extension=EXT load the named extension before running the\n"));
printf(_(" tests; can appear multiple times\n"));
- printf(_(" --make-testtablespace-dir create testtablespace directory\n"));
printf(_(" --max-connections=N maximum number of concurrent connections\n"));
printf(_(" (default is 0, meaning unlimited)\n"));
printf(_(" --max-concurrent-tests=N maximum number of concurrent tests in schedule\n"));
@@ -2069,12 +2042,10 @@ regression_main(int argc, char *argv[],
{"load-extension", required_argument, NULL, 22},
{"config-auth", required_argument, NULL, 24},
{"max-concurrent-tests", required_argument, NULL, 25},
- {"make-testtablespace-dir", no_argument, NULL, 26},
{NULL, 0, NULL, 0}
};
bool use_unix_sockets;
- bool make_testtablespace_dir = false;
_stringlist *sl;
int c;
int i;
@@ -2200,9 +2171,6 @@ regression_main(int argc, char *argv[],
case 25:
max_concurrent_tests = atoi(optarg);
break;
- case 26:
- make_testtablespace_dir = true;
- break;
default:
/* getopt_long already emitted a complaint */
fprintf(stderr, _("\nTry \"%s -h\" for more information.\n"),
@@ -2255,9 +2223,6 @@ regression_main(int argc, char *argv[],
unlimit_core_size();
#endif
- if (make_testtablespace_dir)
- prepare_testtablespace_dir();
-
if (temp_instance)
{
FILE *pg_conf;
diff --git a/src/test/regress/sql/tablespace.sql b/src/test/regress/sql/tablespace.sql
index 92076db9a1..896f05cea3 100644
--- a/src/test/regress/sql/tablespace.sql
+++ b/src/test/regress/sql/tablespace.sql
@@ -1,11 +1,18 @@
--- directory paths are passed to us in environment variables
-\getenv abs_builddir PG_ABS_BUILDDIR
+-- relative tablespace locations are not allowed
+CREATE TABLESPACE regress_tblspace LOCATION 'relative'; -- fail
-\set testtablespace :abs_builddir '/testtablespace'
+-- empty tablespace locations are not usually allowed
+CREATE TABLESPACE regress_tblspace LOCATION ''; -- fail
+
+-- as a special developer-only option to allow us to use tablespaces
+-- with streaming replication on the same server, an empty location
+-- can be allowed as a way to say that the tablespace should be created
+-- as a directory in pg_tblspc, rather than being a symlink
+SET allow_in_place_tablespaces = true;
-- create a tablespace using WITH clause
-CREATE TABLESPACE regress_tblspacewith LOCATION :'testtablespace' WITH (some_nonexistent_parameter = true); -- fail
-CREATE TABLESPACE regress_tblspacewith LOCATION :'testtablespace' WITH (random_page_cost = 3.0); -- ok
+CREATE TABLESPACE regress_tblspacewith LOCATION '' WITH (some_nonexistent_parameter = true); -- fail
+CREATE TABLESPACE regress_tblspacewith LOCATION '' WITH (random_page_cost = 3.0); -- ok
-- check to see the parameter was used
SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
@@ -14,7 +21,7 @@ SELECT spcoptions FROM pg_tablespace WHERE spcname = 'regress_tblspacewith';
DROP TABLESPACE regress_tblspacewith;
-- create a tablespace we can use
-CREATE TABLESPACE regress_tblspace LOCATION :'testtablespace';
+CREATE TABLESPACE regress_tblspace LOCATION '';
-- try setting and resetting some properties for the new tablespace
ALTER TABLESPACE regress_tblspace SET (random_page_cost = 1.0, seq_page_cost = 1.1);
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 29086cab51..fc7679e1d1 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -133,7 +133,6 @@ sub installcheck_internal
"--bindir=../../../$Config/psql",
"--schedule=${schedule}_schedule",
"--max-concurrent-tests=20",
- "--make-testtablespace-dir",
"--encoding=SQL_ASCII",
"--no-locale");
push(@args, $maxconn) if $maxconn;
@@ -168,7 +167,6 @@ sub check
"--bindir=",
"--schedule=${schedule}_schedule",
"--max-concurrent-tests=20",
- "--make-testtablespace-dir",
"--encoding=SQL_ASCII",
"--no-locale",
"--temp-instance=./tmp_check");
--
2.30.2
From 64e768eb3b438684ef5a4e836b1ef73712abd14e Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Fri, 10 Dec 2021 12:22:55 +1300
Subject: [PATCH v10 3/6] Add new simple TAP test for tablespaces.
The tablespace tests in the main regression tests have been changed to
use "in-place" tablespaces, so that they work when streamed to a replica
on the same host. Add a new TAP test that exercises tablespaces with
absolute paths (but couldn't be replicated), for coverage.
Reviewed-by: Andres Freund <and...@anarazel.de>
Reviewed-by: Michael Paquier <mich...@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
---
.../modules/test_misc/t/002_tablespace.pl | 96 +++++++++++++++++++
1 file changed, 96 insertions(+)
create mode 100644 src/test/modules/test_misc/t/002_tablespace.pl
diff --git a/src/test/modules/test_misc/t/002_tablespace.pl b/src/test/modules/test_misc/t/002_tablespace.pl
new file mode 100644
index 0000000000..5f1be368a7
--- /dev/null
+++ b/src/test/modules/test_misc/t/002_tablespace.pl
@@ -0,0 +1,96 @@
+# Simple tablespace tests that can't be replicated on the same host
+# due to the use of absolute paths, so we keep them out of the regular
+# regression tests.
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More tests => 20;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->start;
+
+# Create a couple of directories to use as tablespaces.
+my $TS1_LOCATION = $node->basedir() . "/ts1";
+mkdir($TS1_LOCATION);
+my $TS2_LOCATION = $node->basedir() . "/ts2";
+mkdir($TS2_LOCATION);
+
+my $result;
+
+# Create a tablespace with an absolute path
+$result = $node->psql('postgres',
+ "CREATE TABLESPACE regress_ts1 LOCATION '$TS1_LOCATION'");
+ok($result == 0, 'create tablespace with absolute path');
+
+# Can't create a tablespace where there is one already
+$result = $node->psql('postgres',
+ "CREATE TABLESPACE regress_ts1 LOCATION '$TS1_LOCATION'");
+ok($result != 0, 'clobber tablespace with absolute path');
+
+# Create table in it
+$result = $node->psql('postgres',
+ "CREATE TABLE t () TABLESPACE regress_ts1");
+ok($result == 0, 'create table in tablespace with absolute path');
+
+# Can't drop a tablespace that still has a table in it
+$result = $node->psql('postgres',
+ "DROP TABLESPACE regress_ts1");
+ok($result != 0, 'drop tablespace with absolute path');
+
+# Drop the table
+$result = $node->psql('postgres', "DROP TABLE t");
+ok($result == 0, 'drop table in tablespace with absolute path');
+
+# Drop the tablespace
+$result = $node->psql('postgres', "DROP TABLESPACE regress_ts1");
+ok($result == 0, 'drop tablespace with absolute path');
+
+# Create two absolute tablespaces and two in-place tablespaces, so we can
+# testing various kinds of tablespace moves.
+$result = $node->psql('postgres',
+ "CREATE TABLESPACE regress_ts1 LOCATION '$TS1_LOCATION'");
+ok($result == 0, 'create tablespace 1 with absolute path');
+$result = $node->psql('postgres',
+ "CREATE TABLESPACE regress_ts2 LOCATION '$TS2_LOCATION'");
+ok($result == 0, 'create tablespace 2 with absolute path');
+$result = $node->psql('postgres',
+ "SET allow_in_place_tablespaces=on; CREATE TABLESPACE regress_ts3 LOCATION ''");
+ok($result == 0, 'create tablespace 3 with in-place directory');
+$result = $node->psql('postgres',
+ "SET allow_in_place_tablespaces=on; CREATE TABLESPACE regress_ts4 LOCATION ''");
+ok($result == 0, 'create tablespace 4 with in-place directory');
+
+# Create a table and test moving between absolute and in-place tablespaces
+$result = $node->psql('postgres',
+ "CREATE TABLE t () TABLESPACE regress_ts1");
+ok($result == 0, 'create table in tablespace 1');
+$result = $node->psql('postgres',
+ "ALTER TABLE t SET tablespace regress_ts2");
+ok($result == 0, 'move table abs->abs');
+$result = $node->psql('postgres',
+ "ALTER TABLE t SET tablespace regress_ts3");
+ok($result == 0, 'move table abs->in-place');
+$result = $node->psql('postgres',
+ "ALTER TABLE t SET tablespace regress_ts4");
+ok($result == 0, 'move table in-place->in-place');
+$result = $node->psql('postgres',
+ "ALTER TABLE t SET tablespace regress_ts1");
+ok($result == 0, 'move table in-place->abs');
+
+# Drop everything
+$result = $node->psql('postgres',
+ "DROP TABLE t");
+ok($result == 0, 'create table in tablespace 1');
+$result = $node->psql('postgres', "DROP TABLESPACE regress_ts1");
+ok($result == 0, 'drop tablespace 1');
+$result = $node->psql('postgres', "DROP TABLESPACE regress_ts2");
+ok($result == 0, 'drop tablespace 2');
+$result = $node->psql('postgres', "DROP TABLESPACE regress_ts3");
+ok($result == 0, 'drop tablespace 3');
+$result = $node->psql('postgres', "DROP TABLESPACE regress_ts4");
+ok($result == 0, 'drop tablespace 4');
+
+$node->stop;
--
2.30.2
From c810b1dece30790f07fe5c0b996e34c78c2b43d1 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Tue, 5 Oct 2021 21:01:02 +1300
Subject: [PATCH v10 4/6] Test replay of regression tests.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Add a new TAP test under src/test/recovery to run the standard
regression tests while a streaming replica replays the WAL. This
provides a basic workout for WAL decoding and redo code, and compares
the replicated result.
Optionally, enable (expensive) wal_consistency_checking if listed in
the env variable PG_TEST_EXTRA.
Reviewed-by: 綱川 貴之 (Takayuki Tsunakawa) <tsunakawa.ta...@fujitsu.com>
Reviewed-by: Andres Freund <and...@anarazel.de>
Reviewed-by: Andrew Dunstan <and...@dunslane.net>
Reviewed-by: Tom Lane <t...@sss.pgh.pa.us>
Reviewed-by: Anastasia Lubennikova <lubennikov...@gmail.com>
Reviewed-by: Michael Paquier <mich...@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGKpRWQ9SxdxxDmTBCJoR0YnFpMBe7kyzY8SUQk%2BHeskxg%40mail.gmail.com
---
doc/src/sgml/regress.sgml | 11 ++++
src/test/recovery/Makefile | 6 +-
src/test/recovery/t/027_stream_regress.pl | 80 +++++++++++++++++++++++
src/tools/msvc/vcregress.pl | 2 +
4 files changed, 98 insertions(+), 1 deletion(-)
create mode 100644 src/test/recovery/t/027_stream_regress.pl
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 724ef566e7..64fc81776d 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -289,6 +289,17 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl'
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><literal>wal_consistency_checking</literal></term>
+ <listitem>
+ <para>
+ Uses <literal>wal_consistency_checking=all</literal> while running
+ some of the tests under <filename>src/test/recovery</filename>. Not
+ enabled by default because it is resource intensive.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
Tests for features that are not supported by the current build
diff --git a/src/test/recovery/Makefile b/src/test/recovery/Makefile
index 288c04b861..c557018b38 100644
--- a/src/test/recovery/Makefile
+++ b/src/test/recovery/Makefile
@@ -15,10 +15,14 @@ subdir = src/test/recovery
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
-# required for 017_shm.pl
+# required for 017_shm.pl and 027_stream_regress.pl
REGRESS_SHLIB=$(abs_top_builddir)/src/test/regress/regress$(DLSUFFIX)
export REGRESS_SHLIB
+# required for 027_stream_regress.pl
+REGRESS_OUTPUTDIR=$(abs_top_builddir)/src/test/recovery
+export REGRESS_OUTPUTDIR
+
check:
$(prove_check)
diff --git a/src/test/recovery/t/027_stream_regress.pl b/src/test/recovery/t/027_stream_regress.pl
new file mode 100644
index 0000000000..6619298694
--- /dev/null
+++ b/src/test/recovery/t/027_stream_regress.pl
@@ -0,0 +1,80 @@
+# Run the standard regression tests with streaming replication
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More tests => 4;
+use File::Basename;
+
+# Initialize primary node
+my $node_primary = PostgreSQL::Test::Cluster->new('primary');
+$node_primary->init(allows_streaming => 1);
+$node_primary->adjust_conf('postgresql.conf', 'max_connections', '25', 1);
+$node_primary->append_conf('postgresql.conf', 'max_prepared_transactions = 10');
+
+# WAL consistency checking is resource intensive so require opt-in with the
+# PG_TEST_EXTRA environment variable.
+if ($ENV{PG_TEST_EXTRA} &&
+ $ENV{PG_TEST_EXTRA} =~ m/\bwal_consistency_checking\b/) {
+ $node_primary->append_conf('postgresql.conf',
+ 'wal_consistency_checking = all');
+}
+
+$node_primary->start;
+is( $node_primary->psql(
+ 'postgres',
+ qq[SELECT pg_create_physical_replication_slot('standby_1');]),
+ 0,
+ 'physical slot created on primary');
+my $backup_name = 'my_backup';
+
+# Take backup
+$node_primary->backup($backup_name);
+
+# Create streaming standby linking to primary
+my $node_standby_1 = PostgreSQL::Test::Cluster->new('standby_1');
+$node_standby_1->init_from_backup($node_primary, $backup_name,
+ has_streaming => 1);
+$node_standby_1->append_conf('postgresql.conf',
+ "primary_slot_name = standby_1");
+$node_standby_1->start;
+
+my $dlpath = PostgreSQL::Test::Utils::perl2host(dirname($ENV{REGRESS_SHLIB}));
+my $outputdir = PostgreSQL::Test::Utils::perl2host($ENV{REGRESS_OUTPUTDIR});
+
+# Run the regression tests against the primary.
+my $extra_opts = $ENV{EXTRA_REGRESS_OPTS} || "";
+system_or_bail($ENV{PG_REGRESS} . " " .
+ "--dlpath=$dlpath " .
+ "--bindir= " .
+ "--port=" . $node_primary->port . " " .
+ "--schedule=../regress/parallel_schedule " .
+ "--max-concurrent-tests=20 " .
+ "--inputdir=../regress " .
+ "--outputdir=$outputdir " .
+ $extra_opts);
+
+# Clobber all sequences with their next value, so that we don't have
+# differences between nodes due to caching.
+$node_primary->psql('regression',
+ "select setval(seqrelid, nextval(seqrelid)) from pg_sequence");
+
+# Wait for standby to catch up
+$node_primary->wait_for_catchup($node_standby_1, 'replay',
+ $node_primary->lsn('insert'));
+
+# Perform a logical dump of primary and standby, and check that they match
+command_ok(
+ [ 'pg_dumpall', '-f', $outputdir . '/primary.dump', '--no-sync',
+ '-p', $node_primary->port ],
+ 'dump primary server');
+command_ok(
+ [ 'pg_dumpall', '-f', $outputdir . '/standby.dump', '--no-sync',
+ '-p', $node_standby_1->port ],
+ 'dump standby server');
+command_ok(
+ [ 'diff', $outputdir . '/primary.dump', $outputdir . '/standby.dump' ],
+ 'compare primary and standby dumps');
+
+$node_standby_1->stop;
+$node_primary->stop;
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index fc7679e1d1..73d411dcf5 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -535,6 +535,8 @@ sub recoverycheck
{
InstallTemp();
+ $ENV{REGRESS_OUTPUTDIR} = "$topdir/src/test/recovery";
+
my $mstat = 0;
my $dir = "$topdir/src/test/recovery";
my $status = tap_check($dir);
--
2.30.2
