Re: refactor subscription tests to use PostgresNode's wait_for_catchup

On 1/8/18 23:47, Michael Paquier wrote:
>> @@ -1505,7 +1515,7 @@ sub wait_for_catchup
>> . $target_lsn . " on "
>> . $self->name . "\n";
>> my $query =
>> -qq[SELECT '$target_lsn' <= ${mode}_lsn FROM pg_catalog.pg_stat_replication WHERE application_name = '$standby_name';];
>> +qq[SELECT $lsn_expr <= ${mode}_lsn FROM pg_catalog.pg_stat_replication WHERE application_name = '$standby_name';];
>> $self->poll_query_until('postgres', $query)
>> or die "timed out waiting for catchup, current location is "
>> . ($self->safe_psql('postgres', $query) || '(unknown)');
>
> This log is wrong from the beginning. Here $query returns a boolean
> status and not a location. I think that when the poll dies because of a
> timeout you should do a lookup at ${mode}_lsn from pg_stat_replication
> when application_name matching $standby_name. Could you fix that as
> well?

Should we just remove it? Apparently, it was never functional to begin
with. Otherwise, we'd have to write a second query to return the value
to print. wait_for_slot_catchup has the same issue. Seems like a lot
of overhead for something that has never been used.

> Could you also update promote_standby in RewindTest.pm? Your refactoring
> to use pg_current_wal_lsn() if a target_lsn is not possible makes this
> move possible. Using the generic APIs gives better logs as well.

Right.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services