Re: Create replication slot in pg_basebackup if requested and not yet present

Hi,

Am Dienstag, den 21.03.2017, 11:03 +0900 schrieb Michael Paquier:
> On Tue, Mar 21, 2017 at 1:32 AM, Michael Banck
> <michael(dot)banck(at)credativ(dot)de> wrote:
> /*
> + * Try to create a permanent replication slot if one is specified. Do
> + * not error out if the slot already exists, other errors are already
> + * reported by CreateReplicationSlot().
> + */
> + if (!stream->temp_slot && stream->replication_slot)
> + {
> + if (!CreateReplicationSlot(conn, stream->replication_slot,
> NULL, true, true))
> + return false;
> + }
> This could be part of an else taken from the previous if where
> temp_slot is checked.

Not sure how this would work, as ISTM the if clause above only sets the
name for param->temp_slot (if supported), but doesn't distinguish which
kind of slot (if any) will actually be created.

I've done it for the second (refactoring) patch though, but I had to
make `no_slot' a global variable to not have the logic be too
complicated.

> There should be some TAP tests included. And +1 for sharing the same
> behavior as pg_receivewal.

Well, I've adjusted the TAP tests in so far as it's now checking that
the physical slot got created, previously it checked for the opposite:

|-$node->command_fails(
|+$node->command_ok(
| [ 'pg_basebackup', '-D',
| "$tempdir/backupxs_sl_fail", '-X',
| 'stream', '-S',
|- 'slot1' ],
|- 'pg_basebackup fails with nonexistent replication slot');
|+ 'slot0' ],
|+ 'pg_basebackup runs with nonexistent replication slot');
|+
|+my $lsn = $node->safe_psql('postgres',
|+ q{SELECT restart_lsn FROM pg_replication_slots WHERE slot_name
|= 'slot0'}
|+);
|+like($lsn, qr!^0/[0-9A-F]{7,8}$!, 'slot is present');

I have now made the message a bit clearer by saying "pg_basebackup -S
creates previously nonexistent replication slot".

Probably there could be additional TAP tests, but off the top of my head
I can't think of any?

New patches attached.

Michael

--
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax: +49 2166 9901-100
Email: michael(dot)banck(at)credativ(dot)de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer