| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> | 
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> | 
| Cc: | shveta malik <shveta(dot)malik(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> | 
| Subject: | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart | 
| Date: | 2025-10-29 18:08:23 | 
| Message-ID: | CAD21AoAQX6_=FPeGaKkhFvS6hGkFkPs9T0HHQfH0Tvk4NMEKuQ@mail.gmail.com | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
On Tue, Oct 28, 2025 at 1:55 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Oct 28, 2025 at 8:59 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Mon, Oct 27, 2025 at 11:56 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > >
> > > On Mon, Oct 27, 2025 at 8:08 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> > > >
> > > > On Mon, Oct 27, 2025 at 6:12 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > > > >
> > > > >
> > > > > Thanks for looking into it. I didn't get a chance to review the entire
> > > > > 0002 but I looked at InvalidateObsoleteReplicationSlots() and have a
> > > > > few questions related to that.
> > > > >
> > > > > In InvalidateObsoleteReplicationSlots(), the patch increments
> > > > > n_valid_logicalslots before trying to invalidate the slot. Say, if
> > > > > there is just one logical slot which got invalidated, then because we
> > > > > have first incremented n_valid_logicalslots, how will it request to
> > > > > disable logical_decoding after invalidating the last logical slot?
> > > > >
> > > >
> > > > My initial understand on this:
> > > >
> > > > Whenever it invalidates a slot, the code jumps to the restart label,
> > > > which in turn sets n_valid_logicalslots to 0. If it does not
> > > > invalidate the slot but a logical slot exists, then
> > > > n_valid_logicalslots remains greater than 0. Therefore, by the end of
> > > > the function:
> > > >
> > > > --If valid logical slots were found and all were invalidated,
> > > > n_valid_logicalslots must be 0.
> > > > --If a valid logical slot was found but was not invalidated,
> > > > n_valid_logicalslots must be greater than 0.
> > > >
> > > > But on looking again, I found that the code jumps to restart-label if
> > > > lock-was released in interim. So can it happen that 'invalidated' is
> > > > true but lock was not released by InvalidatePossiblyObsoleteSlot()
> > > > causing n_valid_logicalslots to be greater than 0 even when the slot
> > > > was actually invalidated?
> > >
> > > IIUC it cannot happen. When we invalidate one logical slot we release
> > > the lwlock to avoid holding a lwlock while ReplicationSlotSave(). It's
> > > not necessarily true that if we release the lwlock we invalidate the
> > > slot but the opposite is always true.
> > >
> >
> > Okay. Thanks for confirming. After looking more into the flow, I agree on this.
> >
>
> But, is it a good idea to rely on such a flag? I feel there should be
> some other bullet-proof way to detect whether slot is invalidated so
> that even if this internal API changes in future, we won't go wrong.
> In any case, if we still want to go the same way at least a comment
> explaining this rationale would be required.
Good point. I'll refactor these functions to make it more bullet-proof.
Regards,
-- 
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Robert Haas | 2025-10-29 18:29:45 | Re: apply_scanjoin_target_to_paths and partitionwise join | 
| Previous Message | Masahiko Sawada | 2025-10-29 18:07:03 | Re: POC: enable logical decoding when wal_level = 'replica' without a server restart |