Re: [HACKERS] Create replication slot in pg_basebackup if requestedand 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.banck@credativ.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.banck@credativ.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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers