| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Vitaly Davydov <v(dot)davydov(at)postgrespro(dot)ru>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "suyu(dot)cmj" <mengjuan(dot)cmj(at)alibaba-inc(dot)com>, tomas <tomas(at)vondra(dot)me>, michael <michael(at)paquier(dot)xyz>, "bharath(dot)rupireddyforpostgres" <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
| Subject: | Re: Newly created replication slot may be invalidated by checkpoint |
| Date: | 2025-12-30 00:01:18 |
| Message-ID: | CAD21AoC0PZPzAM8notki09kjkMSteVtYEOx6_Ab1QfbfVHDP2w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Dec 14, 2025 at 8:21 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Dec 11, 2025 at 12:39 PM Zhijie Hou (Fujitsu)
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > >
> > > The other idea to fix this problem is suggested by Alexander in his
> > > email [1] which is to introduce a new ReplicationSlotReserveWALLock
> > > for this purpose. I think introducing LWLock in back branches could be
> > > questionable. Did you evaluate the pros and cons of using that
> > > approach?
> >
> > I reviewed that approach, and I think the main distinction lies in whether to
> > use a new LWLock to serialize the process or rely on an existing lock.
> > Introducing a new LWLock in back branches would alter the size of
> > MainLWLockArray and affect NUM_INDIVIDUAL_LWLOCKS/LWTRANCHE_FIRST_USER_DEFINED.
> > Although this may not directly impact user applications since users typically
> > use standard APIs like RequestNamedLWLockTranche and LWLockNewTrancheId to add
> > private LWLocks, it still has a slight risk. Additionally, using an existing
> > lock could keep code similarity with the HEAD, which can be helpful for future
> > bug fixes and analysis.
> >
>
> Fair enough. I'll wait for Sawada-san/Vitaly to see what their opinion
> on this matter is.
While it's hacky that the proposed approach takes
ReplicationSlotAllocationLock before
XLogGetReplicationSlotMinimumLSN() during checkpoint, I find that it's
better than introducing a new LWLock in minor versions which could
lead unexpected compatibility issues.
Regarding the v10 patch, do we need to take
ReplicationSlotAllocationLock also at the following place?
/*
* Recalculate the current minimum LSN to be used in the WAL segment
* cleanup. Then, we must synchronize the replication slots again in
* order to make this LSN safe to use.
*/
slotsMinReqLSN = XLogGetReplicationSlotMinimumLSN();
CheckPointReplicationSlots(shutdown);
I think we need to add some comments regardless of taking the lwlock.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Henson Choi | 2025-12-30 00:39:17 | Re: RFC: PostgreSQL Storage I/O Transformation Hooks |
| Previous Message | Chao Li | 2025-12-30 00:00:25 | Fix duplicated word in lsyscache.c comment |