Re: Conflict detection and logging in logical replication

On Wed, Jul 31, 2024 at 7:40 AM Zhijie Hou (Fujitsu)
<houzj.fnst@fujitsu.com> wrote:
>
> >
> > 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].

Thanks for the details.

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

Thanks for the patch. I verified that all the bugs reported so far are
addressed. Few trivial comments:

1)
029_on_error.pl:
--I did not understand the intent of this change. The existing insert
would also have resulted in conflict (insert_exists) and we would have
identified and skipped that. Why change to UPDATE?

 $node_publisher->safe_psql(
  'postgres',
  qq[
 BEGIN;
-INSERT INTO tbl VALUES (1, NULL);
+UPDATE tbl SET i = 2;
 PREPARE TRANSACTION 'gtx';
 COMMIT PREPARED 'gtx';
 ]);


2)
logical-replication.sgml
--In doc, shall we have 'delete_differ' first and then
'delete_missing', similar to what we have for update (first
'update_differ' and then 'update_missing')

3)
logical-replication.sgml: "For instance, the origin in the above log
indicates that the existing row was modified by a local change."

--This clarification about origin was required when we had 'origin 0'
in 'DETAILS'. Now we have "locally":
"Key (c)=(1) already exists in unique index "t_pkey", which was
modified locally in transaction 740".

And thus shall we rephrase the concerned line ?

thanks
Shveta