RE: Conflict detection and logging in logical replication

On Tuesday, July 30, 2024 5:06 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Mon, Jul 29, 2024 at 11:44 AM Zhijie Hou (Fujitsu)
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > Here is the V7 patch set that addressed all the comments so far[1][2][3].
>
> Thanks for the patch, few comments:

Thanks for the comments !

>
> 2)
> apply_handle_delete_internal()
>
> --Do we need to check "(!edata->mtstate || edata->mtstate->operation !=
> CMD_UPDATE)" in the else part as well? Can there be a scenario where during
> update flow, it is trying to delete from a partition and comes here, but till then
> that row is deleted already and we end up raising 'delete_missing' additionally
> instead of 'update_missing'
> alone?

I think this shouldn't happen because the row to be deleted should have been
locked before entering the apply_handle_delete_internal(). Actually, calling
apply_handle_delete_internal() for cross-partition update is a big buggy because the
row to be deleted has already been found in apply_handle_tuple_routing(), so we
could have avoid scanning the tuple again. I have posted another patch to fix
this issue in thread[1].

Here is the V8 patch set. It includes the following changes:

* Addressed the comments from Shveta.
* Reported the origin name in the DETAIL instead of the origin id.
* fixed the issue Dilip pointed[2].
* fixed one old issue[3] Nisha pointed that I missed to fix in previous version.
* Improved the document a bit.

[1] https://www.postgresql.org/message-id/CAA4eK1JsNPzFE8dgFOm-Tfk_CDZyg1R3zuuQWkUnef-N-vTkoA%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAFiTN-tYdN63U%3Dd8V8rBfRtFmhZ%3DQQX7jEmj1cdWMe_NM%2B7%3DTQ%40mail.gmail.com
[3] https://www.postgresql.org/message-id/CABdArM6%2BN1Xy_%2BtK%2Bu-H%3DsCB%2B92rAUh8qH6GDsB%2B1naKzgGKzQ%40mail.gmail.com

Best Regards,
Hou zj