Feedback-ID: i0fe9450f:Fastmail
Date: Fri, 29 Nov 2024 14:06:36 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Michail Nikolaev <michail.nikolaev@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
	nathan@postgresql.org, Heikki Linnakangas <hlinnaka@iki.fi>
Subject: Re: Strange assertion in procarray.c
Message-ID: <Z0lL3GKCyijL8HcI@paquier.xyz>
References: <CANtu0oiTgFW47QgpTwrMOVm3Bq4N0Y5bjvTy5sP0gYWLQuVgjw@mail.gmail.com>
 <CANtu0ojbx6=esP8euQgzD1CN6tigTQvDmupwEmLTHZT=6_yx_A@mail.gmail.com>
 <CANtu0oiXq548OM_RqBbDQ0dgqtKLAg=x_Fu4eOGRNW0tBsNj_Q@mail.gmail.com>
 <CANtu0oiBAcVnzYYETbWY+2gFXUeAx8BKArjnFco4LeAHfH38Sw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="+YnPvJhoyyjhkbM0"
Content-Disposition: inline
In-Reply-To: <CANtu0oiBAcVnzYYETbWY+2gFXUeAx8BKArjnFco4LeAHfH38Sw@mail.gmail.com>
Archived-At: <https://www.postgresql.org/message-id/Z0lL3GKCyijL8HcI%40paquier.xyz>
Precedence: bulk


--+YnPvJhoyyjhkbM0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Nov 28, 2024 at 09:04:36PM +0100, Michail Nikolaev wrote:
> I discovered that a similar issue was previously addressed by Heikki in
> commit [0], where installcheck was disabled for injection point tests.
> However, in the meson build configuration, this was only applied to
> regression tests - the isolation and TAP tests are still running during
> installcheck.

I fail to see how this is related?  The original issue was that this
was impossible to run safely concurrently, but now we have the
facilities able to do so.  There are a few cases where using a wait
point has limits, for example outside a transaction context for some
of the processes, but that has not really been an issue up to now.

> As demonstrated in the previously shared reproducer [1], even *local*
> injection points can cause backend crashes through unexpected side effect=
s.
> Therefore, I propose extending the installcheck disable to cover both TAP
> and isolation tests as well.
>=20
> I've attached a patch implementing these changes.

@@ -426,6 +427,7 @@ InvalidateCatalogSnapshot(void)
         pairingheap_remove(&RegisteredSnapshots, &CatalogSnapshot->ph_node=
);
         CatalogSnapshot =3D NULL;
         SnapshotResetXmin();
+        INJECTION_POINT("invalidate_catalog_snapshot_end");
[...]
+step s4_attach_locally	{ SELECT
injection_points_attach('invalidate_catalog_snapshot_end', 'wait'); }
[...]
#4  0x0000563f09d22f39 in ExceptionalCondition
(conditionName=3D0x563f0a072d00=20
"TransactionIdIsValid(proc->xid)", fileName=3D0x563f0a072a4a
"procarray.c", lineNumber=3D677)     at assert.c:66
#5  0x0000563f096f0684 in ProcArrayEndTransaction
(proc=3D0x7fbd4d083ac0, latestXid=3D750) at procarray.c:677
#6  0x0000563f088c54f3 in AbortTransaction () at xact.c:2946
#7  0x0000563f088c758d in AbortCurrentTransactionInternal () at
#xact.c:3531
#8  0x0000563f088c72a6 in AbortCurrentTransaction () at xact.c:3449
#9  0x0000563f0979c0f7 in PostgresMain (dbname=3D0x563f0f128100
"isolation_regression", username=3D0x563f0f1280e8 "ioltas") at
postgres.c:4524
#10 0x0000563f0978a5e5 in BackendMain (startup_data=3D0x7ffdcf50cfa8 "",
startup_data_len=3D4) at backend_startup.c:107
#11 0x0000563f094d8613 in postmaster_child_launch
(child_type=3DB_BACKEND, child_slot=3D1,
startup_data=3D0x7ffdcf50cfa8 "", startup_data_len=3D4,
client_sock=3D0x7ffdcf50cfe0)=20

Isn't that pointing to an actual bug with serializable transactions?

What you are telling here is that there is a race condition where it
is possible to trigger an assertion failure when finishing a session
while another one is waiting on an invalidation, if there's in the mix
a read/write dependency error.  Disabling the test hides the problem,
it does not fix it.  And we should do the latter, not the former.
--
Michael

--+YnPvJhoyyjhkbM0
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmdJS9wACgkQnvQgOdby
QH0YwBAAgTjXuu0VBgjsIMVfH2/bFr1PQdG2c3EvWNllJxDAg+NuzugpMP6z/324
+Epas10bD5jJvPN38HorODbm6/5ip11iW/iQqXCDDhhq5Uc0dRxeHlATdc9Y8ubb
mtaoLudD5/33V/72xTDZzMZ2pXx5iUmgcJoZ01vWzsZUKCgGPAgeZxyJphpDOKZC
Z3Ds7lIbKeljRy7oUoq+epGquJNgJ9vLmaZN3rGW7BXlD3Lk0j4LxjLZMekfzxu9
JCXpywJmXijsEX1VICRsz4UiEWwoBQzM2g1yzNG9xuxVf2L8wvSByg8vWtlao+gn
EtOIj3gL8wtLul02OgJYyDc3RP1AOLKf2hCPleWV2QP2D6DGPzZO03n9S/hgBQiI
1AtmaUf7iQ+udRwKIJoo6+aHMFsVDi9wHzhTjGR8PkFfRAcakB5pHQTAmnY0v+aV
KX7tW9eKOdiV1UeJKBfKl8zEbttKrIzDsOPeFFLGXl7fOlJjpNNe1F1/Rr8SDIsu
P5/LMAaPJ6B2ouHZeP8EnZ6taSJpwejt7sUSj+t9PYvdtuRjXtWBjsMoKWDwLNlM
Wwcp4wVpN2RlDF//ZQnZL3xD6Z8BJhbjNqhkmrYYy22HNZklWKCcKPdvPZy5Etbh
Prgnq+3xXaDxE1DFs7dtJFjd0/5w1OORebPWoffOzKOZkJTdDMw=
=lVNB
-----END PGP SIGNATURE-----

--+YnPvJhoyyjhkbM0--