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: Tue, 29 Sep 2026 08:29:49 +0000
Message-ID: <art2/XiyZ0ZLG/da@bdtpg> (raw)
In-Reply-To: <CAJpy0uADOggVZJ3Z5E8C1K8D21i6PLgGB+=W+SZ9-5rn+GbYVg@mail.gmail.com>
References: <CAJpy0uBQgPf3R04RZp5BxfgABeDOyZdwm44hSOWA4JJraSo-_A@mail.gmail.com>
<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>
<CAJpy0uADOggVZJ3Z5E8C1K8D21i6PLgGB+=W+SZ9-5rn+GbYVg@mail.gmail.com>
Hi,
On Tue, Sep 29, 2026 at 11:07:34AM +0530, shveta malik wrote:
> >
> Bertrand, I will come to this Assert soon. First I would like to
> think/discuss if we can get rid of passing the 'update_inactive_since'
> boolean altogether. Currently, we need it mainly for two reasons:
>
> a) ReplicationSlotRelease() does not know when it should update
> inactive_since and when it should skip it.
> b) The slot-skip and other invalidation flows currently behave differently.
>
> We could eliminate the second difference by making the logic same for
> both the flows. I don't think there is any harm in skipping the
> 'inactive_since' update for the slot-sync's
> slot-invalidation-persist's error case as well. We never use
> 'inactive_since' to invalidate idle synced slots (see
> CanInvalidateIdleSlot()). And 'inactive_since' only matters for
> synced slots after standby promotion, when it is reset for all synced
> slots by update_synced_slots_inactive_since() from ShutDownSlotSync()
> (promotion's flow). So I don't think we need to maintain separate
> logic for this rare error case. If really needed in the future, we
> could still preserve the current behavior IsSyncingReplicationSlots()
> check in ReplicationSlotRelease(), but I don't think it is worth the
> extra complexity.
Yeah, that makes sense. This is a rare error path, so I agree that it is not
worth the extra complexity.
> That leaves us with just handling the failed-invalidation case where
> ReplicationSlotRelease() need to avoid update of inactive_since. How
> about using a static flag for this? We can set it in the CATCH block
> of ReplicationSlotPersistInvalidation() before calling
> ReplicationSlotRelease().
>
> With this approach, both flows can use ReplicationSlotRelease() in the
> same way and we don't need to split the logic into
> ReplicationSlotReleaseInternal() either. I have attached a sample
> patch. Please let me know your thoughts.
The static flag looks safe, but I wonder if it wouldn't be clearer to keep
ReplicationSlotReleaseInternal() and call it with false from the error path?
That would still allow us to remove update_inactive_since from
ReplicationSlotPersistInvalidation() and its callers, while keeping the
exceptional release behavior explicit.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
view thread (38+ messages) latest in thread
Message-ID: <art2/XiyZ0ZLG/da@bdtpg>
Permalink: ../art2%2FXiyZ0ZLG%2Fda@bdtpg/
Also on: postgresql.org/message-id/art2/XiyZ0ZLG/da@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: <art2/XiyZ0ZLG/da@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