Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, Am Mittwoch, den 27.09.2017, 10:10 -0400 schrieb Peter Eisentraut: > On 9/11/17 03:11, Michael Banck wrote: > > So my patch only moves the slot creation slightly further forward, > > AFAICT. > > I have committed this patch, along with some associated cleanup. Thanks! > > AIUI, wal streaming always begins at last checkpoint and from my tests > > the restart_lsn of the created replication slot is also before that > > checkpoint's lsn. However, I hope somebody more familiar with the > > WAL/replication slot code could comment on that. What I dropped in the > > refactoring is the RESERVE_WAL that used to be there when the temporary > > slot gets created, I have readded that now. > > I had to make some changes to that, because the way your patch was > written it would use RESERVE_WAL also for the calls from pg_receivewal > --create-slot, which would have been a behavior change. So I added > another argument to CreateReplicationSlot() to control that. Euh right, thanks for catching that. > > I also added a TAP test case that tries to check that the restart_lsn is > > lower than the checkpoint_lsn, which appears to be the case. > > I think that test was written incorrectly, because it didn't actually > check the $checkpoint_lsn that it read. I have not included that test > in the committed patch. Feel free to send another patch if you want to > add more or different tests. I will take a look at that. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On 9/11/17 03:11, Michael Banck wrote: > So my patch only moves the slot creation slightly further forward, > AFAICT. I have committed this patch, along with some associated cleanup. > AIUI, wal streaming always begins at last checkpoint and from my tests > the restart_lsn of the created replication slot is also before that > checkpoint's lsn. However, I hope somebody more familiar with the > WAL/replication slot code could comment on that. What I dropped in the > refactoring is the RESERVE_WAL that used to be there when the temporary > slot gets created, I have readded that now. I had to make some changes to that, because the way your patch was written it would use RESERVE_WAL also for the calls from pg_receivewal --create-slot, which would have been a behavior change. So I added another argument to CreateReplicationSlot() to control that. > I also added a TAP test case that tries to check that the restart_lsn is > lower than the checkpoint_lsn, which appears to be the case. I think that test was written incorrectly, because it didn't actually check the $checkpoint_lsn that it read. I have not included that test in the committed patch. Feel free to send another patch if you want to add more or different tests. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On Thu, Sep 14, 2017 at 8:23 AM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 9/12/17 16:39, Michael Banck wrote: > > We could split up the logic here and create the optional physical > > replication slot in the main connection and the temporary one in the WAL > > streamer connection, but this would keep any fragility around for > > (likely more frequently used) temporary replication slots. It would make > > the patch much smaller though if I revert touching temporary slots at > > all. > > That's what I was thinking. > > But: > > If the race condition concern that Jeff was describing is indeed > correct, then the current use of temporary replication slots would be > equally affected. So I think either we already have a problem, or we > don't and then this patch wouldn't introduce a new one. > > I don't know the details of this well enough. > It is possible the race condition is entirely theoretical, as the WAL files won't be eligible for recycling until the end of the *next* (future) checkpoint (for no good reason, as far as I can tell, although fortunate in this case) so that should give plenty of opportunity for the WAL streamer to connect in all but the weirdest cases. When it reserves the WAL, I assume it does so back-dating to the LSN position reported by pg_start_backup and will promptly fail if those are no longer available? I don't want to hold up the patch based on theoretical questions when it solves an actual problem. Cheers, Jeff
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On 9/12/17 16:39, Michael Banck wrote: > We could split up the logic here and create the optional physical > replication slot in the main connection and the temporary one in the WAL > streamer connection, but this would keep any fragility around for > (likely more frequently used) temporary replication slots. It would make > the patch much smaller though if I revert touching temporary slots at > all. That's what I was thinking. But: If the race condition concern that Jeff was describing is indeed correct, then the current use of temporary replication slots would be equally affected. So I think either we already have a problem, or we don't and then this patch wouldn't introduce a new one. I don't know the details of this well enough. Thoughts from others? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, Am Dienstag, den 12.09.2017, 08:53 -0400 schrieb Peter Eisentraut: > On 9/11/17 03:11, Michael Banck wrote: > > > Is there a race condition here? The slot is created after the checkpoint > > > is completed. But you have to start streaming from the LSN where the > > > checkpoint started, so shouldn't the slot be created before the checkpoint > > > is started? > > > > So my patch only moves the slot creation slightly further forward, > > AFAICT. > > > > AIUI, wal streaming always begins at last checkpoint and from my tests > > the restart_lsn of the created replication slot is also before that > > checkpoint's lsn. However, I hope somebody more familiar with the > > WAL/replication slot code could comment on that. What I dropped in the > > refactoring is the RESERVE_WAL that used to be there when the temporary > > slot gets created, I have readded that now. > > Maybe there is an argument to be made here about whether this is correct > or not, but why bother and risk the fragility? Why not create the > replication slot first thing. I would put it after the server version > checks and before we write recovery.conf. The replication slots are created via the replication protocol through the second background connection that is used for WAL streaming in StartLogStreamer(). By their nature temporary replication slots must be created by that WAL streamer using them and cannot be created by the main connection which initiates the snapshot (or else you get a "replication slot "pg_basebackup_XXX" is active for PID XXX" error in the WAL streamer). So ISTM we cannot rip out CreateReplicationSlot() (or the manual CREATE_REPLICATION_SLOT that is currently in master) from StartLogStreamer() at least for temporary slots. We could split up the logic here and create the optional physical replication slot in the main connection and the temporary one in the WAL streamer connection, but this would keep any fragility around for (likely more frequently used) temporary replication slots. It would make the patch much smaller though if I revert touching temporary slots at all. The alternative would be to call StartLogStreamer() earlier, but it requires xlogstart as argument so we cannot move its call before the checkpoint is taken and xlogstart is determined, the earliest I managed was when starttli is determined, which is just few instructions earlier than now. Thoughts? Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On 9/11/17 03:11, Michael Banck wrote: >> Is there a race condition here? The slot is created after the checkpoint >> is completed. But you have to start streaming from the LSN where the >> checkpoint started, so shouldn't the slot be created before the checkpoint >> is started? > > So my patch only moves the slot creation slightly further forward, > AFAICT. > > AIUI, wal streaming always begins at last checkpoint and from my tests > the restart_lsn of the created replication slot is also before that > checkpoint's lsn. However, I hope somebody more familiar with the > WAL/replication slot code could comment on that. What I dropped in the > refactoring is the RESERVE_WAL that used to be there when the temporary > slot gets created, I have readded that now. Maybe there is an argument to be made here about whether this is correct or not, but why bother and risk the fragility? Why not create the replication slot first thing. I would put it after the server version checks and before we write recovery.conf. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, On Fri, Sep 08, 2017 at 08:41:56AM +0200, Michael Banck wrote: > Am Mittwoch, den 06.09.2017, 12:22 -0400 schrieb Peter Eisentraut: > > On 8/18/17 05:28, Michael Banck wrote: > > > > > Rebased, squashed and slighly edited version attached. I've added this > > > > > to the 2017-07 commitfest now as well: > > > > > > > > > > https://commitfest.postgresql.org/14/1112/ > > > > > > > > Can you rebase this past some conflicting changes? > > > > > > Thanks for letting me know, PFA a rebased version. > > > > I have reviewed the thread so far. I think there is general agreement > > that something like this would be good to have. > > > > I have not found any explanation, however, why the "if not exists" > > behavior is desirable, let alone as the default. I can only think of > > two workflows here: Either you have scripts for previous PG versions > > that create the slot externally, in which can you omit --create, or you > > use the new functionality to have pg_basebackup create the slot. I > > don't see any use for pg_basebackup to opportunistically use a slot if > > it happens to exist. Even if there is one, it should not be the > > default. So please change that. > > Ok, I tried to research why that was the case and couldn't find any > trace of a discussion either. > > So we should just error out in CreateReplicationSlot() in case a slot > exists, right? I think having yet another option like --create-if-not- > exists does not sound needed from what you wrote above. > > > A minor point, I suggest to print the message about the replication slot > > being created *after* the slot has been created. This aligns with how > > logical replication subscriptions work. > > Ok. > > > I don't see the need for printing a message about temporary slots. > > Since they are temporary, they will go away automatically, so there is > > nothing the user needs to know there. > > Ok. I thought I'd remembered some request around having this reported > always (maybe from Magnus), but I couldn't find anything in the prior > discussions either. > > If we don't print the message for temporary slots, then the > CreateReplicationSlot() refactoring and the addition of the > temp_replication_slot argument would be no longer needed, or is this > something useful on its own? New reworked and rebased patch attached, including setting RESERVE_WAL for physical replication slots and the additional TAP test comparing the restart_lsn with the checkpoint_lsn. The only thing from the above I did not change yet is the removal of the temporary slots creation message in verbose mode. I have the feeling knowing the temporary slot name could be helpful for debugging and pg_basebackup is already pretty chatty in verbose mode so I preferred to keep it, but I can remove it of course if that is the consensus. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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 >From fca622f6f7c6ef9e566c5a1b72044b824e7e02cc Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Sun, 10 Sep 2017 18:07:33 +0200 Subject: [PATCH] Add option to create a replication slot in pg_basebackup if not yet present. When requesting a particular replication slot, the new pg_basebackup option --create-slot tries to create it before starting to replicate from it unless it exists already. In order to allow reporting of temporary replication slot creation in --verbose mode, further refactor the slot creation logic. Add new argument `is_temporary' to CreateReplicationSlot() in streamutil.c which specifies whether a physical replication slot is temporary or not. Update all callers. Move the creation of the temporary replication slot for pg_basebackup from receivelog.c to pg_basebackup.c and mention it in --verbose mode. At the same time, also create the temporary slot via CreateReplicationSlot() instead of creating the temporary slot with an explicit SQL command. --- doc/src/sgml/ref/pg_basebackup.sgml | 12 ++ src/bin/pg_basebackup/pg_basebackup.c| 56 ++-- src/bin/pg_basebackup/pg_receivewal.c| 2 +- src/bin/pg_basebackup/pg_recvlogical.c | 2 +- src/bin/pg_basebackup/receivelog.c | 18 - src/bin/pg_basebackup/streamutil.c | 15 +--- src/bin/pg_basebackup/streamutil.h | 2 +- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 34 +++-- 8 files changed, 107 insertions(+), 34 deletions(-) diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 2454d35af3..57086295d3 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -269,6 +269,18 @@ PostgreSQL documentation + -C + --create-slot + + +
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, On Fri, Sep 08, 2017 at 10:30:20AM -0700, Jeff Janes wrote: > On Wed, Sep 6, 2017 at 9:22 AM, Peter Eisentraut < > peter.eisentr...@2ndquadrant.com> wrote: > > > On 8/18/17 05:28, Michael Banck wrote: > > >>> Rebased, squashed and slighly edited version attached. I've added this > > >>> to the 2017-07 commitfest now as well: > > >>> > > >>> https://commitfest.postgresql.org/14/1112/ > > >> Can you rebase this past some conflicting changes? > > > Thanks for letting me know, PFA a rebased version. > > > > I have reviewed the thread so far. I think there is general agreement > > that something like this would be good to have. > > > > I have not found any explanation, however, why the "if not exists" > > behavior is desirable, let alone as the default. I can only think of > > two workflows here: Either you have scripts for previous PG versions > > that create the slot externally, in which can you omit --create, or you > > use the new functionality to have pg_basebackup create the slot. I > > don't see any use for pg_basebackup to opportunistically use a slot if > > it happens to exist. Even if there is one, it should not be the > > default. So please change that. > > +1. I'd rather just get an error and re-run without the --create switch. > That way you are forced to think about what you are doing. OK. > Is there a race condition here? The slot is created after the checkpoint > is completed. But you have to start streaming from the LSN where the > checkpoint started, so shouldn't the slot be created before the checkpoint > is started? So my patch only moves the slot creation slightly further forward, AFAICT. AIUI, wal streaming always begins at last checkpoint and from my tests the restart_lsn of the created replication slot is also before that checkpoint's lsn. However, I hope somebody more familiar with the WAL/replication slot code could comment on that. What I dropped in the refactoring is the RESERVE_WAL that used to be there when the temporary slot gets created, I have readded that now. I also added a TAP test case that tries to check that the restart_lsn is lower than the checkpoint_lsn, which appears to be the case. If there is still a race condition here, do you have a suggestion in how to try to trigger it? Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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 >From fca622f6f7c6ef9e566c5a1b72044b824e7e02cc Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Sun, 10 Sep 2017 18:07:33 +0200 Subject: [PATCH] Add option to create a replication slot in pg_basebackup if not yet present. When requesting a particular replication slot, the new pg_basebackup option --create-slot tries to create it before starting to replicate from it unless it exists already. In order to allow reporting of temporary replication slot creation in --verbose mode, further refactor the slot creation logic. Add new argument `is_temporary' to CreateReplicationSlot() in streamutil.c which specifies whether a physical replication slot is temporary or not. Update all callers. Move the creation of the temporary replication slot for pg_basebackup from receivelog.c to pg_basebackup.c and mention it in --verbose mode. At the same time, also create the temporary slot via CreateReplicationSlot() instead of creating the temporary slot with an explicit SQL command. --- doc/src/sgml/ref/pg_basebackup.sgml | 12 ++ src/bin/pg_basebackup/pg_basebackup.c| 56 ++-- src/bin/pg_basebackup/pg_receivewal.c| 2 +- src/bin/pg_basebackup/pg_recvlogical.c | 2 +- src/bin/pg_basebackup/receivelog.c | 18 - src/bin/pg_basebackup/streamutil.c | 15 +--- src/bin/pg_basebackup/streamutil.h | 2 +- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 34 +++-- 8 files changed, 107 insertions(+), 34 deletions(-) diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 2454d35af3..57086295d3 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -269,6 +269,18 @@ PostgreSQL documentation + -C + --create-slot + + +This option can only be used together with the --slot +option. It causes the specified slot name to be created unless it +exists already. + + + + + -T olddir=newdir --tablespace-mapping=olddir=newdir diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 51509d150e..35ea707384 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -92,6 +92,8 @@ static pg_time_t last_progress_report = 0; static int32
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On Wed, Sep 6, 2017 at 9:22 AM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 8/18/17 05:28, Michael Banck wrote: > >>> Rebased, squashed and slighly edited version attached. I've added this > >>> to the 2017-07 commitfest now as well: > >>> > >>> https://commitfest.postgresql.org/14/1112/ > >> Can you rebase this past some conflicting changes? > > Thanks for letting me know, PFA a rebased version. > > I have reviewed the thread so far. I think there is general agreement > that something like this would be good to have. > > I have not found any explanation, however, why the "if not exists" > behavior is desirable, let alone as the default. I can only think of > two workflows here: Either you have scripts for previous PG versions > that create the slot externally, in which can you omit --create, or you > use the new functionality to have pg_basebackup create the slot. I > don't see any use for pg_basebackup to opportunistically use a slot if > it happens to exist. Even if there is one, it should not be the > default. So please change that. > +1. I'd rather just get an error and re-run without the --create switch. That way you are forced to think about what you are doing. Is there a race condition here? The slot is created after the checkpoint is completed. But you have to start streaming from the LSN where the checkpoint started, so shouldn't the slot be created before the checkpoint is started? Cheers, Jeff
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, Am Mittwoch, den 06.09.2017, 12:22 -0400 schrieb Peter Eisentraut: > On 8/18/17 05:28, Michael Banck wrote: > > > > Rebased, squashed and slighly edited version attached. I've added this > > > > to the 2017-07 commitfest now as well: > > > > > > > > https://commitfest.postgresql.org/14/1112/ > > > > > > Can you rebase this past some conflicting changes? > > > > Thanks for letting me know, PFA a rebased version. > > I have reviewed the thread so far. I think there is general agreement > that something like this would be good to have. > > I have not found any explanation, however, why the "if not exists" > behavior is desirable, let alone as the default. I can only think of > two workflows here: Either you have scripts for previous PG versions > that create the slot externally, in which can you omit --create, or you > use the new functionality to have pg_basebackup create the slot. I > don't see any use for pg_basebackup to opportunistically use a slot if > it happens to exist. Even if there is one, it should not be the > default. So please change that. Ok, I tried to research why that was the case and couldn't find any trace of a discussion either. So we should just error out in CreateReplicationSlot() in case a slot exists, right? I think having yet another option like --create-if-not- exists does not sound needed from what you wrote above. > A minor point, I suggest to print the message about the replication slot > being created *after* the slot has been created. This aligns with how > logical replication subscriptions work. Ok. > I don't see the need for printing a message about temporary slots. > Since they are temporary, they will go away automatically, so there is > nothing the user needs to know there. Ok. I thought I'd remembered some request around having this reported always (maybe from Magnus), but I couldn't find anything in the prior discussions either. If we don't print the message for temporary slots, then the CreateReplicationSlot() refactoring and the addition of the temp_replication_slot argument would be no longer needed, or is this something useful on its own? Thanks, Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, not directly related to the topic, but: On Tue, Mar 21, 2017 at 03:34:00PM -0400, Robert Haas wrote: > For example, somebody creates a replica using the new super-easy > method, and then blows it away without dropping the slot from the > master, Just a thought, but maybe there should be some pg_dropstandby command or similar, which cleans everything (what exactly?) up? That might go some way to ensure this does not happen. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On 8/18/17 05:28, Michael Banck wrote: >>> Rebased, squashed and slighly edited version attached. I've added this >>> to the 2017-07 commitfest now as well: >>> >>> https://commitfest.postgresql.org/14/1112/ >> Can you rebase this past some conflicting changes? > Thanks for letting me know, PFA a rebased version. I have reviewed the thread so far. I think there is general agreement that something like this would be good to have. I have not found any explanation, however, why the "if not exists" behavior is desirable, let alone as the default. I can only think of two workflows here: Either you have scripts for previous PG versions that create the slot externally, in which can you omit --create, or you use the new functionality to have pg_basebackup create the slot. I don't see any use for pg_basebackup to opportunistically use a slot if it happens to exist. Even if there is one, it should not be the default. So please change that. A minor point, I suggest to print the message about the replication slot being created *after* the slot has been created. This aligns with how logical replication subscriptions work. I don't see the need for printing a message about temporary slots. Since they are temporary, they will go away automatically, so there is nothing the user needs to know there. With these two changes, I think I'd be content with this patch. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, On Tue, Aug 15, 2017 at 02:14:58PM -0700, Jeff Janes wrote: > On Sun, Apr 9, 2017 at 4:22 AM, Michael Banck > wrote: > > Rebased, squashed and slighly edited version attached. I've added this > > to the 2017-07 commitfest now as well: > > > > https://commitfest.postgresql.org/14/1112/ > > Can you rebase this past some conflicting changes? Thanks for letting me know, PFA a rebased version. I also adressed the following message which appeared when starting the TAP tests: 't/010_pg_basebackup.pl ... "my" variable $lsn masks earlier declaration in same scope at t/010_pg_basebackup.pl line 287.' Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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 >From 62973bba3338abfbf4d9e2f0f05a338eb283b4b8 Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Fri, 18 Aug 2017 11:28:26 +0200 Subject: [PATCH] Add option to create a replication slot in pg_basebackup if not yet present. When requesting a particular replication slot, the new pg_basebackup option --create-slot tries to create it before starting to replicate from it in case it does not exist already. In order to allow reporting of temporary replication slot creation in --verbose mode, further refactor the slot creation logic. Add new argument `is_temporary' to CreateReplicationSlot() in streamutil.c which specifies whether a physical replication slot is temporary or not. Update all callers. Move the creation of the temporary replication slot for pg_basebackup from receivelog.c to pg_basebackup.c and mention it in --verbose mode. At the same time, also create the temporary slot via CreateReplicationSlot() instead of creating the temporary slot with an explicit SQL command. --- doc/src/sgml/ref/pg_basebackup.sgml | 12 ++ src/bin/pg_basebackup/pg_basebackup.c| 58 ++-- src/bin/pg_basebackup/pg_receivewal.c| 2 +- src/bin/pg_basebackup/pg_recvlogical.c | 2 +- src/bin/pg_basebackup/receivelog.c | 18 - src/bin/pg_basebackup/streamutil.c | 18 ++--- src/bin/pg_basebackup/streamutil.h | 2 +- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 25 ++-- 8 files changed, 103 insertions(+), 34 deletions(-) diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index 2454d35af3..5397a185d2 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -269,6 +269,18 @@ PostgreSQL documentation + -C + --create-slot + + +This option can only be used together with the --slot +option. It causes the specified slot name to be created if it does not +exist already. + + + + + -T olddir=newdir --tablespace-mapping=olddir=newdir diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index dfb9b5ddcb..2bc7e868e0 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -92,6 +92,8 @@ static pg_time_t last_progress_report = 0; static int32 maxrate = 0; /* no limit by default */ static char *replication_slot = NULL; static bool temp_replication_slot = true; +static bool create_slot = false; +static bool no_slot = false; static bool success = false; static bool made_new_pgdata = false; @@ -337,6 +339,7 @@ usage(void) " write recovery.conf for replication\n")); printf(_(" -S, --slot=SLOTNAMEreplication slot to use\n")); printf(_(" --no-slot prevent creation of temporary replication slot\n")); + printf(_(" -C, --create-slot create replication slot if not present already\n")); printf(_(" -T, --tablespace-mapping=OLDDIR=NEWDIR\n" " relocate tablespace in OLDDIR to NEWDIR\n")); printf(_(" -X, --wal-method=none|fetch|stream\n" @@ -492,8 +495,6 @@ LogStreamerMain(logstreamer_param *param) stream.partial_suffix = NULL; stream.replication_slot = replication_slot; stream.temp_slot = param->temp_slot; - if (stream.temp_slot && !stream.replication_slot) - stream.replication_slot = psprintf("pg_basebackup_%d", (int) PQbackendPID(param->bgconn)); if (format == 'p') stream.walmethod = CreateWalDirectoryMethod(param->xlog, 0, do_sync); @@ -586,6 +587,29 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) else param->temp_slot = temp_replication_slot; + /* + * Create replication slot if one is needed. + */ + if (!no_slot && (temp_replication_slot || create_slot)) + { + if (!replication_slot) + replication_slot = psprintf("pg_basebackup_%d", (int) getpid()); + + if (verbose) + { + if (temp_replication_slot) +fprintf(stderr, _("%s:
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On Sun, Apr 9, 2017 at 4:22 AM, Michael Banck wrote: > Hi, > > Am Freitag, den 24.03.2017, 19:32 +0100 schrieb Michael Banck: > > On Thu, Mar 23, 2017 at 12:41:54PM +0100, Magnus Hagander wrote: > > > On Tue, Mar 21, 2017 at 8:34 PM, Robert Haas > wrote: > > > > So I tend to think that there should always be some explicit user > > > > action to cause the creation of a slot, like --create-slot-if-needed > > > > or --create-slot=name. That still won't prevent careless use of that > > > > option but it's less dangerous than assuming that a user who refers > to > > > > a nonexistent slot intended to create it when, perhaps, they just > > > > typo'd it. > > > > > > Well, the explicit user action would be "--slot". But sure, I can > > > definitely live with a --create-if-not-exists. > > > > Can we just make that option create slot and don't worry if one exists > > already? CreateReplicationSlot() can be told to not mind about already > > existing slots, and I don't see a huge point in erroring out if the slot > > exists already, unless somebody can show that this leads to data loss > > somehow. > > > > > The important thing, to me, is that you can run it as a single > > > command, which makes it a lot easier to work with. (And not like we > > > currently have for pg_receivewal which requires a separate command to > > > create the slot) > > > > Oh, that is how it works with pg_receivewal, I have to admit I've never > > used it so was a bit confused about this when I read its code. > > > > So in that case I think we don't necessarily need to have the same user > > interface at all. I first thought about just adding "-C, --create" (as > > in "--create --slot=foo"), but this on second thought this looked a bit > > shortsighted - who knows what flashy thing pg_basebackup might create in > > 5 years... So I settled on --create-slot, which is only slightly more to > > type (albeit repetive, "--create-slot --slot=foo"), but adding a short > > option "-C" would be fine I thinkg "-C -S foo". > > > > So attached is a patch with adds that option. If people really think it > > should be --create-slot-if-not-exists instead I can update the patch, of > > course. > > > > I again added a second patch with some further refactoring which makes > > it print a message on temporary slot creation in verbose mode. > > Rebased, squashed and slighly edited version attached. I've added this > to the 2017-07 commitfest now as well: > > https://commitfest.postgresql.org/14/1112/ Can you rebase this past some conflicting changes? Thanks, Jeff
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, Am Freitag, den 24.03.2017, 19:32 +0100 schrieb Michael Banck: > On Thu, Mar 23, 2017 at 12:41:54PM +0100, Magnus Hagander wrote: > > On Tue, Mar 21, 2017 at 8:34 PM, Robert Haas wrote: > > > So I tend to think that there should always be some explicit user > > > action to cause the creation of a slot, like --create-slot-if-needed > > > or --create-slot=name. That still won't prevent careless use of that > > > option but it's less dangerous than assuming that a user who refers to > > > a nonexistent slot intended to create it when, perhaps, they just > > > typo'd it. > > > > Well, the explicit user action would be "--slot". But sure, I can > > definitely live with a --create-if-not-exists. > > Can we just make that option create slot and don't worry if one exists > already? CreateReplicationSlot() can be told to not mind about already > existing slots, and I don't see a huge point in erroring out if the slot > exists already, unless somebody can show that this leads to data loss > somehow. > > > The important thing, to me, is that you can run it as a single > > command, which makes it a lot easier to work with. (And not like we > > currently have for pg_receivewal which requires a separate command to > > create the slot) > > Oh, that is how it works with pg_receivewal, I have to admit I've never > used it so was a bit confused about this when I read its code. > > So in that case I think we don't necessarily need to have the same user > interface at all. I first thought about just adding "-C, --create" (as > in "--create --slot=foo"), but this on second thought this looked a bit > shortsighted - who knows what flashy thing pg_basebackup might create in > 5 years... So I settled on --create-slot, which is only slightly more to > type (albeit repetive, "--create-slot --slot=foo"), but adding a short > option "-C" would be fine I thinkg "-C -S foo". > > So attached is a patch with adds that option. If people really think it > should be --create-slot-if-not-exists instead I can update the patch, of > course. > > I again added a second patch with some further refactoring which makes > it print a message on temporary slot creation in verbose mode. Rebased, squashed and slighly edited version attached. I've added this to the 2017-07 commitfest now as well: https://commitfest.postgresql.org/14/1112/ Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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 From 47fc0d543a43441f239f109d08bfc7c082f4c091 Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Fri, 24 Mar 2017 18:27:47 +0100 Subject: [PATCH] Add option to create a replication slot in pg_basebackup if not yet present. When requesting a particular replication slot, the new pg_basebackup option --create-slot tries to create it before starting to replicate from it in case it does not exist already. In order to allow reporting of temporary replication slot creation in --verbose mode, further refactor the slot creation logic. Add new argument `is_temporary' to CreateReplicationSlot() in streamutil.c which specifies whether a physical replication slot is temporary or not. Update all callers. Move the creation of the temporary replication slot for pg_basebackup from receivelog.c to pg_basebackup.c and mention it in --verbose mode. At the same time, also create the temporary slot via CreateReplicationSlot() instead of creating the temporary slot with an explicit SQL command. --- doc/src/sgml/ref/pg_basebackup.sgml | 12 ++ src/bin/pg_basebackup/pg_basebackup.c| 58 ++-- src/bin/pg_basebackup/pg_receivewal.c| 2 +- src/bin/pg_basebackup/pg_recvlogical.c | 2 +- src/bin/pg_basebackup/receivelog.c | 18 - src/bin/pg_basebackup/streamutil.c | 18 ++--- src/bin/pg_basebackup/streamutil.h | 2 +- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 23 +-- 8 files changed, 102 insertions(+), 33 deletions(-) diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index e1cec9d..65ca8dc 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -269,6 +269,18 @@ PostgreSQL documentation + -C + --create-slot + + +This option can only be used together with the --slot +option. It causes the specified slot name to be created if it does not +exist already. + + + + + -T olddir=newdir --tablespace-mapping=olddir=newdir diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 40ec0e1..6442a54 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, On Thu, Mar 23, 2017 at 12:41:54PM +0100, Magnus Hagander wrote: > On Tue, Mar 21, 2017 at 8:34 PM, Robert Haas wrote: > > So I tend to think that there should always be some explicit user > > action to cause the creation of a slot, like --create-slot-if-needed > > or --create-slot=name. That still won't prevent careless use of that > > option but it's less dangerous than assuming that a user who refers to > > a nonexistent slot intended to create it when, perhaps, they just > > typo'd it. > > Well, the explicit user action would be "--slot". But sure, I can > definitely live with a --create-if-not-exists. Can we just make that option create slot and don't worry if one exists already? CreateReplicationSlot() can be told to not mind about already existing slots, and I don't see a huge point in erroring out if the slot exists already, unless somebody can show that this leads to data loss somehow. > The important thing, to me, is that you can run it as a single > command, which makes it a lot easier to work with. (And not like we > currently have for pg_receivewal which requires a separate command to > create the slot) Oh, that is how it works with pg_receivewal, I have to admit I've never used it so was a bit confused about this when I read its code. So in that case I think we don't necessarily need to have the same user interface at all. I first thought about just adding "-C, --create" (as in "--create --slot=foo"), but this on second thought this looked a bit shortsighted - who knows what flashy thing pg_basebackup might create in 5 years... So I settled on --create-slot, which is only slightly more to type (albeit repetive, "--create-slot --slot=foo"), but adding a short option "-C" would be fine I thinkg "-C -S foo". So attached is a patch with adds that option. If people really think it should be --create-slot-if-not-exists instead I can update the patch, of course. I again added a second patch with some further refactoring which makes it print a message on temporary slot creation in verbose mode. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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 >From 4e37e110ac402e67874f729832b330a837284d4b Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Fri, 24 Mar 2017 18:27:47 +0100 Subject: [PATCH 1/2] Add option to create a replication slot in pg_basebackup if not yet present. When reqeusting a particular replication slot, the new option --create tries to create it before starting to replicate from it in case it does not exist already. --- doc/src/sgml/ref/pg_basebackup.sgml | 12 src/bin/pg_basebackup/pg_basebackup.c| 44 +++- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 23 +-- 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml index e1cec9d..789f649 100644 --- a/doc/src/sgml/ref/pg_basebackup.sgml +++ b/doc/src/sgml/ref/pg_basebackup.sgml @@ -269,6 +269,18 @@ PostgreSQL documentation + -C + --create-slot + + +This option can only be used together with the --slot + option. It causes the specified slot name to be created if it does not +exist already. + + + + + -T olddir=newdir --tablespace-mapping=olddir=newdir diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 0a4944d..2af2e22 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -92,6 +92,7 @@ static pg_time_t last_progress_report = 0; static int32 maxrate = 0; /* no limit by default */ static char *replication_slot = NULL; static bool temp_replication_slot = true; +static bool create_slot = false; static bool success = false; static bool made_new_pgdata = false; @@ -337,6 +338,7 @@ usage(void) " write recovery.conf for replication\n")); printf(_(" -S, --slot=SLOTNAMEreplication slot to use\n")); printf(_(" --no-slot prevent creation of temporary replication slot\n")); + printf(_(" -C, --create-slot create replication slot if not present already\n")); printf(_(" -T, --tablespace-mapping=OLDDIR=NEWDIR\n" " relocate tablespace in OLDDIR to NEWDIR\n")); printf(_(" -X, --wal-method=none|fetch|stream\n" @@ -581,6 +583,19 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) else param->temp_slot = temp_replication_slot; + /* + * Create permanent physical replication slot if requested. + */ + if (replication_slot && create_slot) + { + if (verbose) + fprintf(stderr, _("%s: creating replication slot \"%s\"\n"), + p
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On Tue, Mar 21, 2017 at 8:34 PM, Robert Haas wrote: > On Sun, Mar 19, 2017 at 12:01 PM, Magnus Hagander > wrote: > > I think maybe we should output a message when the slot is created, at > least > > in verbose mode, to make sure people realize that happened. Does that > seem > > reasonable? > > Slots are great until you leave one lying around by accident. I'm > afraid that no matter what we do, we're going to start getting > complaints from people who mess that up. For example, somebody > creates a replica using the new super-easy method, and then blows it > away without dropping the slot from the master, and then days or weeks > later pg_wal fills up and takes the server down. The user says, oh, > these old write-ahead log files should have gotten removed, and > removes them all. Oops. > So I tend to think that there should always be some explicit user > action to cause the creation of a slot, like --create-slot-if-needed > or --create-slot=name. That still won't prevent careless use of that > option but it's less dangerous than assuming that a user who refers to > a nonexistent slot intended to create it when, perhaps, they just > typo'd it. > Well, the explicit user action would be "--slot". But sure, I can definitely live with a --create-if-not-exists. The important thing, to me, is that you can run it as a single command, which makes it a lot easier to work with. (And not like we currently have for pg_receivewal which requires a separate command to create the slot) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hello, I favor this feature. At Wed, 22 Mar 2017 00:18:19 -0400, Peter Eisentraut wrote in <1f5daba9-773d-9281-5608-37f049025...@2ndquadrant.com> > On 3/21/17 15:34, Robert Haas wrote: > > So I tend to think that there should always be some explicit user > > action to cause the creation of a slot, like --create-slot-if-needed > > or --create-slot=name. That still won't prevent careless use of that > > option but it's less dangerous than assuming that a user who refers to > > a nonexistent slot intended to create it when, perhaps, they just > > typo'd it. > > I have the same concern. A slot created as !immeediately_reserve (even though currently CREATE_REPLICATION_SLOT command doesn't seem to have the option) won't do such a trick but I agree to the point. I think that any explicit action is required unless any anticipated catastrophic end caused by remainig slots is evaded implicitly. Do ephemeral or temporary slots help this case? Otherwise, I'm proposing a patch to ignore restart-lsn of slots that let too many WALs to stay on. https://www.postgresql.org/message-id/20170228.122736.123383594.horiguchi.kyot...@lab.ntt.co.jp Instaed just ignoring restart-lsn, like Andres' suggestion, removing (or just disabling) a slot that is marked as 'auto-removable' with the same kind (including disconnection timeout) of trigger also will work. I had a similar annoyance with CREATE SUBSCRIPTION. It implicitly creates a slot on publisher and subscriber fails to have the same subscription after re-initialization. Of couse DROP SUBSCRIPTION doesn't help the case. Users don't have a clue to solution, I suppose. But this would be another topic. regards, -- Kyotaro Horiguchi NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On 3/21/17 15:34, Robert Haas wrote: > So I tend to think that there should always be some explicit user > action to cause the creation of a slot, like --create-slot-if-needed > or --create-slot=name. That still won't prevent careless use of that > option but it's less dangerous than assuming that a user who refers to > a nonexistent slot intended to create it when, perhaps, they just > typo'd it. I have the same concern. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, Am Mittwoch, den 22.03.2017, 00:40 +0100 schrieb Michael Banck: > I guess if we decide (physical) slots should not be created implicitly, > then using the same UI as pg_receivewal makes sense for the sake of > consistency, i.e. "--slot=name --create-slot [--if-not-exists]". That is > rather verbose though and I don't think the --if-not-exists is great UX, > but maybe there is some use-case for erroring out if a slot already > exists that I haven't thought of. Thinking about this some more, there's the case of somebody accidentally using an already existing slot that was meant for another standby which happens to be down just at that moment. >From some quick testing, that makes replication fail once the other standby is started up again, but soon after the new standby is taken down, replication picks up without apparent problems for the other standby. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Am Dienstag, den 21.03.2017, 15:34 -0400 schrieb Robert Haas: > On Sun, Mar 19, 2017 at 12:01 PM, Magnus Hagander wrote: > > I think maybe we should output a message when the slot is created, at least > > in verbose mode, to make sure people realize that happened. Does that seem > > reasonable? > > Slots are great until you leave one lying around by accident. I'm > afraid that no matter what we do, we're going to start getting > complaints from people who mess that up. For example, somebody > creates a replica using the new super-easy method, and then blows it > away without dropping the slot from the master, and then days or weeks > later pg_wal fills up and takes the server down. The user says, oh, > these old write-ahead log files should have gotten removed, and > removes them all. Oops. Hrm. Maybe it would be useful to log a warning like "slot XY has not been active for X minutes", based on some threshold, though possibly the people not keeping an eye on their replication slots won't spot that in the logfiles, either. > So I tend to think that there should always be some explicit user > action to cause the creation of a slot, like --create-slot-if-needed > or --create-slot=name. That still won't prevent careless use of that > option but it's less dangerous than assuming that a user who refers to > a nonexistent slot intended to create it when, perhaps, they just > typo'd it. I guess if we decide (physical) slots should not be created implicitly, then using the same UI as pg_receivewal makes sense for the sake of consistency, i.e. "--slot=name --create-slot [--if-not-exists]". That is rather verbose though and I don't think the --if-not-exists is great UX, but maybe there is some use-case for erroring out if a slot already exists that I haven't thought of. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On Sun, Mar 19, 2017 at 12:01 PM, Magnus Hagander wrote: > I think maybe we should output a message when the slot is created, at least > in verbose mode, to make sure people realize that happened. Does that seem > reasonable? Slots are great until you leave one lying around by accident. I'm afraid that no matter what we do, we're going to start getting complaints from people who mess that up. For example, somebody creates a replica using the new super-easy method, and then blows it away without dropping the slot from the master, and then days or weeks later pg_wal fills up and takes the server down. The user says, oh, these old write-ahead log files should have gotten removed, and removes them all. Oops. So I tend to think that there should always be some explicit user action to cause the creation of a slot, like --create-slot-if-needed or --create-slot=name. That still won't prevent careless use of that option but it's less dangerous than assuming that a user who refers to a nonexistent slot intended to create it when, perhaps, they just typo'd it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Am Dienstag, den 21.03.2017, 12:52 +0100 schrieb Michael Banck: > New patches attached. On second though, there was a superfluous whitespace change in t/010_pg_basebackup.pl, and I've moved the check-for-hex regex fix to the second patch as it's cosmetic and not related to changing the --slot creation behaviour. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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 From 2ab1be27a341ecdcd2d6a3dd5ce535aba5e16b03 Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Sun, 19 Mar 2017 10:58:13 +0100 Subject: [PATCH 01/29] Create replication slot in pg_basebackup if requested and not yet present. If a replication slot is explicitly requested, try to create it before starting to replicate from it. --- src/bin/pg_basebackup/pg_basebackup.c| 15 +++ src/bin/pg_basebackup/t/010_pg_basebackup.pl | 15 ++- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 0a4944d..69ca4be 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -581,6 +581,21 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) else param->temp_slot = temp_replication_slot; + /* + * 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 (!temp_replication_slot && replication_slot) + { + if (verbose) + fprintf(stderr, _("%s: creating replication slot \"%s\"\n"), + progname, replication_slot); + + if (!CreateReplicationSlot(param->bgconn, replication_slot, NULL, true, true)) + return 1; + } + if (format == 'p') { /* diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 14bd813..70c3284 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -4,7 +4,7 @@ use Cwd; use Config; use PostgresNode; use TestLib; -use Test::More tests => 72; +use Test::More tests => 73; program_help_ok('pg_basebackup'); program_version_ok('pg_basebackup'); @@ -246,14 +246,19 @@ $node->command_ok( 'pg_basebackup -X stream runs with --no-slot'); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1' ], + [ 'pg_basebackup', '-D', "$tempdir/fail", '-X', 'none', '-S', 'slot0' ], 'pg_basebackup with replication slot fails without -X stream'); -$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 -S creates previously 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'); $node->safe_psql('postgres', q{SELECT * FROM pg_create_physical_replication_slot('slot1')}); -- 2.1.4 From 5f0f31f61bf668de937868840a97c0a56dc2cd5d Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Tue, 21 Mar 2017 12:50:22 +0100 Subject: [PATCH 02/29] Refactor replication slot creation in pg_basebackup et al. Add new argument `is_temporary' to CreateReplicationSlot() in streamutil.c which specifies whether a physical replication slot is temporary or not. Update all callers. Move the creation of the temporary replication slot for pg_basebackup from receivelog.c to pg_basebackup.c. At the same time, also create the temporary slot via CreateReplicationSlot() instead of creating the temporary slot with an explicit SQL command. --- src/bin/pg_basebackup/pg_basebackup.c| 28 +--- src/bin/pg_basebackup/pg_receivewal.c| 2 +- src/bin/pg_basebackup/pg_recvlogical.c | 2 +- src/bin/pg_basebackup/receivelog.c | 18 -- src/bin/pg_basebackup/streamutil.c | 18 +- src/bin/pg_basebackup/streamutil.h | 2 +- src/bin/pg_basebackup/t/010_pg_basebackup.pl | 2 +- 7 files changed, 34 insertions(+), 38 deletions(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 69ca4be..c7ae281 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -92,6 +92,7 @@ static pg_time_t last_progress_report = 0; static int32 maxrate = 0; /* no limit by default */ static char *replication_slot = NULL; static bool temp_replication_slot = true; +static bool no_slot = false; static bool success = false; static bool made_new_pgdat
Re: [HACKERS] 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 > 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.ba...@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 From 93337fe0ef320fe8ef68a9b64c4df85a63c4346c Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Sun, 19 Mar 2017 10:58:13 +0100 Subject: [PATCH 1/2] Create replication slot in pg_basebackup if requested and not yet present. If a replication slot is explicitly requested, try to create it before starting to replicate from it. --- src/bin/pg_basebackup/pg_basebackup.c| 15 +++ src/bin/pg_basebackup/t/010_pg_basebackup.pl | 19 --- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 0a4944d..69ca4be 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -581,6 +581,21 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) else param->temp_slot = temp_replication_slot; + /* + * 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 (!temp_replication_slot && replication_slot) + { + if (verbose) + fprintf(stderr, _("%s: creating replication slot \"%s\"\n"), + progname, replication_slot); + + if (!CreateReplicationSlot(param->bgconn, replication_slot, NULL, true, true)) + return 1; + } + if (format == 'p') { /* diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl index 14bd813..f50005f 100644 --- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl +++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl @@ -4,7 +4,7 @@ use Cwd; use Config; use PostgresNode; use TestLib; -use Test::More tests => 72; +use Test::More tests => 73; program_help_ok('pg_basebackup'); program_version_ok('pg_basebackup'); @@ -246,17 +246,22 @@ $node->command_ok( 'pg_basebackup -X stream runs with --no-slot'); $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1' ], + [ 'pg_basebackup', '-D', "$tempdir/fail", '-X', 'none', '-S', 'slot0' ], 'pg_basebackup with replication slot fails without -X stream'); -$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 -S creates previously 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'); $node->saf
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On Tue, Mar 21, 2017 at 1:32 AM, Michael Banck wrote: > On Mon, Mar 20, 2017 at 02:42:32PM +0300, Arthur Zakirov wrote: >> Also maybe it would be good if pg_basebackup had a way to drop created slot. >> Although "drop slot" is not related with concept of automatically created >> slots, it will good if user will have a way to drop slots. > > If you want to drop the slot after basebackup finishes you'd just use a > temporary slot (i.e. the default), or am I understanding your use-case > incorrectly? Yup, agreed. > I assumed the primary use-case for creating a non-temporary slot is to > keep it around while the standby is active. Indeed. /* + * 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. There should be some TAP tests included. And +1 for sharing the same behavior as pg_receivewal. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, On Mon, Mar 20, 2017 at 02:42:32PM +0300, Arthur Zakirov wrote: > Also maybe it would be good if pg_basebackup had a way to drop created slot. > Although "drop slot" is not related with concept of automatically created > slots, it will good if user will have a way to drop slots. If you want to drop the slot after basebackup finishes you'd just use a temporary slot (i.e. the default), or am I understanding your use-case incorrectly? I assumed the primary use-case for creating a non-temporary slot is to keep it around while the standby is active. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hello, On 19.03.2017 21:45, Michael Banck wrote: So the patch I sent earlier creates the slot in ReceiveXlogStream() in receivewal.c, as that's where the temp slot gets created as well, but now I wonder whether that is maybe not the best place, as pg_receivewal also calls that function. The other problem with receivewal.c is that `verbose' isn't around in it so I don't how I'd print out a message there. So probably it is better to create the slot in pg_basebackup.c's StartLogStreamer(), see the attached first patch, that one also adds a verbose message. I think such too. I suppose it is more clearly. StartLogStreamer() is better place for creating permanent and temporary slots. Also maybe it would be good if pg_basebackup had a way to drop created slot. Although "drop slot" is not related with concept of automatically created slots, it will good if user will have a way to drop slots. -- Arthur Zakirov Postgres Professional: http://www.postgrespro.com Russian Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, On Sun, Mar 19, 2017 at 05:01:23PM +0100, Magnus Hagander wrote: > On Sun, Mar 19, 2017 at 11:21 AM, Michael Banck > wrote: > > So I propose the attached tiny patch to just create the slot (if it does > > not exist already) in pg_basebackup, somewhat similar to what > > pg_receivewal does, albeit unconditionally, if the user explicitly > > requested a slot: > > > > $ pg_basebackup --pgdata=data2 --write-recovery-conf --slot=pg2 > > $ echo $? > > 0 > > $ psql -c "DROP_REPLICATION_SLOT pg2" "host=127.0.0.1 port=5432 > > replication=database user=mba dbname=postgres" > > SELECT > > $ > > > > This would get us somewhat closer to near zero-config replication, in my > > opinion. Pardon me if that was discussed already and shot down > > > > Comments? > > I think this is a good idea, since it makes replication slots easier to > use. The change to use temporary slots was good for backups, but didn't > help people setting up replicas. > > I've been annoyed for a while we didn't have a "create slot" mode in > pg_basebackup, but doing it integrated like this is definitely a step even > better than that. Great! > I think maybe we should output a message when the slot is created, at least > in verbose mode, to make sure people realize that happened. Does that seem > reasonable? So the patch I sent earlier creates the slot in ReceiveXlogStream() in receivewal.c, as that's where the temp slot gets created as well, but now I wonder whether that is maybe not the best place, as pg_receivewal also calls that function. The other problem with receivewal.c is that `verbose' isn't around in it so I don't how I'd print out a message there. So probably it is better to create the slot in pg_basebackup.c's StartLogStreamer(), see the attached first patch, that one also adds a verbose message. I also now realized I didn't ran the TAP tests and they need updating, this has been done in the first attached patch as well. Independently of this patch series I think those two hunks from the third patch are applicable to current master as well: $node->command_fails( - [ 'pg_basebackup', '-D', "$tempdir/fail", '-S', 'slot1' ], + [ 'pg_basebackup', '-D', "$tempdir/fail", '-X', 'none', '-S', 'slot0' ], 'pg_basebackup with replication slot fails without -X stream'); as '-X stream' is now the default, and (more cosmetically) -like($lsn, qr!^0/[0-9A-Z]{7,8}$!, 'restart LSN of slot has advanced'); +like($lsn, qr!^0/[0-9A-F]{7,8}$!, 'restart LSN of slot has advanced'); as we are looking for hex values. However, the other thing to ponder is that we don't really know whether the user maybe setup a replication slot on the primary in preparation already as there seems to be no way to query the list of current slots via the replication protocal, and setting up another normal connection just to query pg_replication_slots seems to be overkill. So maybe the user would be confused then why the slot is created, but IDK. If there are other uses for querying the available replication slots, maybe the protocol could be extended to that end for 11. Finally, it is a bit inconsitent that we'd report the creation of the permanent slot, but not the temporary one. I took a look and it seems the main reason why ReceiveXlogStream() does not call streamutil,c's CreateReplicationSlot() seems to be that the latter has no notion of temporary slots yet. I'm attaching a second patch which refactors things a bit more, adding a `is_temporary' argument to CreateReplicationSlot() and moving the creation of the temporary slot to pg_basebackup.c's StartLogStreamer() as well (as pg_recvlogical and pg_receivewal do not deal with temp slots anyway). This way, the creation of the temporary slot can be mention on --verbose as well. Personally, I think it'd be good to be able to have the --slot option just work in 10, so if the first patch is still acceptable (pending review) for 10 but not the second, that'd be entirely fine. Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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 >From 835e6fe3e30d534bc5918d1d6c399074a9a13626 Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Sun, 19 Mar 2017 10:58:13 +0100 Subject: [PATCH 1/2] Create replication slot in pg_basebackup if requested and not yet present. If a replication slot is explicitly requested, try to create it before starting to replicate from it. --- src/bin/pg_basebackup/pg_basebackup.c| 15 +++ src/bin/pg_basebackup/t/010_pg_basebackup.pl | 19 --- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 0a4944d..69ca4be 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg
Re: [HACKERS] Create replication slot in pg_basebackup if requested and not yet present
On Sun, Mar 19, 2017 at 11:21 AM, Michael Banck wrote: > Hi, > > with the default configuration improvements so far for 10, it seems to > be very easy to setup streaming replication (at least locally): > > $ initdb --pgdata=data1 > $ pg_ctl --pgdata=data1 start > $ pg_basebackup --pgdata=data2 --write-recovery-conf > $ sed -i -e 's/^#port.=.5432/port = 5433/' \ > > -e 's/^#hot_standby.=.off/hot_standby = on/' \ > > data2/postgresql.conf > $ pg_ctl --pgdata=data2 start > > (there might be a case for having hot_standby=on by default, but I think > that got discussed elsewhere and is anyway a different thread). > > However, the above does not use replication slots, and if you want to do > so, you get: > > $ pg_basebackup --pgdata=data2 --write-recovery-conf --slot=pg2 > 2017-03-19 11:04:37.978 CET [25362] ERROR: replication slot "pg2" does > not exist > pg_basebackup: could not send replication command "START_REPLICATION": > ERROR: replication slot "pg2" does not exist > pg_basebackup: child process exited with error 1 > pg_basebackup: removing data directory "data2" > > The error message is clear enough, but I wonder whether people will > start writing streaming replication tutorials just glossing over this > because it's one more step to run CREATE_REPLICATION_SLOT manually. > > So I propose the attached tiny patch to just create the slot (if it does > not exist already) in pg_basebackup, somewhat similar to what > pg_receivewal does, albeit unconditionally, if the user explicitly > requested a slot: > > $ pg_basebackup --pgdata=data2 --write-recovery-conf --slot=pg2 > $ echo $? > 0 > $ psql -c "DROP_REPLICATION_SLOT pg2" "host=127.0.0.1 port=5432 > replication=database user=mba dbname=postgres" > SELECT > $ > > This would get us somewhat closer to near zero-config replication, in my > opinion. Pardon me if that was discussed already and shot down > > Comments? > I think this is a good idea, since it makes replication slots easier to use. The change to use temporary slots was good for backups, but didn't help people setting up replicas. I've been annoyed for a while we didn't have a "create slot" mode in pg_basebackup, but doing it integrated like this is definitely a step even better than that. I think maybe we should output a message when the slot is created, at least in verbose mode, to make sure people realize that happened. Does that seem reasonable? -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
[HACKERS] Create replication slot in pg_basebackup if requested and not yet present
Hi, with the default configuration improvements so far for 10, it seems to be very easy to setup streaming replication (at least locally): $ initdb --pgdata=data1 $ pg_ctl --pgdata=data1 start $ pg_basebackup --pgdata=data2 --write-recovery-conf $ sed -i -e 's/^#port.=.5432/port = 5433/' \ > -e 's/^#hot_standby.=.off/hot_standby = on/' \ > data2/postgresql.conf $ pg_ctl --pgdata=data2 start (there might be a case for having hot_standby=on by default, but I think that got discussed elsewhere and is anyway a different thread). However, the above does not use replication slots, and if you want to do so, you get: $ pg_basebackup --pgdata=data2 --write-recovery-conf --slot=pg2 2017-03-19 11:04:37.978 CET [25362] ERROR: replication slot "pg2" does not exist pg_basebackup: could not send replication command "START_REPLICATION": ERROR: replication slot "pg2" does not exist pg_basebackup: child process exited with error 1 pg_basebackup: removing data directory "data2" The error message is clear enough, but I wonder whether people will start writing streaming replication tutorials just glossing over this because it's one more step to run CREATE_REPLICATION_SLOT manually. So I propose the attached tiny patch to just create the slot (if it does not exist already) in pg_basebackup, somewhat similar to what pg_receivewal does, albeit unconditionally, if the user explicitly requested a slot: $ pg_basebackup --pgdata=data2 --write-recovery-conf --slot=pg2 $ echo $? 0 $ psql -c "DROP_REPLICATION_SLOT pg2" "host=127.0.0.1 port=5432 replication=database user=mba dbname=postgres" SELECT $ This would get us somewhat closer to near zero-config replication, in my opinion. Pardon me if that was discussed already and shot down Comments? Michael -- Michael Banck Projektleiter / Senior Berater Tel.: +49 2166 9901-171 Fax: +49 2166 9901-100 Email: michael.ba...@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 From 8a52b7d8cc6f956fba47465d280581e200ec5239 Mon Sep 17 00:00:00 2001 From: Michael Banck Date: Sun, 19 Mar 2017 10:58:13 +0100 Subject: [PATCH] Create replication slot in pg_basebackup if requested and not yet present. If a replication slot is explicitly requested, try to create it before starting to replicate from it. --- src/bin/pg_basebackup/receivelog.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/src/bin/pg_basebackup/receivelog.c b/src/bin/pg_basebackup/receivelog.c index f415135..0ddb7a6 100644 --- a/src/bin/pg_basebackup/receivelog.c +++ b/src/bin/pg_basebackup/receivelog.c @@ -527,6 +527,17 @@ ReceiveXlogStream(PGconn *conn, StreamCtl *stream) } /* + * 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; + } + + /* * initialize flush position to starting point, it's the caller's * responsibility that that's sane. */ -- 2.1.4 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers