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.fnst@fujitsu.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
