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

From: Michael Banck <michael(dot)banck(at)credativ(dot)de>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Arthur Zakirov <a(dot)zakirov(at)postgrespro(dot)ru>, Magnus Hagander <magnus(at)hagander(dot)net>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Create replication slot in pg_basebackup if requested and not yet present
Date: 2017-03-21 11:52:50
Message-ID: 1490097170.32520.10.camel@credativ.de
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-Create-replication-slot-in-pg_basebackup-if-requeste.patch text/x-patch 3.3 KB
0002-Refactor-replication-slot-creation-in-pg_basebackup-.patch text/x-patch 7.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2017-03-21 12:04:11 Re: Patch: Write Amplification Reduction Method (WARM)
Previous Message Ashutosh Bapat 2017-03-21 11:46:00 Re: Partition-wise join for join between (declaratively) partitioned tables