RE: Conflict detection for update_deleted in logical replication

From: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
To: shveta malik <shveta(dot)malik(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: RE: Conflict detection for update_deleted in logical replication
Date: 2025-06-27 02:28:10
Message-ID: OS0PR01MB571666B8A44628EEC94F2D089445A@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jun 26, 2025 at 4:28 PM shveta malik wrote:
>
> On Thu, Jun 26, 2025 at 8:31 AM Zhijie Hou (Fujitsu)
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > Thanks for the comments. All of them look good to me and
> > have been addressed in V42.
> >
>
> Thank You for the patches. Few comments.
>
> t/035_conflicts.pl:
>
> 1)
> Both the subscriptions subname_BA and subname_AB have rci enabled
> during CREATE sub itself. And later in the second test, we are trying
> to enable rci of subname_AB to test WARNING and NOTICE, but rci is
> already enabled. Shall we have one CREATE sub with rci enabled while
> another CREATE sub with default rci. And then we try to enable rci of
> the second sub later and check pg_conflict_detection slot has been
> created once we enabled rci. This way, it will cover more scenarios.

Agreed and changed as suggested. I removed the test for WARNING since the
message is the same as the NOITCE and it seems not worthwhile to disable
the subscription again to verify one message.

>
> 2)
> +$node_B->safe_psql('postgres', "UPDATE tab SET b = 3 WHERE a = 1;");
> +$node_A->safe_psql('postgres', "DELETE FROM tab WHERE a = 1;");
> +
> +$node_A->wait_for_catchup($subname_BA);
>
> Can you please help me understand why we are doing wait_for_catchup
> here? Do we want DELETE to be replicated from A to B? IMO, this step
> is not essential for our test as we have node_A->poll_query until
> xmin = $next_xid in pg_conflict_detection and that should suffice to
> ensure both DELETE and UPDATE are replicated from one to other.

I think this step belongs to a later patch to ensure the DELETE operation is
replicated to Node B, allowing us to verify the `delete_origin_differ`
conflicts detected there. So, I moved it to the later patches.

Here is the V43 patch set which includes the following changes:

0001:
* Addressed the comments above.

0002:
No change.

0003:
No change.

0004:
* Moved some tests from 0001 to here.

0005:
No change.

Best Regards,
Hou zj

Attachment Content-Type Size
v43-0005-Allow-altering-retain_conflict_info-for-enabled-.patch application/octet-stream 32.7 KB
v43-0001-Preserve-conflict-relevant-data-during-logical-r.patch application/octet-stream 172.7 KB
v43-0002-Introduce-a-new-GUC-max_conflict_retention_durat.patch application/octet-stream 31.1 KB
v43-0003-Re-create-the-replication-slot-if-the-conflict-r.patch application/octet-stream 7.0 KB
v43-0004-Support-the-conflict-detection-for-update_delete.patch application/octet-stream 30.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shinya Kato 2025-06-27 02:55:18 Re: Extend COPY FROM with HEADER <integer> to skip multiple lines
Previous Message Hayato Kuroda (Fujitsu) 2025-06-27 02:22:27 RE: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly