On Mon, Aug 30, 2021 at 5:48 PM Ajin Cherian <itsajin@gmail.com> wrote:
>
> On Mon, Aug 30, 2021 at 7:52 PM Amit Kapila <amit.kapila16@gmail.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.
--
With Regards,
Amit Kapila.
