agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Ashutosh Sharma <ashu.coek88@gmail.com>
Cc: 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: Sun, 4 Oct 2026 05:59:59 +0000
Message-ID: <asHrX+DPqiuK8IR6@bdtpg> (raw)
In-Reply-To: <CAE9k0Pn1THJ0Hw6bZKSrZUnvxSrh19=PAu85Ka=99LzGki750g@mail.gmail.com>
References: <arOP6DEhgKcsNS5b@bdtpg>
	<CAJpy0uAiqT-__cV-MWNCQTXpAjcVDNh53+L2-DzZnYH8O5K1Vg@mail.gmail.com>
	<arP4wN5HW8h0Sr90@bdtpg>
	<CAJpy0uCeW9Kf1Ofgn=ojiG-FEpsf=KVgpF-289A_j9xX0G5AyA@mail.gmail.com>
	<CAJpy0uBDGfq8sermY3CE9M_LeBx+fAWsiKQLSZkpGeUOzEgcwg@mail.gmail.com>
	<arTwLinbxvVkEqu2@bdtpg>
	<CAJpy0uDASVicCkn-BJtecgpS081UXf4kG7UPicCB=cMzVJveMA@mail.gmail.com>
	<arYModKZ2uLwUyES@bdtpg>
	<arqLwLn0w9DdfSZG@bdtpg>
	<CAE9k0Pn1THJ0Hw6bZKSrZUnvxSrh19=PAu85Ka=99LzGki750g@mail.gmail.com>

Hi,

On Wed, Sep 30, 2026 at 03:50:30PM +0530, Ashutosh Sharma wrote:
> Hi,
> 
> On Mon, Sep 28, 2026 at 9:16 PM Bertrand Drouvot
> <bertranddrouvot.pg@gmail.com> wrote:
> >
> > The patch needed a rebase, so at the same time I went ahead with the proposed
> > changes above (plus the one in the commit message suggested by Rui in [1]).
> >
> 
> Thanks for reporting the problem and providing a patch for it. The
> approach looks good to me, but I have a few comments to share:

Thanks for looking at it!

> 
> I think it would be good to add a comment above
> InvalidatePossiblyObsoleteSlot() explaining the reason for this change
> in the usual slot update pattern.

What about something like?

"
Unlike normal replication slot updates, persist the invalidation before
publishing it in shared memory. Publishing it first could allow resource
horizon computations to remove resources required by the slot before the
invalidation reaches disk. If saving then failed, a restart could restore
the old valid slot. Keeping the shared slot valid until the invalidated
image is durable avoids that state.
"

> 
> 2)
> 
> + {
> + SaveSlotToPath(slot, path, ERROR, cause, clear_restart_lsn);
> + }
> 
> Do we need to pass clear_restart_lsn here? Cause can be used to
> determine the restart lsn value later, no?

I don't think so. InvalidatePossiblyObsoleteSlot() clears restart_lsn for
RS_INVAL_WAL_REMOVED, while slotsync must preserve the local restart_lsn
when copying the same invalidation cause. So the cause alone is not enough to
determine the desired behavior.

> 3)
> 
> The current patch changes the interface for SaveSlotToPath() (), isn't
> it possible to keep it unchanged, that would probably also reduce some
> amount of complexity.

Yeah, that seems worthwhile. I'll change it that way.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com






view thread (42+ messages)  latest in thread

Message-ID: <asHrX+DPqiuK8IR6@bdtpg>
Permalink:  ../asHrX+DPqiuK8IR6@bdtpg/
Also on:    postgresql.org/message-id/asHrX+DPqiuK8IR6@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, ashu.coek88@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: <asHrX+DPqiuK8IR6@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