On Thu, Jul 3, 2025 at 10:43 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Jul 3, 2025 at 10:26 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > On Wed, Jul 2, 2025 at 12:58 PM Zhijie Hou (Fujitsu) > > <houzj.f...@fujitsu.com> wrote: > > > > > > > > During local testing, I discovered a bug caused by my oversight in > > > assigning > > > the new xmin to slot.effective, which resulted in dead tuples remaining > > > non-removable until restart. I apologize for the error and have provided > > > corrected patches. Kindly use the latest patch set for performance > > > testing. > > > > You changes related to write barrier LGTM, however I have question > > regarding below change, IIUC, in logical replication > > MyReplicationSlot->effective_xmin should be the xmin value which has > > been flushed to the disk, but here we are just setting "data.xmin = > > new_xmin;" and marking the slot dirty so I believe its not been yet > > flushed to the disk right? > > > > Yes, because this is a physical slot and we need to follow > PhysicalConfirmReceivedLocation()/PhysicalReplicationSlotNewXmin(). > The patch has kept a comment in advance_conflict_slot_xmin() as to why > it is okay not to flush the slot immediately.
Oh right, I forgot its physical slot. I think we are good, thanks. -- Regards, Dilip Kumar Google