public inbox for [email protected]  
help / color / mirror / Atom feed
From: Michael Paquier <[email protected]>
To: Michail Nikolaev <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: [email protected]
Cc: Heikki Linnakangas <[email protected]>
Subject: Re: Strange assertion in procarray.c
Date: Fri, 29 Nov 2024 14:06:36 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <CANtu0oiBAcVnzYYETbWY+2gFXUeAx8BKArjnFco4LeAHfH38Sw@mail.gmail.com>
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>

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 effects.
> Therefore, I propose extending the installcheck disable to cover both TAP
> and isolation tests as well.
> 
> I've attached a patch implementing these changes.

@@ -426,6 +427,7 @@ InvalidateCatalogSnapshot(void)
         pairingheap_remove(&RegisteredSnapshots, &CatalogSnapshot->ph_node);
         CatalogSnapshot = 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=0x563f0a072d00 
"TransactionIdIsValid(proc->xid)", fileName=0x563f0a072a4a
"procarray.c", lineNumber=677)     at assert.c:66
#5  0x0000563f096f0684 in ProcArrayEndTransaction
(proc=0x7fbd4d083ac0, latestXid=750) 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=0x563f0f128100
"isolation_regression", username=0x563f0f1280e8 "ioltas") at
postgres.c:4524
#10 0x0000563f0978a5e5 in BackendMain (startup_data=0x7ffdcf50cfa8 "",
startup_data_len=4) at backend_startup.c:107
#11 0x0000563f094d8613 in postmaster_child_launch
(child_type=B_BACKEND, child_slot=1,
startup_data=0x7ffdcf50cfa8 "", startup_data_len=4,
client_sock=0x7ffdcf50cfe0) 

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


Attachments:

  [application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
  download

view thread (6+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Strange assertion in procarray.c
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox