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
signature.asc
Description: PGP signature
