RE: Conflict detection for multiple_unique_conflicts in logical replication

From: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
To: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>
Subject: RE: Conflict detection for multiple_unique_conflicts in logical replication
Date: 2025-03-21 08:17:43
Message-ID: OS0PR01MB57169BA3CB76A79B30223E3894DB2@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Mar 21, 2025 at 12:50 PM Nisha Moond wrote:

> Thanks, Hou-san, for the review and fix patches. I’ve incorporated
> your suggestions.
> Attached are the v7 patches, including patch 002, which implements
> stats collection for 'multiple_unique_conflicts' in pg_stat_subscription_stats.

Thanks for updating the patches.

Here are some more comments:

1.

The comments atop of ReportApplyConflict() should be updated to reflect the
updates made to the input parameters.

2.

Add ConflictTupleInfo in typedefs.list.

3.

Few typos exiting => existing

4.

$node_subscriber->wait_for_log(
qr/ERROR: conflict detected on relation \"public.conf_tab\": conflict=multiple_unique_conflicts/,

We should avoid adding the elevel('ERROR') body here as the format could be
changed depending on the log_error_verbosity. Please refer to d13ff82 for
details.

Please see attachment 0001 for the proposed changes.

5.
ok( $node_subscriber->log_contains(
qr/Key already exists in unique index \"conf_tab_pkey\".*\n.*Key \(a\)=\(2\); existing local tuple \(2, 2, 2\); remote tuple \(2, 3, 4\)./,
$log_offset),
'multiple_unique_conflicts detected during insertion for conf_tab_pkey (a) = (2)'
);
...
ok( $node_subscriber->log_contains(
qr/Key already exists in unique index \"conf_tab_b_key\".*\n.*Key \(b\)=\(3\); existing local tuple \(3, 3, 3\); remote tuple \(2, 3, 4\)./,
$log_offset),
'multiple_unique_conflicts detected during insertion for conf_tab_b_key (b) = (3)'
);

Currently, different detail lines are checked in separate test cases. It would
be clearer to merge these checks, ensuring comprehensive validation
including line breaks. See attachment 0002 for the proposed changes.

6.

The document related to the conflict log format should be updated. E.g., all the
places that mentioned insert|update_exists might need to mention the new
multi-key conflict as well. And it would be better to mention there would be
multiple detail lines in the new conflict. Please see attachment 0003 for the
proposed changes.

Best Regards,
Hou zj

Attachment Content-Type Size
0001-fix-comments-and-tests.patch.txt text/plain 3.9 KB
0002-merge-tests.patch.txt text/plain 4.0 KB
0003-add-documents.patch.txt text/plain 2.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ni Ku 2025-03-21 08:48:30 Re: Changing shared_buffers without restart
Previous Message Michael Paquier 2025-03-21 07:33:41 Re: Proposal - Allow extensions to set a Plan Identifier