agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Michael Paquier <michael@paquier.xyz>
To: Zsolt Parragi <zsolt.parragi@percona.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: injection_points: canceled or terminated waiters leak their wait slots
Date: Wed, 22 Jul 2026 10:29:23 +0900
Message-ID: <amAc853flInQAEI5@paquier.xyz> (raw)
In-Reply-To: <CAN4CZFO+KF=cc0-iEg28RhqRBp_fTs6D4b8b7D7DB-pGYP3Ccg@mail.gmail.com>
References: <CAN4CZFO+KF=cc0-iEg28RhqRBp_fTs6D4b8b7D7DB-pGYP3Ccg@mail.gmail.com>
On Tue, Jul 21, 2026 at 09:28:22AM +0100, Zsolt Parragi wrote:
> While stress-testing REPACK CONCURRENTLY on 19beta2 I saw a logical
> decoding activation race, and I extended 051_effective_wal_level.pl
> with a test that waits on an injection point and wakes it up later.
> 051 cancels two injection point waiters earlier in the script, and
> injection_wait() never cleans up after a canceled waiter, the wakeup
> never arrived and the test deadlocked.
>
> I think we are missing an ENSURE_ERROR_CLEANUP block there. See
> attached patch with a testcase reproducing the issue.
Hmm. I think that I'd rather use a PG_TRY/PG_FINALLY and avoid the
refactoring with the extra routine required, keeping the cleanup
action local to injection_wait(). That's also because the cleanup
action is the same for both the "normal" exit path and the interrupt
path.
> I also attached a separate version for pg19, as master has a
> refactored version of injection_wait. All previous branches have the
> 19 version, it should be easy to backport to other branches.
Thanks for that. I'm always OK to deal with a backpatch as required.
Posting versions saves some time, of course, just don't feel obliged
if you feel that this is extra work on your side.
On an unpatched code, the test would hang due to the fact that we are
doing a wait but we should not because the slot was not cleaned up.
It means that a failure mode equals to a timeout. Why not, we have
other tests of this class. Another thought: the addition of a SQL
function that provides the list of waiters that we reuse here. I
don't really see why this is worth the cost compared to your test, but
opinions of others are welcome, of course.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (832B, ../amAc853flInQAEI5@paquier.xyz/2-signature.asc)
download
view thread (19+ messages) latest in thread
Message-ID: <amAc853flInQAEI5@paquier.xyz>
Permalink: ../amAc853flInQAEI5@paquier.xyz/
Also on: postgresql.org/message-id/amAc853flInQAEI5@paquier.xyz
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: pgsql-hackers@postgresql.org
Cc: michael@paquier.xyz, zsolt.parragi@percona.com, pgsql-hackers@lists.postgresql.org
Subject: Re: injection_points: canceled or terminated waiters leak their wait slots
In-Reply-To: <amAc853flInQAEI5@paquier.xyz>
* 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