RE: Conflict detection for update_deleted in logical replication

On Friday, October 18, 2024 5:47 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Oct 11, 2024 at 2:04 PM Zhijie Hou (Fujitsu) <houzj(dot)fnst(at)fujitsu(dot)com>
> wrote:
> >
> >
> > Attach the V4 patch set which addressed above comments.
> >
>
> A few minor comments:

Thanks for the 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.

Right, I updated the comments the commit messages for the same.

>
> 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?

I thought that it is possible that we can immediately pass the check in next phase,
thus, wanted to run the codes of next phase as well during one function call.

For example, after switching from DTR_WAIT_FOR_LOCAL_FLUSH to
DTR_REQUEST_WALSENDER_WAL_POS, it’s possible that enough time have passed since
the last wal position request, so It was intended to let it immediately send
the next request in the next if block if possible.

I added some comments in this version to make it clear.

Here is the V5 patch set which addressed above comments.

Best Regards,
Hou zj