From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: shveta malik <shveta.malik@gmail.com>
Cc: Ashutosh Sharma <ashu.coek88@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: Tue, 6 Oct 2026 07:33:11 +0000
Message-ID: <asSkN5GOEOKQZs9w@bdtpg> (raw)
In-Reply-To: <CAJpy0uDw2b+Ykx73E6Xk6Cgo1qL_HOkoOTEv0OU5VsHXkJm2WQ@mail.gmail.com>
References: <CAJpy0uBDGfq8sermY3CE9M_LeBx+fAWsiKQLSZkpGeUOzEgcwg@mail.gmail.com>
<arTwLinbxvVkEqu2@bdtpg>
<CAJpy0uDASVicCkn-BJtecgpS081UXf4kG7UPicCB=cMzVJveMA@mail.gmail.com>
<arYModKZ2uLwUyES@bdtpg>
<arqLwLn0w9DdfSZG@bdtpg>
<CAE9k0Pn1THJ0Hw6bZKSrZUnvxSrh19=PAu85Ka=99LzGki750g@mail.gmail.com>
<asHrX+DPqiuK8IR6@bdtpg>
<CAE9k0P=aea5vARHSEuDh5MRkUheqFHc_MV7CFgv96Ai8-Xuu3A@mail.gmail.com>
<asOlG7/R4X5v7KlL@bdtpg>
<CAJpy0uDw2b+Ykx73E6Xk6Cgo1qL_HOkoOTEv0OU5VsHXkJm2WQ@mail.gmail.com>
Hi,
On Tue, Oct 06, 2026 at 10:25:35AM +0530, shveta malik wrote:
> On Mon, Oct 5, 2026 at 6:54 PM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > Hi,
> >
> > On Mon, Oct 05, 2026 at 03:22:53PM +0530, Ashutosh Sharma wrote:
> > > Thanks. I'll review again once the updated patch is posted.
> >
> > Thanks! Here it is.
> >
>
> Since we have now modularized this further by introducing
> SaveSlotToPathInternal() and SaveInvalidatedSlotToPath(), can we make
> SaveSlotToPathInternal() consistent across both flows with respect to
> lock acquisition and release?
>
> We could have SaveSlotToPath() acquire and release the lock,
> preferably within a PG_TRY/PG_CATCH block. Additionally, the
> 'was_dirty' check can also be moved up into SaveSlotToPath(), since
> SaveInvalidatedSlotToPath() always forces a write and doesn't need the
> check.
>
> This way, SaveSlotToPath() would acquire the lock only when the slot
> is dirty, and SaveSlotToPathInternal() would not need to handle lock
> acquisition/release based on the 'cause' argument. This would also let
> us remove the multiple if blocks that currently check the 'cause' and
> release the lock. Thoughts?
Thanks for the proposal!
I looked at it, but I think that it makes ordinary save errors get reported while
holding io_in_progress_lock. In particular, the LOG path would perform logging
and check for interrupts before releasing the lock.
As this is meant to be backpatched, I'm not sure it's worth changing the existing
behavior as part of this fix. The proposed cleanup could be considered separately
on HEAD as a follow up patch though.
What do you think?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
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, ashu.coek88@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: <asSkN5GOEOKQZs9w@bdtpg>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox