On Fri, Oct 11, 2024 at 2:04 PM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > > Attach the V4 patch set which addressed above comments. >
A few minor comments: 1. + * Retaining the dead tuples for this period is sufficient because any + * subsequent transaction from the publisher will have a later timestamp. + * Therefore, it is acceptable if dead tuples are removed by vacuum and an + * update_missing conflict is detected, as the correct resolution for the + * last-update-wins strategy in this case is to convert the UPDATE to an INSERT + * and apply it anyway. + * + * The 'remote_wal_pos' will be reset after sending a new request to walsender. + */ +static void +maybe_advance_nonremovable_xid(XLogRecPtr *remote_wal_pos, + DeadTupleRetainPhase *phase) We should cover the key point of retaining dead tuples which is to avoid converting updates to inserts (considering the conflict as update_missing) in the comments above and also in the commit message. 2. In maybe_advance_nonremovable_xid() all three phases are handled by different if blocks but as per my understanding the phase value will be unique in one call to the function. So, shouldn't it be handled with else if? -- With Regards, Amit Kapila.