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 |
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 |