Re: Trap errors from streaming child in pg_basebackup to exit early

On Mon, Feb 21, 2022 at 03:11:30PM +0100, Daniel Gustafsson wrote:
>On 21 Feb 2022, at 03:03, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> +is($node->poll_query_until('postgres',
>> + "SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE " .
>> + "application_name = '010_pg_basebackup.pl' AND wait_event =
>> 'WalSenderMain' " .
>> + "AND backend_type = 'walsender'"), "1", "Walsender killed");
>> If you do that, don't you have a risk to kill the WAL sender doing the
>> BASE_BACKUP? That could falsify the test. It seems to me that it
>> would be safer to add a check on query ~ 'START_REPLICATION' or
>> something like that.
>
> I don't think there's a risk, but I've added the check on query as well since
> it also makes it more readable.

Okay, thanks.

>> - diag("aborting wait: program timed out");
>> - diag("stream contents: >>", $$stream, "<<");
>> - diag("pattern searched for: ", $untl);
>> Keeping some of this information around would be useful for
>> debugging in the refactored routine.
>
> Maybe, but we don't really have diag output anywhere in the modules or the
> tests so I didn't see much of a precedent for keeping it. Inspectig the repo I
> think we can remove two more in pg_rewind, which I just started a thread for.

Hmm. If you think this is better this way, I won't fight hard on this
point, either.

The patch set looks fine overall.
--
Michael