agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: shveta malik <shveta.malik@gmail.com>
Cc: JoongHyuk Shin <sjh910805@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Rui Zhao <zhaorui126@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Persist slot invalidations before publishing them
Date: Wed, 23 Sep 2026 06:06:58 +0000
Message-ID: <arNsgodAHkkxbxvX@bdtpg> (raw)
In-Reply-To: <CAJpy0uDAyMbD61iJE-2qRMCtdQ4Qtuq+hy=mqDszFapgCHXaJQ@mail.gmail.com>
References: <ao7u5I9OeIR72kGp@bdtpg>
<CAA4eK1LH_yjAwcn87gxdawJn+VsAt5HSjXdfAA7gBB-Gb9BGaA@mail.gmail.com>
<apGN1gY3ATFuxd0C@bdtpg>
<CACSdjfOa0c1dqXZQhCBD+72tq6sOccfk1qs3SkYz+8Hgn=My5g@mail.gmail.com>
<arI8AEPtq4EtgzWL@bdtpg>
<CAJpy0uDAyMbD61iJE-2qRMCtdQ4Qtuq+hy=mqDszFapgCHXaJQ@mail.gmail.com>
Hi,
On Tue, Sep 22, 2026 at 03:46:55PM +0530, shveta malik wrote:
> I had a look at 002 to review slotsync path,
Thanks for looking at it!
> + /*
> + * A failed invalidation can still hold the slot's I/O lock. Release it
> + * before slot cleanup acquires ReplicationSlotAllocationLock, which
> + * checkpoints hold while acquiring slot I/O locks.
> + */
> + LWLockReleaseAll();
> +
>
> Could it be problematic to call LWLockReleaseAll() inside a localized
> error cleanup callback (PG_ENSURE_ERROR_CLEANUP) rather than waiting
> for AbortTransaction or proc_exit? Since the goal is just to avoid
> deadlock with the Checkpointer, shouldn't we explicitly release that
> one specific lock?
> if (MyReplicationSlot != NULL &&
> LWLockHeldByMe(&MyReplicationSlot->io_in_progress_lock))
> {
> LWLockRelease(&MyReplicationSlot->io_in_progress_lock);
> }
>
> I don't have an exact scenario to worry about, but it seems like
> overkill. Thoughts?
Yeah, it's probably better to be specific here.
One concern with the proposed check is that all existing uses of LWLockHeldByMe()
appear to be for assertions or debugging (as documented on top of LWLockHeldByMe()).
Also, releasing an LWLock after ERROR requires restoring the interrupt holdoff
expected by LWLockRelease().
Another possibility would be to make ReplicationSlotPersistInvalidation() always
leave the caller acquired I/O lock held. Slotsync could then release that specific
lock in a PG_CATCH() block, something like:
"
PG_CATCH();
{
HOLD_INTERRUPTS();
LWLockRelease(&slot->io_in_progress_lock);
PG_RE_THROW();
}
PG_END_TRY();
LWLockRelease(&slot->io_in_progress_lock);
"
This would avoid both LWLockReleaseAll() and using LWLockHeldByMe() for normal
control flow. Does that sound preferable?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
view thread (32+ messages) latest in thread
Message-ID: <arNsgodAHkkxbxvX@bdtpg>
Permalink: ../arNsgodAHkkxbxvX@bdtpg/
Also on: postgresql.org/message-id/arNsgodAHkkxbxvX@bdtpg
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: bertranddrouvot.pg@gmail.com, shveta.malik@gmail.com, sjh910805@gmail.com, amit.kapila16@gmail.com, zhaorui126@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Persist slot invalidations before publishing them
In-Reply-To: <arNsgodAHkkxbxvX@bdtpg>
* 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