On Wed, Jul 27, 2022 at 7:08 PM shiy.f...@fujitsu.com <shiy.f...@fujitsu.com> wrote: > > On Tue, Jul 26, 2022 3:42 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > I've attached an updated patch as well as a patch to remove duplicated > > waits in 007_ddl.pl. > > > > Thanks for your patch. Here are some comments.
Thank you for the comments! > > 1. > I think some comments need to be changed in the patch. > For example: > # Also wait for initial table sync to finish > # Wait for initial sync to finish as well > > Words like "Also" and "as well" can be removed now, we originally used them > because we wait for catchup and "also" wait for initial sync. Agreed. > > 2. > In the following places, we can remove wait_for_catchup() and then call it in > wait_for_subscription_sync(). > > 2.1. > 030_origin.pl: > @@ -128,8 +120,7 @@ $node_B->safe_psql( > > $node_C->wait_for_catchup($appname_B2); > > -$node_B->poll_query_until('postgres', $synced_query) > - or die "Timed out while waiting for subscriber to synchronize data"; > +$node_B->wait_for_subscription_sync; > > 2.2. > 031_column_list.pl: > @@ -385,7 +373,7 @@ $node_subscriber->safe_psql( > ALTER SUBSCRIPTION sub1 SET PUBLICATION pub2, pub3 > )); > > -wait_for_subscription_sync($node_subscriber); > +$node_subscriber->wait_for_subscription_sync; > > $node_publisher->wait_for_catchup('sub1'); > > 2.3. > 100_bugs.pl: > @@ -281,8 +276,7 @@ $node_subscriber->safe_psql('postgres', > $node_publisher->wait_for_catchup('tap_sub'); > > # Also wait for initial table sync to finish > -$node_subscriber->poll_query_until('postgres', $synced_query) > - or die "Timed out while waiting for subscriber to synchronize data"; > +$node_subscriber->wait_for_subscription_sync; > > is( $node_subscriber->safe_psql( > 'postgres', "SELECT * FROM tab_replidentity_index"), Agreed. I've attached updated patches that incorporated the above comments as well as the comment from Amit. BTW regarding 0001 patch to remove the duplicated wait, should we backpatch to v15? I think we can do that as it's an obvious fix and it seems to be an oversight in 8f2e2bbf145. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
v3-0001-Remove-duplicated-wait-for-subscription-synchroni.patch
Description: Binary data
v3-0002-Introduce-wait_for_subscription_sync-for-TAP-test.patch
Description: Binary data