Re: Add two missing tests in 035_standby_logical_decoding.pl

On Wed, Apr 26, 2023 at 4:41 PM Drouvot, Bertrand
<bertranddrouvot.pg@gmail.com> wrote:
>
> On 4/26/23 12:27 PM, Alvaro Herrera wrote:
> >> diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
> >> index 6f7f4e5de4..819667d42a 100644
> >> --- a/src/test/perl/PostgreSQL/Test/Cluster.pm
> >> +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
> >> @@ -2644,7 +2644,16 @@ sub wait_for_catchup
> >>      }
> >>      if (!defined($target_lsn))
> >>      {
> >> -            $target_lsn = $self->lsn('write');
> >> +            my $isrecovery = $self->safe_psql('postgres', "SELECT pg_is_in_recovery()");
> >> +            chomp($isrecovery);
> >> +            if ($isrecovery eq 't')
> >> +            {
> >> +                    $target_lsn = $self->lsn('replay');
> >> +            }
> >> +            else
> >> +            {
> >> +                    $target_lsn = $self->lsn('write');
> >> +            }
> >
> > Please modify the function's documentation to account for this code change.
> >
>
> Good point, thanks! Done in V6 attached.
>

+When in recovery, the default value of target_lsn is $node->lsn('replay')
+instead. This is needed when the publisher passed to
wait_for_subscription_sync()
+is a standby node.

I think this will be useful whenever wait_for_catchup has been called
for a standby node (where self is a standby node). I have tried even
by commenting wait_for_subscription_sync in the new test then it fails
for $node_standby->wait_for_catchup('tap_sub');. So instead, how about
a comment like: "When in recovery, the default value of target_lsn is
$node->lsn('replay') instead which ensures that the cascaded standby
has caught up to what has been replayed on the standby."?

-- 
With Regards,
Amit Kapila.