RE: Introduce wait_for_subscription_sync for TAP tests

Yu Shi (Fujitsu) <shiy.fnst@fujitsu.com>

From: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
To: "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-08-05T01:39:49Z
Lists: pgsql-hackers
On Thu, Aug 4, 2022 5:49 PM shiy.fnst@fujitsu.com <shiy.fnst@fujitsu.com> wrote:
> 
> On Thu, Aug 4, 2022 2:28 PM Masahiko Sawada <sawada.mshk@gmail.com>
> wrote:
> >
> > On Thu, Aug 4, 2022 at 10:37 AM Amit Kapila <amit.kapila16@gmail.com>
> > wrote:
> > >
> > > On Wed, Aug 3, 2022 at 10:21 AM Amit Kapila
> <amit.kapila16@gmail.com>
> > wrote:
> > > >
> > > > Pushed this one and now I'll look at your other patch.
> > > >
> > >
> > > I have pushed the second patch as well after making minor changes in
> > > the comments. Alvaro [1] and Tom [2] suggest to back-patch this and
> > > they sound reasonable to me. Will you be able to produce back branch
> > > patches?
> >
> > Yes. I've attached patches for backbranches. The updates are
> > straightforward on v11 - v15. However, on v10, we don't use
> > wait_for_catchup() in some logical replication test cases. The commit
> > bbd3363e128dae refactored the tests to use wait_for_catchup but it's
> > not backpatched. So in the patch for v10, I didn't change the code
> > that was changed by the commit. Also, since wait_for_catchup requires
> > to specify $target_lsn, unlike the one in v11 or later, I changed
> > wait_for_subscription_sync() accordingly.
> >
> 
> Thanks for your patches.
> 
> In the patches for pg11 ~ pg14, it looks we need to add a "=pod" before the
> current change in PostgresNode.pm. Right?
> 

By the way, I notice that in 002_types.pl (on master branch), it seems the "as
well" in the following comment should be removed. Is it worth being fixed?

$node_subscriber->safe_psql('postgres',
	"CREATE SUBSCRIPTION tap_sub CONNECTION '$publisher_connstr' PUBLICATION tap_pub WITH (slot_name = tap_sub_slot)"
);

# Wait for initial sync to finish as well
$node_subscriber->wait_for_subscription_sync($node_publisher, 'tap_sub');

Regards,
Shi yu

Commits

  1. Fix recovery_prefetch with low maintenance_io_concurrency.

  2. Drop replication origin slots before tablesync worker exits.

  3. Back-Patch "Add wait_for_subscription_sync for TAP tests."

  4. Add wait_for_subscription_sync for TAP tests.

  5. Remove duplicated wait for subscription sync from 007_ddl.pl.

  6. Fix intermetent test failure in 028_row_filter.pl.

  7. Refactor subscription tests to use PostgresNode's wait_for_catchup