agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH] Move tablespace cleanup out of pg_regress.
29+ messages / 3 participants
[nested] [flat]
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make. That is not only ugly also leads to do the clean up
twice at once. Let's move the cleanup code out of pg_regress into
vcregress.pl, where is more sutable place.
This also fixes a bug that the test for pg_upgrade mistakenly cleans
up the tablespace directory of default tmp-install location, instead
of its own installation directoty.
---
src/test/regress/GNUmakefile | 6 ++++--
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 25 +++++++++++++++++++++++++
3 files changed, 29 insertions(+), 24 deletions(-)
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index 1a3164065f..815d87904b 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -118,8 +118,10 @@ submake-contrib-spi: | submake-libpgport submake-generated-headers
.PHONY: tablespace-setup
tablespace-setup:
- rm -rf ./testtablespace
- mkdir ./testtablespace
+# extract --outputdir optsion from EXTRA_REGRESS_OPTS
+ $(eval dir := $(shell perl -e 'use Getopt::Long qw(GetOptionsFromString Configure); Configure("pass_through", "gnu_getopt"); ($$r, $$x) = GetOptionsFromString($$ENV{"EXTRA_REGRESS_OPTS"}, "outputdir=s" => \$$dir); print ((defined $$dir ? $$dir : ".") . "/testtablespace");'))
+ rm -rf $(dir)
+ mkdir $(dir)
##
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index 0a98f6e37d..4bada14092 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -114,6 +114,13 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+
+ my $outputdir;
+ foreach (@EXTRA_REGRESS_OPTS)
+ {
+ $outputdir = $1 if (/^ *--outputdir *= *(.+) *$/);
+ }
+ CleanupTablespaceDirectory($outputdir);
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +150,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -169,6 +177,7 @@ sub ecpgcheck
"--no-locale",
"--temp-instance=./tmp_chk");
push(@args, $maxconn) if $maxconn;
+ CleanupTablespaceDirectory();
system(@args);
$status = $? >> 8;
exit $status if $status;
@@ -186,6 +195,7 @@ sub isolationcheck
"--inputdir=.",
"--schedule=./isolation_schedule");
push(@args, $maxconn) if $maxconn;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -382,6 +392,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -444,6 +455,7 @@ sub subdircheck
"--bindir=${topdir}/${Config}/psql",
"--dbname=contrib_regression", @opts, @tests);
print join(' ', @args), "\n";
+ CleanupTablespaceDirectory();
system(@args);
chdir "..";
return;
@@ -739,6 +751,19 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my ($testdir) = @_;
+
+ $testdir = cwd() if (! defined $testdir);
+
+ # don't bother checking trailing directory separator in $testdir
+ my $tablespace = "$testdir/testtablespace";
+
+ rmtree($tablespace) if (-d $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Fri_May_15_17_25_08_2020_691)----
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* [PATCH 1/2] Move tablespace cleanup out of pg_regress.
@ 2020-04-30 05:06 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Kyotaro Horiguchi @ 2020-04-30 05:06 UTC (permalink / raw)
Tablespace directory is cleaned-up in regress/GNUmakefile for all
platforms other than Windows. For Windoiws pg_regress does that on
behalf of make, which is ugly. In addition to that, pg_regress does
the clean up twice at once. This patch moves the cleanup code out of
pg_regress into vcregress.pl, which is more sutable place.
---
src/test/regress/pg_regress.c | 22 ----------------------
src/tools/msvc/vcregress.pl | 14 ++++++++++++++
2 files changed, 14 insertions(+), 22 deletions(-)
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 38b2b1e8e1..c56bfaf7f5 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -491,28 +491,6 @@ convert_sourcefiles_in(const char *source_subdir, const char *dest_dir, const ch
snprintf(testtablespace, MAXPGPATH, "%s/testtablespace", outputdir);
-#ifdef WIN32
-
- /*
- * On Windows only, clean out the test tablespace dir, or create it if it
- * doesn't exist. On other platforms we expect the Makefile to take care
- * of that. (We don't migrate that functionality in here because it'd be
- * harder to cope with platform-specific issues such as SELinux.)
- *
- * XXX it would be better if pg_regress.c had nothing at all to do with
- * testtablespace, and this were handled by a .BAT file or similar on
- * Windows. See pgsql-hackers discussion of 2008-01-18.
- */
- 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);
-#endif
-
/* finally loop on each file and do the replacement */
for (name = names; *name; name++)
{
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index f95f7a5c7a..74d37a31de 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -104,6 +104,7 @@ exit 0;
sub installcheck_internal
{
my ($schedule, @EXTRA_REGRESS_OPTS) = @_;
+
my @args = (
"../../../$Config/pg_regress/pg_regress",
"--dlpath=.",
@@ -114,6 +115,7 @@ sub installcheck_internal
"--no-locale");
push(@args, $maxconn) if $maxconn;
push(@args, @EXTRA_REGRESS_OPTS);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -143,6 +145,7 @@ sub check
"--temp-instance=./tmp_check");
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -178,6 +181,7 @@ sub ecpgcheck
sub isolationcheck
{
chdir "../isolation";
+ CleanupTablespaceDirectory();
copy("../../../$Config/isolationtester/isolationtester.exe",
"../../../$Config/pg_isolation_regress");
my @args = (
@@ -382,6 +386,7 @@ sub plcheck
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=$topdir/$Config/psql",
"--dbname=pl_regression", @lang_args, @tests);
+ CleanupTablespaceDirectory();
system(@args);
my $status = $? >> 8;
exit $status if $status;
@@ -439,6 +444,7 @@ sub subdircheck
print "============================================================\n";
print "Checking $module\n";
+ CleanupTablespaceDirectory();
my @args = (
"$topdir/$Config/pg_regress/pg_regress",
"--bindir=${topdir}/${Config}/psql",
@@ -741,6 +747,14 @@ sub InstallTemp
return;
}
+sub CleanupTablespaceDirectory
+{
+ my $tablespace = 'testtablespace';
+
+ rmtree($tablespace) if (-e $tablespace);
+ mkdir($tablespace);
+}
+
sub usage
{
print STDERR
--
2.18.2
----Next_Part(Mon_May_11_17_13_54_2020_626)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="0002-Don-t-setup-tablespace-directory-while-testing-pg_up.patch"
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: New strategies for freezing, advancing relfrozenxid early
@ 2022-10-04 00:41 Jeff Davis <[email protected]>
2022-10-04 03:11 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Jeff Davis @ 2022-10-04 00:41 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Thu, 2022-09-08 at 13:23 -0700, Peter Geoghegan wrote:
> The new patch unifies the concept of antiwraparound
> VACUUM with the concept of aggressive VACUUM. Now there is only
> antiwraparound and regular VACUUM (uh, barring VACUUM FULL). And now
> antiwraparound VACUUMs are not limited to antiwraparound autovacuums
> -- a manual VACUUM can also be antiwraparound (that's just the new
> name for "aggressive").
I like this general approach. The existing GUCs have evolved in a
confusing way.
> For the most part the
> skipping/freezing strategy stuff has a good sense of what matters
> already, and shouldn't need to be guided very often.
I'd like to know more clearly where manual VACUUM fits in here. Will it
user a more aggressive strategy than an autovacuum, and how so?
> The patch relegates vacuum_freeze_table_age to a compatibility
> option,
> making its default -1, meaning "just use autovacuum_freeze_max_age".
The purpose of vacuum_freeze_table_age seems to be that, if you
regularly issue VACUUM commands, it will prevent a surprise
antiwraparound vacuum. Is that still the case?
Maybe it would make more sense to have vacuum_freeze_table_age be a
fraction of autovacuum_freeze_max_age, and be treated as a maximum so
that other intelligence might kick in and freeze sooner?
> This makes things less confusing for users and hackers.
It may take an adjustment period ;-)
> The details of the skipping-strategy-choice algorithm are still
> unsettled in v3 (no real change there). ISTM that the important thing
> is still the high level concepts. Jeff was slightly puzzled by the
> emphasis placed on the cost model/strategy stuff, at least at one
> point. Hopefully my intent will be made clearer by the ideas featured
> in the new patch.
Yes, it's clearing things up, but it's still a complex problem.
There's:
a. xid age vs the actual amount of deferred work to be done
b. advancing relfrozenxid vs skipping all-visible pages
c. difficulty in controlling reasonable behavior (e.g.
vacuum_freeze_min_age often being ignored, freezing
individual tuples rather than pages)
Your first email described the motivation in terms of (a), but the
patches seem more focused on (b) and (c).
> The skipping strategy decision making process isn't
> particularly complicated, but it now looks more like an optimization
> problem of some kind or other.
There's another important point here, which is that it gives an
opportunity to decide to freeze some all-visible pages in a given round
just to reduce the deferred work, without worrying about advancing
relfrozenxid.
--
Jeff Davis
PostgreSQL Contributor Team - AWS
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: New strategies for freezing, advancing relfrozenxid early
2022-10-04 00:41 Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
@ 2022-10-04 03:11 ` Peter Geoghegan <[email protected]>
2022-10-04 05:13 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Peter Geoghegan @ 2022-10-04 03:11 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Mon, Oct 3, 2022 at 5:41 PM Jeff Davis <[email protected]> wrote:
> I like this general approach. The existing GUCs have evolved in a
> confusing way.
Thanks for taking a look!
> > For the most part the
> > skipping/freezing strategy stuff has a good sense of what matters
> > already, and shouldn't need to be guided very often.
>
> I'd like to know more clearly where manual VACUUM fits in here. Will it
> user a more aggressive strategy than an autovacuum, and how so?
There is no change whatsoever in the relationship between manually
issued VACUUMs and autovacuums. We interpret autovacuum_freeze_max_age
in almost the same way as HEAD. The only detail that's changed is that
we almost always interpret "freeze_table_age" as "just use
autovacuum_freeze_max_age" in the patch, rather than as
"vacuum_freeze_table_age, though never more than 95% of
autovacuum_freeze_max_age", as on HEAD.
Maybe this would be less confusing if I went just a bit further, and
totally got rid of the concept that vacuumlazy.c calls aggressive
VACUUM on HEAD -- then there really would be exactly one kind of
VACUUM, just like before the visibility map was first introduced back
in 2009. This would relegate antiwraparound-ness to just another
condition that autovacuum.c used to launch VACUUMs.
Giving VACUUM the freedom to choose where and how to freeze and
advance relfrozenxid based on both costs and benefits is key here.
Anything that needlessly imposes a rigid rule on vacuumlazy.c
undermines that -- it ties VACUUM's hands. The user can still
influence many of the details using high-level GUCs that work at the
table level, rather than GUCs that can only work at the level of
individual VACUUM operations (that leaves too much to chance). Users
shouldn't want or need to micromanage VACUUM.
> > The patch relegates vacuum_freeze_table_age to a compatibility
> > option,
> > making its default -1, meaning "just use autovacuum_freeze_max_age".
>
> The purpose of vacuum_freeze_table_age seems to be that, if you
> regularly issue VACUUM commands, it will prevent a surprise
> antiwraparound vacuum. Is that still the case?
The user really shouldn't need to do anything with
vacuum_freeze_table_age at all now. It's mostly just a way for the
user to optionally insist on advancing relfrozenxid via a
antiwraparound/aggressive VACUUM -- like in a manual VACUUM FREEZE.
Even VACUUM FREEZE shouldn't be necessary very often.
> Maybe it would make more sense to have vacuum_freeze_table_age be a
> fraction of autovacuum_freeze_max_age, and be treated as a maximum so
> that other intelligence might kick in and freeze sooner?
That's kind of how the newly improved skipping strategy stuff works.
It gives some weight to table age as one additional factor (based on
how close the table's age is to autovacuum_freeze_max_age or its Multi
equivalent).
If table age is (say) 60% of autovacuum_freeze_max_age, then VACUUM
should be "60% as aggressive" as a conventional
aggressive/antiwraparound autovacuum would be. What that actually
means is that the VACUUM will tend to prefer advancing relfrozenxid
the closer we get to the cutoff, gradually giving less and less
consideration to putting off work as we get closer and closer. When we
get to 100% then we'll definitely advance relfrozenxid (via a
conventional aggressive/antiwraparound VACUUM).
The precise details are unsettled, but I'm pretty sure that the
general idea is sound. Basically we're replacing
vacuum_freeze_table_age with a dynamic, flexible version of the same
basic idea. Now we don't just care about the need to advance
relfrozenxid (benefits), though; we also care about costs.
> > This makes things less confusing for users and hackers.
>
> It may take an adjustment period ;-)
Perhaps this is more of an aspiration at this point. :-)
> Yes, it's clearing things up, but it's still a complex problem.
> There's:
>
> a. xid age vs the actual amount of deferred work to be done
> b. advancing relfrozenxid vs skipping all-visible pages
> c. difficulty in controlling reasonable behavior (e.g.
> vacuum_freeze_min_age often being ignored, freezing
> individual tuples rather than pages)
>
> Your first email described the motivation in terms of (a), but the
> patches seem more focused on (b) and (c).
I think that all 3 areas are deeply and hopelessly intertwined.
For example, vacuum_freeze_min_age is effectively ignored in many
important cases right now precisely because we senselessly skip
all-visible pages with unfrozen tuples, no matter what -- the problem
actually comes from the visibility map, which vacuum_freeze_min_age
predates by quite a few years. So how can you possibly address the
vacuum_freeze_min_age issues without also significantly revising VM
skipping behavior? They're practically the same problem!
And once you've fixed vacuum_freeze_min_age (and skipping), how can
you then pass up the opportunity to advance relfrozenxid early when
doing so will require only a little extra work? I'm going to regress
some cases if I simply ignore the relfrozenxid factor. Finally, the
debt issue is itself a consequence of the other problems.
Perhaps this is an example of the inventor's paradox, where the more
ambitious plan may actually be easier and more likely to succeed than
a more limited plan that just focuses on one immediate problem. All of
these problems seem to be a result of adding accretion after accretion
over the years. A high-level rethink is well overdue. We need to
return to basics.
> > The skipping strategy decision making process isn't
> > particularly complicated, but it now looks more like an optimization
> > problem of some kind or other.
>
> There's another important point here, which is that it gives an
> opportunity to decide to freeze some all-visible pages in a given round
> just to reduce the deferred work, without worrying about advancing
> relfrozenxid.
True. Though I think that a strong bias in the direction of advancing
relfrozenxid by some amount (not necessarily by very many XIDs) still
makes sense, especially when we're already freezing aggressively.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: New strategies for freezing, advancing relfrozenxid early
2022-10-04 00:41 Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 03:11 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
@ 2022-10-04 05:13 ` Jeff Davis <[email protected]>
2022-10-04 05:45 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Jeff Davis @ 2022-10-04 05:13 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Mon, 2022-10-03 at 20:11 -0700, Peter Geoghegan wrote:
> True. Though I think that a strong bias in the direction of advancing
> relfrozenxid by some amount (not necessarily by very many XIDs) still
> makes sense, especially when we're already freezing aggressively.
Take the case where you load a lot of data in one transaction. After
the loading transaction finishes, those new pages will soon be marked
all-visible.
In the future, vacuum runs will have to decide what to do. If a vacuum
decides to do an aggressive scan to freeze all of those pages, it may
be at some unfortunate time and disrupt the workload. But if it skips
them all, then it's just deferring the work until it runs up against
autovacuum_freeze_max_age, which might also be at an unfortunate time.
So how does your patch series handle this case? I assume there's some
mechanism to freeze a moderate number of pages without worrying about
advancing relfrozenxid?
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: New strategies for freezing, advancing relfrozenxid early
2022-10-04 00:41 Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 03:11 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 05:13 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
@ 2022-10-04 05:45 ` Peter Geoghegan <[email protected]>
2022-10-04 17:39 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Peter Geoghegan @ 2022-10-04 05:45 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Mon, Oct 3, 2022 at 10:13 PM Jeff Davis <[email protected]> wrote:
> Take the case where you load a lot of data in one transaction. After
> the loading transaction finishes, those new pages will soon be marked
> all-visible.
>
> In the future, vacuum runs will have to decide what to do. If a vacuum
> decides to do an aggressive scan to freeze all of those pages, it may
> be at some unfortunate time and disrupt the workload. But if it skips
> them all, then it's just deferring the work until it runs up against
> autovacuum_freeze_max_age, which might also be at an unfortunate time.
Predicting the future accurately is intrinsically hard. We're already
doing that today by freezing lazily. I think that we can come up with
a better overall strategy, but there is always a risk that we'll come
out worse off in some individual cases. I think it's worth it if it
avoids ever really flying off the rails.
> So how does your patch series handle this case? I assume there's some
> mechanism to freeze a moderate number of pages without worrying about
> advancing relfrozenxid?
It mostly depends on whether or not the table exceeds the new
vacuum_freeze_strategy_threshold GUC in size at the time of the
VACUUM. This is 4GB by default, at least right now.
The case where the table size doesn't exceed that threshold yet will
see each VACUUM advance relfrozenxid when it happens to be very cheap
to do so, in terms of the amount of extra scanned_pages. If the number
of extra scanned_pages is less than 5% of the total table size
(current rel_pages), then we'll advance relfrozenxid early by making
sure to scan any all-visible pages.
Actually, this scanned_pages threshold starts at 5%. It is usually 5%,
but it will eventually start to grow (i.e. make VACUUM freeze eagerly
more often) once table age exceeds 50% of autovacuum_freeze_max_age at
the start of the VACUUM. So the skipping strategy threshold is more or
less a blend of physical units (heap pages) and logical units (XID
age).
Then there is the case where it's already a larger table at the point
a given VACUUM begins -- a table that ends up exceeding the same table
size threshold, vacuum_freeze_strategy_threshold. When that happens
we'll freeze all pages that are going to be marked all-visible as a
matter of policy (i.e. use eager freezing strategy), so that the same
pages can be marked all-frozen instead. We won't freeze pages that
aren't full of all-visible tuples (except for LP_DEAD items), unless
they have XIDs that are so old that vacuum_freeze_min_age triggers
freezing.
Once a table becomes larger than vacuum_freeze_strategy_threshold,
VACUUM stops marking pages all-visible in the first place,
consistently marking them all-frozen instead. So naturally there just
cannot be any all-visible pages after the first eager freezing VACUUM
(actually there are some obscure edge cases that can result in the odd
all-visible page here or there, but this should be extremely rare, and
have only negligible impact).
Bigger tables always have pages frozen eagerly, and in practice always
advance relfrozenxid early. In other words, eager freezing strategy
implies eager freezing strategy -- though not the other way around.
Again, these details that may change in the future. My focus is
validating the high level concepts.
So we avoid big spikes, and try to do the work when it's cheapest.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: New strategies for freezing, advancing relfrozenxid early
2022-10-04 00:41 Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 03:11 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 05:13 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 05:45 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
@ 2022-10-04 17:39 ` Jeff Davis <[email protected]>
2022-10-04 18:09 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Jeff Davis @ 2022-10-04 17:39 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Mon, 2022-10-03 at 22:45 -0700, Peter Geoghegan wrote:
> Once a table becomes larger than vacuum_freeze_strategy_threshold,
> VACUUM stops marking pages all-visible in the first place,
> consistently marking them all-frozen instead.
What are the trade-offs here? Why does it depend on table size?
Regards,
Jeff Davis
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: New strategies for freezing, advancing relfrozenxid early
2022-10-04 00:41 Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 03:11 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 05:13 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 05:45 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 17:39 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
@ 2022-10-04 18:09 ` Peter Geoghegan <[email protected]>
2022-10-05 02:59 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Peter Geoghegan @ 2022-10-04 18:09 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Tue, Oct 4, 2022 at 10:39 AM Jeff Davis <[email protected]> wrote:
> On Mon, 2022-10-03 at 22:45 -0700, Peter Geoghegan wrote:
> > Once a table becomes larger than vacuum_freeze_strategy_threshold,
> > VACUUM stops marking pages all-visible in the first place,
> > consistently marking them all-frozen instead.
>
> What are the trade-offs here? Why does it depend on table size?
That's a great question. The table-level threshold
vacuum_freeze_strategy_threshold more or less buckets every table into
one of two categories: small tables and big tables. Perhaps this seems
simplistic to you. That would be an understandable reaction, given the
central importance of this threshold. The current default of 4GB could
have easily been 8GB or perhaps even 16GB instead.
It's not so much size as the rate of growth over time that matters. We
really want to do eager freezing on "growth tables", particularly
append-only tables. On the other hand we don't want to do useless
freezing on small, frequently updated tables, like pgbench_tellers or
pgbench_branches -- those tables may well require zero freezing, and
yet each VACUUM will advance relfrozenxid to a very recent value
consistently (even on Postgres 15). But "growth" is hard to capture,
because in general we have to infer things about the future from the
past, which is difficult and messy.
Since it's hard to capture "growth table vs fixed size table"
directly, we use table size as a proxy. It's far from perfect, but I
think that it will work quite well in practice because most individual
tables simply never get very large. It's very common for a relatively
small number of tables to consistently grow, without bound (perhaps
not strictly append-only tables, but tables where nothing is ever
deleted and inserts keep happening). So a simplistic threshold
(combined with dynamic per-page decisions about freezing) should be
enough to avoid most of the downside of eager freezing. In particular,
we will still freeze lazily in tables where it's obviously very
unlikely to be worth it.
In general I think that being correct on average is overrated. It's
more important to always avoid being dramatically wrong -- especially
if there is no way to course correct in the next VACUUM. Although I
think that we have a decent chance of coming out ahead by every
available metric, that isn't really the goal. Why should performance
stability not have some cost, at least in some cases? I want to keep
the cost as low as possible (often "negative cost" relative to
Postgres 15), but overall I am consciously making a trade-off. There
are downsides.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: New strategies for freezing, advancing relfrozenxid early
2022-10-04 00:41 Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 03:11 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 05:13 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 05:45 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 17:39 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 18:09 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
@ 2022-10-05 02:59 ` Jeff Davis <[email protected]>
2022-10-05 04:00 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
0 siblings, 1 reply; 29+ messages in thread
From: Jeff Davis @ 2022-10-05 02:59 UTC (permalink / raw)
To: Peter Geoghegan <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Tue, 2022-10-04 at 11:09 -0700, Peter Geoghegan wrote:
> So a simplistic threshold
> (combined with dynamic per-page decisions about freezing) should be
> enough to avoid most of the downside of eager freezing.
...
> I want to keep
> the cost as low as possible (often "negative cost" relative to
> Postgres 15), but overall I am consciously making a trade-off. There
> are downsides.
I am fine with that, but I'd like us all to understand what the
downsides are.
If I understand correctly:
1. Eager freezing (meaning to freeze at the same time as setting all-
visible) causes a modest amount of WAL traffic, hopefully before the
next checkpoint so we can avoid FPIs. Lazy freezing (meaning set all-
visible but don't freeze) defers the work, and it might never need to
be done; but if it does, it can cause spikes at unfortunate times and
is more likely to generate more FPIs.
2. You're trying to mitigate the downsides of eager freezing by:
a. when freezing a tuple, eagerly freeze other tuples on that page
b. optimize WAL freeze records
3. You're trying to capture the trade-off in #1 by using the table size
as a proxy. Deferred work is only really a problem for big tables, so
that's where you use eager freezing. But maybe we can just always use
eager freezing?:
a. You're mitigating the WAL work for freezing.
b. A lot of people run with checksums on, meaning that setting the
all-visible bit requires WAL work anyway, and often FPIs.
c. All-visible is conceptually similar to freezing, but less
important, and it feels more and more like the design concept of all-
visible isn't carrying its weight.
d. (tangent) I had an old patch[1] that actually removed
PD_ALL_VISIBLE (the page bit, not the VM bit), which was rejected, but
perhaps its time has come?
Regards,
Jeff Davis
[1]
https://www.postgresql.org/message-id/1353551097.11440.128.camel%40sussancws0025
^ permalink raw reply [nested|flat] 29+ messages in thread
* Re: New strategies for freezing, advancing relfrozenxid early
2022-10-04 00:41 Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 03:11 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 05:13 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 05:45 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 17:39 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 18:09 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-05 02:59 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
@ 2022-10-05 04:00 ` Peter Geoghegan <[email protected]>
0 siblings, 0 replies; 29+ messages in thread
From: Peter Geoghegan @ 2022-10-05 04:00 UTC (permalink / raw)
To: Jeff Davis <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Tue, Oct 4, 2022 at 7:59 PM Jeff Davis <[email protected]> wrote:
> I am fine with that, but I'd like us all to understand what the
> downsides are.
Although I'm sure that there must be one case that loses measurably,
it's not particularly obvious where to start looking for one. I mean
it's easy to imagine individual pages that we lose on, but a practical
test case where most of the pages are like that reliably is harder to
imagine.
> If I understand correctly:
>
> 1. Eager freezing (meaning to freeze at the same time as setting all-
> visible) causes a modest amount of WAL traffic, hopefully before the
> next checkpoint so we can avoid FPIs. Lazy freezing (meaning set all-
> visible but don't freeze) defers the work, and it might never need to
> be done; but if it does, it can cause spikes at unfortunate times and
> is more likely to generate more FPIs.
Lazy freezing means to freeze every eligible tuple (every XID <
OldestXmin) when one or more XIDs are before FreezeLimit. Eager
freezing means freezing every eligible tuple when the page is about to
be set all-visible, or whenever lazy freezing would trigger freezing.
Eager freezing tends to avoid big spikes in larger tables, which is
very important. It can sometimes be cheaper and better in every way
than lazy freezing. Though lazy freezing sometimes retains an
advantage by avoiding freezing that is never going to be needed
altogether, typically only in small tables.
Lazy freezing is fairly similar to what we do on HEAD now -- though
it's not identical. It's still "page level freezing". It has lazy
criteria for triggering page freezing.
> 2. You're trying to mitigate the downsides of eager freezing by:
> a. when freezing a tuple, eagerly freeze other tuples on that page
> b. optimize WAL freeze records
Sort of.
Both of these techniques apply to eager freezing too, in fact. It's
just that eager freezing is likely to do the bulk of all freezing that
actually goes ahead. It'll disproportionately be helped by these
techniques because it'll do most actual freezing that goes ahead (even
when most VACUUM operations use the lazy freezing strategy, which is
probably the common case -- just because lazy freezing freezes
lazily).
> 3. You're trying to capture the trade-off in #1 by using the table size
> as a proxy. Deferred work is only really a problem for big tables, so
> that's where you use eager freezing.
Right.
> But maybe we can just always use
> eager freezing?:
That doesn't seem like a bad idea, though it might be tricky to put
into practice. It might be possible to totally unite the concept of
all-visible and all-frozen pages in the scope of this work. But there
are surprisingly many tricky details involved. I'm not surprised that
you're suggesting this -- it basically makes sense to me. It's just
the practicalities that I worry about here.
> a. You're mitigating the WAL work for freezing.
I don't see why this would be true. Lazy vs Eager are exactly the same
for a given page at the point that freezing is triggered. We'll freeze
all eligible tuples (often though not always every tuple), or none at
all.
Lazy vs Eager describe the policy for deciding to freeze a page, but
do not affect the actual execution steps taken once we decide to
freeze.
> b. A lot of people run with checksums on, meaning that setting the
> all-visible bit requires WAL work anyway, and often FPIs.
The idea of rolling the WAL records into one does seem appealing, but
we'd still need the original WAL record to set a page all-visible in
VACUUM's second heap pass (only setting a page all-visible in the
first heap pass could be optimized by making the FREEZE_PAGE WAL
record mark the page all-visible too). Or maybe we'd roll that into
the VACUUM WAL record at the same time.
In any case the second heap pass would have to have a totally
different WAL logging strategy to the first heap pass. Not
insurmountable, but not exactly an easy thing to do in passing either.
> c. All-visible is conceptually similar to freezing, but less
> important, and it feels more and more like the design concept of all-
> visible isn't carrying its weight.
Well, not quite -- at least not on the VM side itself.
There are cases where heap_lock_tuple() will update a tuple's xmax,
replacing it with a new Multi. This will necessitate clearly the
page's all-frozen bit in the VM -- but the all-visible bit will stay
set. This is why it's possible for small numbers of all-visible pages
to appear even in large tables that have been eagerly frozen.
> d. (tangent) I had an old patch[1] that actually removed
> PD_ALL_VISIBLE (the page bit, not the VM bit), which was rejected, but
> perhaps its time has come?
I remember that pgCon developer meeting well. :-)
If anything your original argument for getting rid of PD_ALL_VISIBLE
is weakened by the proposal to merge together the WAL records for
freezing and for setting a heap page all visible. You'd know for sure
that the page will be dirtied when such a WAL record needed to be
written, so there is actually no reason to care about dirtying the
page. No?
I'm in favor of reducing the number of WAL records required in common
cases if at all possible -- purely because the generic WAL record
overhead of having an extra WAL record does probably add to the WAL
overhead for work performed in lazy_scan_prune(). But it seems like
separate work to me.
--
Peter Geoghegan
^ permalink raw reply [nested|flat] 29+ messages in thread
end of thread, other threads:[~2022-10-05 04:00 UTC | newest]
Thread overview: 29+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2020-04-30 05:06 [PATCH 1/2] Move tablespace cleanup out of pg_regress. Kyotaro Horiguchi <[email protected]>
2022-10-04 00:41 Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 03:11 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 05:13 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 05:45 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-04 17:39 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-04 18:09 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
2022-10-05 02:59 ` Re: New strategies for freezing, advancing relfrozenxid early Jeff Davis <[email protected]>
2022-10-05 04:00 ` Re: New strategies for freezing, advancing relfrozenxid early Peter Geoghegan <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox