Occasional tablespace.sql failures in check-world -jnn

Started by Andres Freundabout 5 years ago7 messages

andres@anarazel.de

about 5 years ago

Hi,

For fairly obvious reasons I like to run check-world in parallel [1]make -Otarget -j20 -s check-world && echo success || echo failed. In
the last few months I've occasionally seen failures during that that I
cannot recall seeing before.

--- /home/andres/build/postgres/13-assert/vpath/src/test/regress/expected/tablespace.out        2020-12-07 18:41:23.079235588 -0800
+++ /home/andres/build/postgres/13-assert/vpath/src/test/regress/results/tablespace.out 2020-12-07 18:42:01.892632468 -0800
@@ -209,496 +209,344 @@
 ERROR:  cannot specify default tablespace for partitioned relations
 CREATE TABLE testschema.dflt (a int PRIMARY KEY USING INDEX TABLESPACE regress_tblspace) PARTITION BY LIST (a);
 ERROR:  cannot specify default tablespace for partitioned relations
 -- but these work:
 CREATE TABLE testschema.dflt (a int PRIMARY KEY USING INDEX TABLESPACE regress_tblspace) PARTITION BY LIST (a) TABLESPACE regress_tblspace;
 SET default_tablespace TO '';
 CREATE TABLE testschema.dflt2 (a int PRIMARY KEY) PARTITION BY LIST (a);
 DROP TABLE testschema.dflt, testschema.dflt2;
 -- check that default_tablespace doesn't affect ALTER TABLE index rebuilds
 CREATE TABLE testschema.test_default_tab(id bigint) TABLESPACE regress_tblspace;
+ERROR:  could not create directory "pg_tblspc/16387/PG_13_202007201/16384": No such file or directory
 INSERT INTO testschema.test_default_tab VALUES (1);

(many failures follow)

I suspect this is related to the pg_upgrade test and the main regression
test running at the same time. We have the following in src/test/regress/GNUMakefile

# Tablespace setup

.PHONY: tablespace-setup
tablespace-setup:
echo $(realpath ./testtablespace) >> /tmp/tablespace.log
rm -rf ./testtablespace
mkdir ./testtablespace
...

which pg_upgrade triggers. Even though it, as far as I can tell, never
actually ends up putting any data in it:

# Send installcheck outputs to a private directory. This avoids conflict when
# check-world runs pg_upgrade check concurrently with src/test/regress check.
# To retrieve interesting files after a run, use pattern tmp_check/*/*.diffs.
outputdir="$temp_root/regress"
EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --outputdir=$outputdir"
export EXTRA_REGRESS_OPTS
mkdir "$outputdir"
mkdir "$outputdir"/testtablespace

It's not clear to me why we have this logic in the makefile at all?
Somebody taught pg_regress to do so, but only on windows... See
convert_sourcefiles_in().

The other thing that confuses me is why I started getting that error in
*multiple* branches recently, even though I have used the parallel
check-world for ages.

Greetings,

Andres Freund

[1]: make -Otarget -j20 -s check-world && echo success || echo failed

Michael Paquier

michael@paquier.xyz

about 5 years ago

In reply to: Andres Freund (#1)

1 attachment(s)

Re: Occasional tablespace.sql failures in check-world -jnn

On Tue, Dec 08, 2020 at 05:29:11PM -0800, Andres Freund wrote:

I suspect this is related to the pg_upgrade test and the main regression
test running at the same time. We have the following in
src/test/regress/GNUMakefile.

Yes, this one is not completely new to -hackers. See patch 0002 here
that slightly touched the topic by creating a specific makefile rule,
but I never got back to it as I never got annoyed by this problem:
/messages/by-id/20200511.171354.514381788845037011.horikyota.ntt@gmail.com
What we have here is not a solution though...

It's not clear to me why we have this logic in the makefile at all?
Somebody taught pg_regress to do so, but only on windows... See
convert_sourcefiles_in().

... Because we may still introduce this problem again if some new
stuff uses src/test/pg_regress in a way similar to pg_upgrade,
triggering again tablespace-setup. Something like the attached may be
enough, though I have not spent much time checking the surroundings,
Windows included.

The other thing that confuses me is why I started getting that error in
*multiple* branches recently, even though I have used the parallel
check-world for ages.

Perhaps you have just increased -j lately or moved to a faster machine
where there are higher changes of collision?
--
Michael

Attachments:

regress-tbspace-v1.patchtext/x-diff; charset=us-asciiDownload

diff --git a/src/bin/pg_upgrade/test.sh b/src/bin/pg_upgrade/test.sh
index 04aa7fd9f5..355477a8ee 100644
--- a/src/bin/pg_upgrade/test.sh
+++ b/src/bin/pg_upgrade/test.sh
@@ -106,7 +106,6 @@ outputdir="$temp_root/regress"
 EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --outputdir=$outputdir"
 export EXTRA_REGRESS_OPTS
 mkdir "$outputdir"
-mkdir "$outputdir"/testtablespace
 
 logdir=`pwd`/log
 rm -rf "$logdir"
diff --git a/src/test/regress/GNUmakefile b/src/test/regress/GNUmakefile
index c830627b00..583f975f08 100644
--- a/src/test/regress/GNUmakefile
+++ b/src/test/regress/GNUmakefile
@@ -114,13 +114,6 @@ submake-contrib-spi: | submake-libpgport submake-generated-headers
 
 .PHONY: submake-contrib-spi
 
-# Tablespace setup
-
-.PHONY: tablespace-setup
-tablespace-setup:
-	rm -rf ./testtablespace
-	mkdir ./testtablespace
-
 
 ##
 ## Run tests
@@ -128,19 +121,19 @@ tablespace-setup:
 
 REGRESS_OPTS = --dlpath=. --max-concurrent-tests=20 $(EXTRA_REGRESS_OPTS)
 
-check: all tablespace-setup
+check: all
 	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
 
-check-tests: all tablespace-setup | temp-install
+check-tests: all | temp-install
 	$(pg_regress_check) $(REGRESS_OPTS) $(MAXCONNOPT) $(TESTS) $(EXTRA_TESTS)
 
-installcheck: all tablespace-setup
+installcheck: all
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule $(EXTRA_TESTS)
 
-installcheck-parallel: all tablespace-setup
+installcheck-parallel: all
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) $(EXTRA_TESTS)
 
-installcheck-tests: all tablespace-setup
+installcheck-tests: all
 	$(pg_regress_installcheck) $(REGRESS_OPTS) $(TESTS) $(EXTRA_TESTS)
 
 standbycheck: all
@@ -152,10 +145,10 @@ runcheck: check
 runtest: installcheck
 runtest-parallel: installcheck-parallel
 
-bigtest: all tablespace-setup
+bigtest: all
 	$(pg_regress_installcheck) $(REGRESS_OPTS) --schedule=$(srcdir)/serial_schedule numeric_big
 
-bigcheck: all tablespace-setup | temp-install
+bigcheck: all | temp-install
 	$(pg_regress_check) $(REGRESS_OPTS) --schedule=$(srcdir)/parallel_schedule $(MAXCONNOPT) numeric_big
 
 
diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c
index 23d7d0beb2..111457ee8f 100644
--- a/src/test/regress/pg_regress.c
+++ b/src/test/regress/pg_regress.c
@@ -506,24 +506,22 @@ 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 so as it is possible to run the regression tests as a
-	 * Windows administrative user account with the restricted token obtained
-	 * when starting pg_regress.  On other platforms we expect the Makefile to
-	 * take care of that.
+	 * Clean out the test tablespace dir, or create it if it doesn't exist.
+	 * On Windows, doing this cleanup here makes possible to run the
+	 * regression tests as a Windows administrative user account with the
+	 * restricted token obtained when starting pg_regress.
 	 */
 	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 266098e193..3e9110e4ed 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -571,7 +571,6 @@ sub upgradecheck
 	my $outputdir          = "$tmp_root/regress";
 	my @EXTRA_REGRESS_OPTS = ("--outputdir=$outputdir");
 	mkdir "$outputdir"                || die $!;
-	mkdir "$outputdir/testtablespace" || die $!;
 
 	my $logdir = "$topdir/src/bin/pg_upgrade/log";
 	rmtree($logdir);

Peter Eisentraut

peter.eisentraut@enterprisedb.com

almost 5 years ago

In reply to: Andres Freund (#1)

Re: Occasional tablespace.sql failures in check-world -jnn

On 2020-12-09 02:29, Andres Freund wrote:

I suspect this is related to the pg_upgrade test and the main regression
test running at the same time. We have the following in src/test/regress/GNUMakefile

# Tablespace setup

.PHONY: tablespace-setup
tablespace-setup:
echo $(realpath ./testtablespace) >> /tmp/tablespace.log
rm -rf ./testtablespace
mkdir ./testtablespace
...

which pg_upgrade triggers. Even though it, as far as I can tell, never
actually ends up putting any data in it:

# Send installcheck outputs to a private directory. This avoids conflict when
# check-world runs pg_upgrade check concurrently with src/test/regress check.
# To retrieve interesting files after a run, use pattern tmp_check/*/*.diffs.
outputdir="$temp_root/regress"
EXTRA_REGRESS_OPTS="$EXTRA_REGRESS_OPTS --outputdir=$outputdir"
export EXTRA_REGRESS_OPTS
mkdir "$outputdir"
mkdir "$outputdir"/testtablespace

It's not clear to me why we have this logic in the makefile at all?
Somebody taught pg_regress to do so, but only on windows... See
convert_sourcefiles_in().

I vaguely recall that this had something to do with SELinux (or
something similar?), where it matters in what context you create a file
or directory and then certain properties attach to it that are relevant
to subsequent programs that run on it. Again, vague.

Michael Paquier

michael@paquier.xyz

almost 5 years ago

In reply to: Peter Eisentraut (#3)

Re: Occasional tablespace.sql failures in check-world -jnn

On Fri, Jan 15, 2021 at 09:59:02AM +0100, Peter Eisentraut wrote:

I vaguely recall that this had something to do with SELinux (or something
similar?), where it matters in what context you create a file or directory
and then certain properties attach to it that are relevant to subsequent
programs that run on it. Again, vague.

Hmm. Does it? sepgsql has some tests for tablespaces involving only
pg_default, so it does not seem that this applies in the context of
the regression tests. The cleanup of testtablespace in GNUMakefile
comes from 2467394, as of June 2004, that introduced tablespaces.
--
Michael

Peter Eisentraut

peter.eisentraut@enterprisedb.com

almost 5 years ago

In reply to: Michael Paquier (#2)

Re: Occasional tablespace.sql failures in check-world -jnn

On 09.12.20 08:55, Michael Paquier wrote:

It's not clear to me why we have this logic in the makefile at all?
Somebody taught pg_regress to do so, but only on windows... See
convert_sourcefiles_in().

... Because we may still introduce this problem again if some new
stuff uses src/test/pg_regress in a way similar to pg_upgrade,
triggering again tablespace-setup. Something like the attached may be
enough, though I have not spent much time checking the surroundings,
Windows included.

This patch looks alright to me.

Michael Paquier

michael@paquier.xyz

almost 5 years ago

In reply to: Peter Eisentraut (#5)

Re: Occasional tablespace.sql failures in check-world -jnn

On Mon, Mar 08, 2021 at 11:53:57AM +0100, Peter Eisentraut wrote:

On 09.12.20 08:55, Michael Paquier wrote:

... Because we may still introduce this problem again if some new
stuff uses src/test/pg_regress in a way similar to pg_upgrade,
triggering again tablespace-setup. Something like the attached may be
enough, though I have not spent much time checking the surroundings,
Windows included.

This patch looks alright to me.

So, I have spent more time checking the surroundings of this patch,
and finally applied it. Thanks for the review, Peter.
--
Michael

Andres Freund

andres@anarazel.de

almost 5 years ago

In reply to: Michael Paquier (#6)

Re: Occasional tablespace.sql failures in check-world -jnn

Hi,

On 2021-03-10 15:40:38 +0900, Michael Paquier wrote:

On Mon, Mar 08, 2021 at 11:53:57AM +0100, Peter Eisentraut wrote:

On 09.12.20 08:55, Michael Paquier wrote:

... Because we may still introduce this problem again if some new
stuff uses src/test/pg_regress in a way similar to pg_upgrade,
triggering again tablespace-setup. Something like the attached may be
enough, though I have not spent much time checking the surroundings,
Windows included.

This patch looks alright to me.

So, I have spent more time checking the surroundings of this patch,
and finally applied it. Thanks for the review, Peter.

Thanks!

Greetings,

Andres Freund