Re: Conflict detection and logging in logical replication

Michail Nikolaev <michail.nikolaev@gmail.com>

From: Michail Nikolaev <michail.nikolaev@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, shveta malik <shveta.malik@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Dilip Kumar <dilipbalaut@gmail.com>, Jan Wieck <jan@wi3ck.info>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: 2024-08-09T11:45:12Z
Lists: pgsql-hackers
Hello, everyone.

There are some comments on this patch related to issue [0].
In short: any DirtySnapshot index scan may fail to find an existing tuple
in the case of a concurrent update.

- FindConflictTuple may return false negative result in the case of
concurrent update because ExecCheckIndexConstraints uses SnapshotDirty.
- As a result, CheckAndReportConflict may fail to report the conflict.
- In apply_handle_update_internal we may get an CT_UPDATE_MISSING instead
of CT_UPDATE_DIFFER
- In apply_handle_update_internal we may get an CT_DELETE_MISSING instead
of CT_DELETE_DIFFER
- In apply_handle_tuple_routing we may get an CT_UPDATE_MISSING instead of
CT_UPDATE_DIFFER

If you're interested, I could create a test to reproduce the issue within
the context of logical replication. Issue [0] itself includes a test case
to replicate the problem.

It also seems possible that a conflict could be resolved by a concurrent
update before the call to CheckAndReportConflict, which means there's no
guarantee that the conflict will be reported correctly.
Should we be concerned about this?

[0]: https://commitfest.postgresql.org/49/5151/

Commits

  1. Rename the conflict types for the origin differ cases.

  2. Doc: explain the log format of logical replication conflicts.

  3. Log the conflicts while applying changes in logical replication.

  4. Avoid duplicate table scans for cross-partition updates during logical replication.