RE: Conflict detection for update_deleted in logical replication

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

From: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>

To: Amit Kapila <amit.kapila16@gmail.com>

Cc: shveta malik <shveta.malik@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, vignesh C <vignesh21@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>

Date: 2025-07-31T10:18:51Z

Lists: pgsql-hackers

Attachments

v55-0004-Re-create-the-replication-slot-if-the-conflict-r.patch (application/octet-stream) patch v55-0004
v55-0001-Support-the-conflict-detection-for-update_delete.patch (application/octet-stream) patch v55-0001
v55-0002-Collect-statistics-for-update_deleted-conflicts.patch (application/octet-stream) patch v55-0002
v55-0003-Introduce-a-new-GUC-max_conflict_retention_durat.patch (application/octet-stream) patch v55-0003

On Thursday, July 31, 2025 5:29 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Tue, Jul 29, 2025 at 10:51 AM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
> wrote:
> >
> > This is the V54 patch set, with only patch 0001 updated to address the
> > latest comments.
> >
> 
> Few minor comments:

Thanks for the comments.

> 1.
> /* The row to be updated was deleted by a different origin */
> CT_UPDATE_DELETED,
> /* The row to be updated was modified by a different origin */
> CT_UPDATE_ORIGIN_DIFFERS,
> /* The updated row value violates unique constraint */ CT_UPDATE_EXISTS,
> /* The row to be updated is missing */
> CT_UPDATE_MISSING,
> 
> Is there a reason to keep CT_UPDATE_DELETED before
> CT_UPDATE_ORIGIN_DIFFERS? I mean why not keep it just before
> CT_UPDATE_MISSING on the grounds that they are always handled together?

I agree that it makes more sense to put it before update_missing, and changed it.

> 
> 2. Will it be better to name FindRecentlyDeletedTupleInfoByIndex as
> RelationFindDeletedTupleInfoByIndex to make it similar to existing function
> RelationFindReplTupleByIndex? If you agree then make a similar change for
> FindRecentlyDeletedTupleInfoSeq as well.

Yes, the suggested name looks better.

> 
> Apart from above, please find a number of comment edits and other cosmetic
> changes in the attached.

Thanks, I have addressed above comments and merge the patch into 0001.

Here is V55 patch set.

Best Regards,
Hou zj

Commits

Fix intermittent BF failures in 035_conflicts.
- 0f42206531b3 19 (unreleased) landed
Resume conflict-relevant data retention automatically.
- 0d48d393d465 19 (unreleased) landed
Fix intermittent test failure introduced in 6456c6e2c4.
- 01d793698f59 19 (unreleased) landed
Fix Coverity issue reported in commit a850be2fe.
- 5ac3c1ac22cb 19 (unreleased) landed
Add test to prevent premature removal of conflict-relevant data.
- 6456c6e2c4ad 19 (unreleased) landed
Post-commit review fixes for 228c370868.
- 1f7e9ba3ac4e 19 (unreleased) landed
Add max_retention_duration option to subscriptions.
- a850be2fe653 19 (unreleased) landed
Detect and report update_deleted conflicts.
- fd5a1a0c3e56 19 (unreleased) landed
Preserve conflict-relevant data during logical replication.
- 228c37086855 19 (unreleased) landed
Integrate FullTransactionIds deeper into two-phase code
- 62a17a92833d 19 (unreleased) cited
Improve checks for GUC recovery_target_timeline
- fd7d7b719137 19 (unreleased) cited
Prevent excessive delays before launching new logrep workers.
- fd519419c948 18.0 cited
Keep WAL segments by slot's last saved restart LSN
- ca307d5cec90 18.0 cited
Rework some code handling pg_subscription data in psql and pg_dump
- 08691ea958c2 18.0 cited
Use generateClonedIndexStmt to propagate CREATE INDEX to partitions.
- 68dfecbef210 18.0 cited