On Mon, 19 Jan 2026 at 10:57, Dilip Kumar <[email protected]> wrote:
>
> On Mon, Jan 19, 2026 at 9:42 AM Peter Smith <[email protected]> wrote:
> >
> > Some review comments for v22-0003.
> >
> > ======
> >
> > 1.
> > It looks like none of my previous v20-0003 review comments [1] have
> > been addressed. Maybe accidentally overlooked?
> >
> > ======
> >
> > 2.
> > + <caution>
> > + <para>
> > + The internal conflict logging table is strictly tied to
> > the lifecycle of the
> > + subscription or the
> > <literal>conflict_log_destination</literal> setting. If
> > + the subscription is dropped, or if the destination is changed to
> > + <literal>log</literal>, the table and all its recorded
> > conflict data are
> > + <emphasis>permanently deleted</emphasis>. To perform a
> > post-mortem analysis
> > + after removing a subscription, users must manually back up
> > or rename the
> > + conflict table before the deletion occurs.
> > + </para>
> > + </caution>
> >
> > 2a.
> > Let's consistently call this the "Conflict log table", same as
> > everywhere else, not "logging table".
> >
> > ~
> >
> > 2b.
> > This is only a caution for the CLT, so I felt it's better to put this
> > in the scope of the 'table' param value.
> >
> > ~~~
> >
> > 3.
> > + analysis of conflicts. This table is automatically
> > dropped when the
> > + subscription is removed.
> >
> > If you move the <caution> to this scope, as suggested above in #2b,
> > then you can remove the sentence "This table is automatically dropped
> > when the subscription is removed", because that is duplicate
> > information you already wrote in the caution.
>
> The attached patch fixes above comments and other comments reported in
> v22-0001 and v22-0002
The tests are failing randomly at the following places
+# Wait for the conflict to be logged in the CLT
+my $log_check = $node_subscriber->poll_query_until(
+ 'postgres',
+ "SELECT count(*) > 0 FROM $clt;"
+);
+
+my $conflict_check = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM $clt WHERE conflict_type =
'multiple_unique_conflicts';");
+is($conflict_check, 1, 'Verified multiple_unique_conflicts logged
into conflict log table');
+# Wait for the conflict to be logged in the CLT
+$log_check = $node_subscriber->poll_query_until(
+ 'postgres',
+ "SELECT count(*) > 0 FROM $clt;"
+);
+
+$conflict_check = $node_subscriber->safe_psql('postgres',
+ "SELECT count(*) FROM $clt WHERE conflict_type =
'multiple_unique_conflicts';");
+is($conflict_check, 1, 'Verified multiple_unique_conflicts logged
into conflict log table');
In both places it fails because the number of conflict records
inserted can be more than 1 like below:
[18:35:58.342](1.786s) not ok 1 - Verified multiple_unique_conflicts
logged into conflict log table
[18:35:58.346](0.004s)
[18:35:58.347](0.002s) # Failed test 'Verified
multiple_unique_conflicts logged into conflict log table'
# at t/035_conflicts.pl line 104.
[18:35:58.348](0.000s) # got: '2'
# expected: '1'
How about we check that the record count >= 1 and check for 't'.
Regards,
Vignesh
