From: | Ajin Cherian <itsajin(at)gmail(dot)com> |
---|---|
To: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Failure of subscription tests with topminnow |
Date: | 2021-08-31 12:43:45 |
Message-ID: | CAFPTHDaRKVbCuh8AktHWZ6R-0V_2cRiv1BMn+jV=fyk8oss9_g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Aug 31, 2021 at 3:47 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Tue, Aug 31, 2021 at 12:11 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Mon, Aug 30, 2021 at 5:48 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> > >
> > > On Mon, Aug 30, 2021 at 7:52 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> > >
> > > I have made the above changes on HEAD.
> > >
> >
> > Thanks, this looks mostly good to me. I'll push and backpatch this
> > tomorrow unless you or someone else thinks otherwise.
> >
> > Minor comments
> > ==============
> > 1.
> > $oldpid = $node_publisher->safe_psql('postgres',
> > - "SELECT pid FROM pg_stat_replication WHERE application_name = 'tap_sub';"
> > + "SELECT pid FROM pg_stat_replication WHERE application_name =
> > 'tap_sub' AND state = 'streaming';;"
> > );
> >
> > An extra semicolon at the end of the statement.
> >
> > 2.
> > +# restart of subscription workers. We check the state along with
> > application_name
> > +# to ensure that the walsender is (re)started.
> >
> > It is better to keep application_name in an above comment in the
> > second line as that will make this line looks a bit more consistent
> > with other comments.
> >
> > 3. In commit message, the text: "The reason was that the test was
> > assuming the walsender started before it reaches the 'streaming' state
> > and The check to test whether the subscription workers were restarting
> > after a change in the subscription was failing." seems to be
> > repeated/redundant.
> >
> > 4. Kindly submit the patches for back-branches.
>
> The patch with the above comments looks good to me. One minor
> suggestion is to change the two messages of "die" to make the
> investigation a bit easier. For example,
>
> $node_publisher->poll_query_until('postgres',
> "SELECT pid != $oldpid FROM pg_stat_replication WHERE
> application_name = 'tap_sub' AND state = 'streaming';"
> ) or die "Timed out while waiting for apply to restart after changing
> CONNECTION";
>
> and
>
> $node_publisher->poll_query_until('postgres',
> "SELECT pid != $oldpid FROM pg_stat_replication WHERE
> application_name = 'tap_sub' AND state = 'streaming';"
> ) or die "Timed out while waiting for apply to restart after changing
> PUBLICATION";
>
> Regards,
>
Thanks Masahiko-san. I have included this change and made a new patch-set.
Hi Amit,
I have included your comments as well and also attached the patches
for the back-branches.
regards,
Ajin Cherian
Fujitsu Australia
Attachment | Content-Type | Size |
---|---|---|
head-v4-0001-Fix-the-random-test-failure-in-001_rep_changes.patch | application/octet-stream | 4.3 KB |
REL-10-STABLE-v2-0001-Fix-the-random-test-failure-in-001_rep_changes.patch | application/octet-stream | 4.3 KB |
REL-11-STABLE-v2-0001-Fix-the-random-test-failure-in-001_rep_changes.patch | application/octet-stream | 4.1 KB |
REL-12-STABLE-v2-0001-Fix-the-random-test-failure-in-001_rep_changes.patch | application/octet-stream | 4.3 KB |
REL-13-STABLE-v2-0001-Fix-the-random-test-failure-in-001_rep_changes.patch | application/octet-stream | 4.3 KB |
REL-14-STABLE-v2-0001-Fix-the-random-test-failure-in-001_rep_changes.patch | application/octet-stream | 4.3 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Antonin Houska | 2021-08-31 12:46:30 | Re: POC: Cleaning up orphaned files using undo logs |
Previous Message | Bharath Rupireddy | 2021-08-31 12:21:50 | Re: pg_receivewal starting position |