From: Daniel Gustafsson <daniel@yesql.se>
Message-Id: <6BF8531D-355E-4D41-BA13-653505F97684@yesql.se>
Content-Type: multipart/mixed;
	boundary="Apple-Mail=_85ADE0B7-2CDA-40FF-856D-FDD26568F145"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.120.41.1.4\))
Subject: Re: pgsql: Clean up role created in new subscription test.
Date: Wed, 27 Mar 2024 14:42:02 +0100
In-Reply-To: <20240325184824.uxbh6o7w4442o4nv@awork3.anarazel.de>
Cc: Peter Eisentraut <peter@eisentraut.org>,
 vignesh C <vignesh21@gmail.com>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>,
 Tom Lane <tgl@sss.pgh.pa.us>,
 Robert Haas <robertmhaas@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
To: Andres Freund <andres@anarazel.de>
References: <202312011219.aikuwve3cbku@alvherre.pgsql>
 <57373BD5-C0D9-486D-9096-276FE91411D7@yesql.se>
 <CALDaNm1eHOMxq0GmZstftKW+W8xxRsG6Ww5u4ncEVq7ZD+NrVw@mail.gmail.com>
 <1ECB500B-F76F-4BBE-AE4D-91DDC5E59D7E@yesql.se>
 <a6d4b632-e8fd-43e0-91ab-643d1e990941@eisentraut.org>
 <20240325184824.uxbh6o7w4442o4nv@awork3.anarazel.de>
Archived-At: 
 <https://www.postgresql.org/message-id/6BF8531D-355E-4D41-BA13-653505F97684%40yesql.se>
Precedence: bulk


--Apple-Mail=_85ADE0B7-2CDA-40FF-856D-FDD26568F145
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

> On 25 Mar 2024, at 19:48, Andres Freund <andres@anarazel.de> wrote:

> I don't think it's that, but that the freebsd task tests the =
installcheck
> equivalent in meson.  I haven't checked what your patch is doing, but =
perhaps
> the issue is that it's seeing global objects concurrently created by =
another
> test?

Sorry, I had a look when Peter replied a while back but forgot to update =
this
thread.  The reason for the failure is that when running multiple =
pg_regress in
parallel against a single instance it is impossible to avoid global =
object
pollution from other tests concurrently executing.  Since pg_regress has =
no
idea about the contents of the tests it also cannot apply any smarts in
filtering out such objects.  The CI failures comes from the contrib =
tests which
run in parallel.

The only option is to make the check opt-in via a command-line parameter =
for
use it in the main regress tests, and not use it for the contrib tests.  =
The
attached v7 does that and passes CI, but I wonder if it's worth it all =
with
that restriction?

The 0001 cleanup patch is still relevant (and was found by using this =
feature)
though but that might be all for this thread.

--
Daniel Gustafsson


--Apple-Mail=_85ADE0B7-2CDA-40FF-856D-FDD26568F145
Content-Disposition: attachment;
	filename=v7-0002-pg_regress-Detect-global-objects-left-over-after-.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v7-0002-pg_regress-Detect-global-objects-left-over-after-.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=20303fd690e09584ac6ae39e0e1bd3bc88e83cfefe=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20Daniel=20Gustafsson=20=
<dgustafsson@postgresql.org>=0ADate:=20Fri,=201=20Dec=202023=2012:10:17=20=
+0100=0ASubject:=20[PATCH=20v7=202/2]=20pg_regress:=20Detect=20global=20=
objects=20left=20over=20after=20test=0A=0AThe=20project=20policy=20is=20=
that=20regress=20tests=20should=20not=20leave=20any=0Aglobal=20objects=20=
behind,=20but=20there=20was=20no=20check=20enforcing=20this.=0A=
pg_regress=20will=20now=20extract=20a=20list=20of=20roles,=20tablespaces=20=
and=0Asubscriptions=20before=20and=20after=20running=20the=20test(s),=20=
and=20then=0Acompares=20the=20lists=20to=20ensure=20no=20new=20objects=20=
are=20present.=0A=0ADiscussion:=20=
https://postgr.es/m/CA+TgmoZ+K2OU0Ojs8_4SgqPWJJq7e0XiLd-uJGZ11R_WKF1URQ@ma=
il.gmail.com=0A---=0A=20src/test/regress/GNUmakefile=20=20|=20=202=20+-=0A=
=20src/test/regress/pg_regress.c=20|=2098=20=
+++++++++++++++++++++++++++++++++++=0A=202=20files=20changed,=2099=20=
insertions(+),=201=20deletion(-)=0A=0Adiff=20--git=20=
a/src/test/regress/GNUmakefile=20b/src/test/regress/GNUmakefile=0Aindex=20=
9003435aab..1f774693f4=20100644=0A---=20a/src/test/regress/GNUmakefile=0A=
+++=20b/src/test/regress/GNUmakefile=0A@@=20-115,7=20+115,7=20@@=20=
REGRESS_OPTS=20=3D=20--dlpath=3D.=20--max-concurrent-tests=3D20=20\=0A=20=
=09$(EXTRA_REGRESS_OPTS)=0A=20=0A=20check:=20all=0A-=09=
$(pg_regress_check)=20$(REGRESS_OPTS)=20=
--schedule=3D$(srcdir)/parallel_schedule=20$(MAXCONNOPT)=20=
$(EXTRA_TESTS)=0A+=09$(pg_regress_check)=20$(REGRESS_OPTS)=20=
--globals-check=20--schedule=3D$(srcdir)/parallel_schedule=20=
$(MAXCONNOPT)=20$(EXTRA_TESTS)=0A=20=0A=20check-tests:=20all=20|=20=
temp-install=0A=20=09$(pg_regress_check)=20$(REGRESS_OPTS)=20=
$(MAXCONNOPT)=20$(TESTS)=20$(EXTRA_TESTS)=0Adiff=20--git=20=
a/src/test/regress/pg_regress.c=20b/src/test/regress/pg_regress.c=0A=
index=2076f01c73f5..c59b6b4964=20100644=0A---=20=
a/src/test/regress/pg_regress.c=0A+++=20b/src/test/regress/pg_regress.c=0A=
@@=20-120,6=20+120,7=20@@=20static=20char=20*dlpath=20=3D=20PKGLIBDIR;=0A=
=20static=20char=20*user=20=3D=20NULL;=0A=20static=20_stringlist=20=
*extraroles=20=3D=20NULL;=0A=20static=20char=20*config_auth_datadir=20=3D=20=
NULL;=0A+static=20bool=20globals_check=20=3D=20false;=0A=20=0A=20/*=20=
internal=20variables=20*/=0A=20static=20const=20char=20*progname;=0A@@=20=
-132,6=20+133,8=20@@=20static=20char=20sockself[MAXPGPATH];=0A=20static=20=
char=20socklock[MAXPGPATH];=0A=20static=20StringInfo=20failed_tests=20=3D=20=
NULL;=0A=20static=20bool=20in_note=20=3D=20false;=0A+static=20=
_stringlist=20*globals_before=20=3D=20NULL;=0A+static=20_stringlist=20=
*globals_after=20=3D=20NULL;=0A=20=0A=20static=20_resultmap=20*resultmap=20=
=3D=20NULL;=0A=20=0A@@=20-244,6=20+247,57=20@@=20=
split_to_stringlist(const=20char=20*s,=20const=20char=20*delim,=20=
_stringlist=20**listhead)=0A=20=09free(sc);=0A=20}=0A=20=0A+/*=0A+=20*=20=
Query=20the=20cluster=20and=20save=20the=20first=20column=20of=20the=20=
results=20into=20a=0A+=20*=20stringlist.=0A+=20*/=0A+static=20void=0A=
+query_to_stringlist(const=20char=20*dbname,=20const=20char=20*query,=20=
_stringlist=20**listhead)=0A+{=0A+=09PGconn=09=20=20=20*conn;=0A+=09=
PGresult=20=20=20*res;=0A+=09StringInfo=09buf=20=3D=20makeStringInfo();=0A=
+=0A+=09appendStringInfo(buf,=20"dbname=3D%s",=20dbname);=0A+=0A+=09conn=20=
=3D=20PQconnectdb(buf->data);=0A+=09if=20(PQstatus(conn)=20!=3D=20=
CONNECTION_OK)=0A+=09=09bail("unable=20to=20connect=20to=20cluster:=20=
%s",=20PQerrorMessage(conn));=0A+=0A+=09res=20=3D=20PQexec(conn,=20=
query);=0A+=09if=20(PQresultStatus(res)=20!=3D=20PGRES_TUPLES_OK)=0A+=09=09=
bail("unable=20to=20query=20database:=20%s",=20PQerrorMessage(conn));=0A=
+=0A+=09for=20(int=20i=20=3D=200;=20i=20<=20PQntuples(res);=20i++)=0A+=09=
=09add_stringlist_item(listhead,=20pstrdup(PQgetvalue(res,=20i,=200)));=0A=
+=0A+=09pfree(buf->data);=0A+=09PQclear(res);=0A+=09PQfinish(conn);=0A+}=0A=
+=0A+/*=0A+=20*=20Compare=20two=20stringlists=20for=20string=20equality=0A=
+=20*/=0A+static=20bool=0A+diff_stringlist(_stringlist=20*a,=20=
_stringlist=20*b)=0A+{=0A+=09_stringlist=20*sla=20=3D=20a;=0A+=09=
_stringlist=20*slb=20=3D=20b;=0A+=0A+=09for=20(;=20sla=20&&=20slb;=20sla=20=
=3D=20sla->next,=20slb=20=3D=20slb->next)=0A+=09{=0A+=09=09if=20=
(strcmp(sla->str,=20slb->str)=20!=3D=200)=0A+=09=09=09return=20false;=0A=
+=09}=0A+=0A+=09/*=20If=20we=20reached=20the=20end=20of=20both=20lists=20=
at=20the=20same=20time,=20they=20are=20equal=20*/=0A+=09if=20(sla=20=3D=3D=
=20NULL=20&&=20slb=20=3D=3D=20NULL)=0A+=09=09return=20true;=0A+=0A+=09=
return=20false;=0A+}=0A+=0A=20/*=0A=20=20*=20Bailing=20out=20is=20for=20=
unrecoverable=20errors=20which=20prevents=20further=20testing=20to=0A=20=20=
*=20occur=20and=20after=20which=20the=20test=20run=20should=20be=20=
aborted.=20By=20passing=20noatexit=0A@@=20-2041,6=20+2095,7=20@@=20=
help(void)=0A=20=09printf(_("=20=20=20=20=20=20--dlpath=3DDIR=20=20=20=20=
=20=20=20=20=20=20=20=20=20=20look=20for=20dynamic=20libraries=20in=20=
DIR\n"));=0A=20=09printf(_("=20=20=20=20=20=20--encoding=3DENCODING=20=20=
=20=20=20=20=20use=20ENCODING=20as=20the=20encoding\n"));=0A=20=09=
printf(_("=20=20=20=20=20=20--expecteddir=3DDIR=20=20=20=20=20=20=20=20=20=
take=20expected=20files=20from=20DIR=20(default=20\".\")\n"));=0A+=09=
printf(_("=20=20=20=20=20=20--globals-check=20=20=20=20=20=20=20=20=20=20=
=20fail=20if=20global=20objects=20are=20left=20after=20test\n"));=0A=20=09=
printf(_("=20=20-h,=20--help=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=
=20=20=20=20=20show=20this=20help,=20then=20exit\n"));=0A=20=09=
printf(_("=20=20=20=20=20=20--inputdir=3DDIR=20=20=20=20=20=20=20=20=20=20=
=20=20take=20input=20files=20from=20DIR=20(default=20\".\")\n"));=0A=20=09=
printf(_("=20=20=20=20=20=20--launcher=3DCMD=20=20=20=20=20=20=20=20=20=20=
=20=20use=20CMD=20as=20launcher=20of=20psql\n"));=0A@@=20-2105,6=20=
+2160,7=20@@=20regression_main(int=20argc,=20char=20*argv[],=0A=20=09=09=
{"config-auth",=20required_argument,=20NULL,=2024},=0A=20=09=09=
{"max-concurrent-tests",=20required_argument,=20NULL,=2025},=0A=20=09=09=
{"expecteddir",=20required_argument,=20NULL,=2026},=0A+=09=09=
{"globals-check",=20no_argument,=20NULL,=2027},=0A=20=09=09{NULL,=200,=20=
NULL,=200}=0A=20=09};=0A=20=0A@@=20-2233,6=20+2289,9=20@@=20=
regression_main(int=20argc,=20char=20*argv[],=0A=20=09=09=09case=2026:=0A=
=20=09=09=09=09expecteddir=20=3D=20pg_strdup(optarg);=0A=20=09=09=09=09=
break;=0A+=09=09=09case=2027:=0A+=09=09=09=09globals_check=20=3D=20true;=0A=
+=09=09=09=09break;=0A=20=09=09=09default:=0A=20=09=09=09=09/*=20=
getopt_long=20already=20emitted=20a=20complaint=20*/=0A=20=09=09=09=09=
pg_log_error_hint("Try=20\"%s=20--help\"=20for=20more=20information.",=0A=
@@=20-2612,6=20+2671,21=20@@=20regression_main(int=20argc,=20char=20=
*argv[],=0A=20=09=09=09create_role(sl->str,=20dblist);=0A=20=09}=0A=20=0A=
+=09/*=0A+=09=20*=20Store=20the=20global=20objects=20before=20the=20test=20=
starts=20such=20that=20we=20can=20check=0A+=09=20*=20for=20any=20objects=20=
left=20behind=20after=20the=20tests=20finish.=0A+=09=20*/=0A+=09if=20=
(globals_check)=0A+=09{=0A+=09=09query_to_stringlist("postgres",=0A+=09=09=
=09=09=09=09=09"(SELECT=20rolname=20AS=20obj=20FROM=20=
pg_catalog.pg_roles=20ORDER=20BY=201)=20"=0A+=09=09=09=09=09=09=09"UNION=20=
ALL=20"=0A+=09=09=09=09=09=09=09"(SELECT=20spcname=20AS=20obj=20FROM=20=
pg_catalog.pg_tablespace=20ORDER=20BY=201)=20"=0A+=09=09=09=09=09=09=09=
"UNION=20ALL=20"=0A+=09=09=09=09=09=09=09"(SELECT=20subname=20AS=20obj=20=
FROM=20pg_catalog.pg_subscription=20ORDER=20BY=201)",=0A+=09=09=09=09=09=09=
=09&globals_before);=0A+=09}=0A+=0A=20=09/*=0A=20=09=20*=20Ready=20to=20=
run=20the=20tests=0A=20=09=20*/=0A@@=20-2625,6=20+2699,30=20@@=20=
regression_main(int=20argc,=20char=20*argv[],=0A=20=09=09=
run_single_test(sl->str,=20startfunc,=20postfunc);=0A=20=09}=0A=20=0A+=09=
/*=0A+=09=20*=20Store=20the=20global=20objects=20again=20after=20the=20=
tests=20have=20finished=20and=0A+=09=20*=20compare=20with=20the=20list=20=
from=20before=20the=20tests.=0A+=09=20*/=0A+=09if=20(globals_check)=0A+=09=
{=0A+=09=09query_to_stringlist("postgres",=0A+=09=09=09=09=09=09=09=
"(SELECT=20rolname=20AS=20obj=20FROM=20pg_catalog.pg_roles=20ORDER=20BY=20=
1)=20"=0A+=09=09=09=09=09=09=09"UNION=20ALL=20"=0A+=09=09=09=09=09=09=09=
"(SELECT=20spcname=20AS=20obj=20FROM=20pg_catalog.pg_tablespace=20ORDER=20=
BY=201)=20"=0A+=09=09=09=09=09=09=09"UNION=20ALL=20"=0A+=09=09=09=09=09=09=
=09"(SELECT=20subname=20AS=20obj=20FROM=20pg_catalog.pg_subscription=20=
ORDER=20BY=201)",=0A+=09=09=09=09=09=09=09&globals_after);=0A+=0A+=09=09=
if=20(!diff_stringlist(globals_before,=20globals_after))=0A+=09=09{=0A+=09=
=09=09test_status_failed("global_objects",=200.0,=20false);=0A+=09=09}=0A=
+=09=09else=0A+=09=09{=0A+=09=09=09test_status_ok("global_objects",=20=
0.0,=20false);=0A+=09=09}=0A+=09}=0A+=0A=20=09/*=0A=20=09=20*=20Shut=20=
down=20temp=20installation's=20postmaster=0A=20=09=20*/=0A--=20=0A2.32.1=20=
(Apple=20Git-133)=0A=0A=

--Apple-Mail=_85ADE0B7-2CDA-40FF-856D-FDD26568F145
Content-Disposition: attachment;
	filename=v7-0001-Drop-global-objects-after-completed-test.patch
Content-Type: application/octet-stream;
	x-unix-mode=0644;
	name="v7-0001-Drop-global-objects-after-completed-test.patch"
Content-Transfer-Encoding: quoted-printable

=46rom=2078aef85ae769fbdce204e6fa15d482c7fee33e92=20Mon=20Sep=2017=20=
00:00:00=202001=0AFrom:=20Daniel=20Gustafsson=20=
<dgustafsson@postgresql.org>=0ADate:=20Fri,=201=20Dec=202023=2012:04:49=20=
+0100=0ASubject:=20[PATCH=20v7=201/2]=20Drop=20global=20objects=20after=20=
completed=20test=0A=0AProject=20policy=20is=20to=20not=20leave=20global=20=
objects=20behind=20after=20a=20regress=0Atest=20run.=0A---=0A=20=
src/test/modules/test_pg_dump/expected/test_pg_dump.out=20|=202=20++=0A=20=
src/test/modules/test_pg_dump/sql/test_pg_dump.sql=20=20=20=20=20=20|=20=
3=20+++=0A=202=20files=20changed,=205=20insertions(+)=0A=0Adiff=20--git=20=
a/src/test/modules/test_pg_dump/expected/test_pg_dump.out=20=
b/src/test/modules/test_pg_dump/expected/test_pg_dump.out=0Aindex=20=
f14f3a6664..f57c96aeb9=20100644=0A---=20=
a/src/test/modules/test_pg_dump/expected/test_pg_dump.out=0A+++=20=
b/src/test/modules/test_pg_dump/expected/test_pg_dump.out=0A@@=20-91,3=20=
+91,5=20@@=20ALTER=20EXTENSION=20test_pg_dump=20DROP=20SERVER=20s0;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20TABLE=20test_pg_dump_t1;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20TYPE=20test_pg_dump_e1;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20VIEW=20test_pg_dump_v1;=0A=
+DROP=20OWNED=20BY=20regress_dump_test_role=20RESTRICT;=0A+DROP=20ROLE=20=
regress_dump_test_role;=0Adiff=20--git=20=
a/src/test/modules/test_pg_dump/sql/test_pg_dump.sql=20=
b/src/test/modules/test_pg_dump/sql/test_pg_dump.sql=0Aindex=20=
a61a7c8c4c..4f1eb9d429=20100644=0A---=20=
a/src/test/modules/test_pg_dump/sql/test_pg_dump.sql=0A+++=20=
b/src/test/modules/test_pg_dump/sql/test_pg_dump.sql=0A@@=20-106,3=20=
+106,6=20@@=20ALTER=20EXTENSION=20test_pg_dump=20DROP=20SERVER=20s0;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20TABLE=20test_pg_dump_t1;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20TYPE=20test_pg_dump_e1;=0A=20=
ALTER=20EXTENSION=20test_pg_dump=20DROP=20VIEW=20test_pg_dump_v1;=0A+=0A=
+DROP=20OWNED=20BY=20regress_dump_test_role=20RESTRICT;=0A+DROP=20ROLE=20=
regress_dump_test_role;=0A--=20=0A2.32.1=20(Apple=20Git-133)=0A=0A=

--Apple-Mail=_85ADE0B7-2CDA-40FF-856D-FDD26568F145--