Horton

Re: Introduce wait_for_subscription_sync for TAP tests

Masahiko Sawada <sawada.mshk@gmail.com>

View thread View on postgr.es
From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-07-28T01:07:36Z
Lists: pgsql-hackers 
On Wed, Jul 27, 2022 at 8:54 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Jul 26, 2022 at 1:12 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Tue, Jul 26, 2022 at 2:01 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > 2.
> > > +    # wait for the replication to catchup if required.
> > > +    if (defined($publisher))
> > > +    {
> > > + croak 'subscription name must be specified' unless defined($subname);
> > > + $publisher->wait_for_catchup($subname, 'replay');
> > > +    }
> > > +
> > > +    # then, wait for all table states to be ready.
> > > +    print "Waiting for all subscriptions in \"$name\" to synchronize data\n";
> > > +    my $query = qq[SELECT count(1) = 0
> > > +     FROM pg_subscription_rel
> > > +     WHERE srsubstate NOT IN ('r', 's');];
> > > +    $self->poll_query_until($dbname, $query)
> > > + or croak "timed out waiting for subscriber to synchronize data";
> > >
> > > In the tests, I noticed that a few places did wait_for_catchup after
> > > the subscription check, and at other places, we did that check before
> > > as you have it here. Ideally, I think wait_for_catchup should be after
> > > confirming the initial sync is over as without initial sync, the
> > > publisher node won't be completely in sync with the subscriber.
> >
> > What do you mean by the last sentence? I thought the order doesn't
> > matter here. Even if we do wait_for_catchup first then the
> > subscription check, we can make sure that the apply worker caught up
> > and table synchronization has been done, no?
> >
>
> That's right. I thought we should first ensure the subscriber has
> finished operations if possible, like in this case, it can ensure
> table sync has finished and then we can ensure whether publisher and
> subscriber are in sync. That sounds more logical to me.

Make sense. I've incorporated it in the v3 patch.

Regards,

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

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