On Tue, Dec 30, 2025 at 5:31 AM Masahiko Sawada <[email protected]> wrote: > > On Sun, Dec 14, 2025 at 8:21 PM Amit Kapila <[email protected]> wrote: > > > > On Thu, Dec 11, 2025 at 12:39 PM Zhijie Hou (Fujitsu) > > <[email protected]> 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); >
No, we don't need the allocation lock here because RedoRecPtr won't change after the previous computation so the WAL reservation during creation of slots won't be impacted. I mean if the slot reservation starts just before this computation, it should use the latest (this checkpoint cycle's RedoRecPtr) whereas that was not true with the case where the patch acquires it. > I think we need to add some comments regardless of taking the lwlock. > I have added a comment though not sure if it is required. Do you have something specific in mind? -- With Regards, Amit Kapila.
v11_PG17-0001-Prevent-invalidation-of-newly-created-replicatio.patch
Description: Binary data
