On Thu, Jul 31, 2025 at 3:49 PM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > On Thursday, July 31, 2025 5:26 PM shveta malik <shveta.ma...@gmail.com> > wrote: > > > > On Tue, Jul 29, 2025 at 10:51 AM Zhijie Hou (Fujitsu) > > <houzj.f...@fujitsu.com> > > wrote: > > > > > > > > > This is the V54 patch set, with only patch 0001 updated to address the > > > latest comments. > > > > > > > Thanks for the patch. > > > > While performing tests on the latest patch, I found an assert in tablesync > > worker in FindDeletedTupleInLocalRel (see > > Assert(TransactionIdIsValid(oldestxmin))). Logs at [1]. > > > > It seems table sync worker is trying to apply changes and going to > > update-deleted conflict detection patch but is not able to find > > MyLogicalRepWorker->oldest_nonremovable_xid as this xid is set only > > for apply-worker. > > Thanks for reporting. I have fixed it by referring to conflict detection > slot's > xmin instead of the leader worker's oldest_nonremovable_xid. This should > be safe because the slot.xmin is always valid. >
Thanks for fixing. In the same context, the comment below still mentions oldest_nonremovable_xid, it can be corrected. + /* + * No need to check for a frozen transaction ID, as + * TransactionIdPrecedes() manages it internally, treating it as falling + * behind the oldest_nonremovable_xid. + */ ~ Also we may mention 'concurrently deleted' in the comment below as that makes more sense. /* The row to be updated was deleted by a different origin */ CT_UPDATE_DELETED, ~ Apart from these trivial comments changes, patch001 and patch002 look good to me. thanks Shveta
