Re: Introduce wait_for_subscription_sync for TAP tests

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Introduce wait_for_subscription_sync for TAP tests
Date: 2022-08-10 05:09:20
Message-ID: CAD21AoA+tLF9cQD5qhqfnUB4rvMJBvfHTCtrUup=ABDruHXbBw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Aug 5, 2022 at 10:39 AM shiy(dot)fnst(at)fujitsu(dot)com
<shiy(dot)fnst(at)fujitsu(dot)com> wrote:
>
> On Thu, Aug 4, 2022 5:49 PM shiy(dot)fnst(at)fujitsu(dot)com <shiy(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > On Thu, Aug 4, 2022 2:28 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
> > wrote:
> > >
> > > On Thu, Aug 4, 2022 at 10:37 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> > > wrote:
> > > >
> > > > On Wed, Aug 3, 2022 at 10:21 AM Amit Kapila
> > <amit(dot)kapila16(at)gmail(dot)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');
>

Thank you for the comments. I've attached updated version patches.
Please review them.

Regards,

--
Masahiko Sawada
EDB: https://www.enterprisedb.com/

Attachment Content-Type Size
REL14_v5-0001-Add-wait_for_subscription_sync-for-TAP-tests.patch application/octet-stream 24.4 KB
REL12_v5-0001-Add-wait_for_subscription_sync-for-TAP-tests.patch application/octet-stream 12.8 KB
REL11_v5-0001-Add-wait_for_subscription_sync-for-TAP-tests.patch application/octet-stream 12.2 KB
REL13_v5-0001-Add-wait_for_subscription_sync-for-TAP-tests.patch application/octet-stream 16.0 KB
REL15_v5-0001-Add-wait_for_subscription_sync-for-TAP-tests.patch application/octet-stream 40.8 KB
REL10_v5-0001-Add-wait_for_subscription_sync-for-TAP-tests.patch application/octet-stream 10.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2022-08-10 05:09:38 Blocking the use of TRIGGER privilege
Previous Message Dilip Kumar 2022-08-10 05:01:27 Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints