Re: [HACKERS] Including replication slot data in base backups

2014-04-14 Thread Fujii Masao
On Thu, Apr 10, 2014 at 12:36 AM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Apr 8, 2014 at 3:08 AM, Michael Paquier
 michael.paqu...@gmail.com wrote:
 On Tue, Apr 8, 2014 at 1:18 AM, Robert Haas robertmh...@gmail.com wrote:
 Not sure if this is exactly the right way to do it, but I agree that
 something along those lines is a good idea.  I also think, maybe even
 importantly, that we should probably document that people using
 file-copy based hot backups should strongly consider removing the
 replication slots by hand before using the backup.
 Good point. Something here would be adapted in this case:
 http://www.postgresql.org/docs/devel/static/backup-file.html
 I am attaching an updated patch as well.

 What you've got here doesn't look like the right section to update -
 the section you've updated is on taking a cold backup.  The section I
 think you want to update is Making a Base Backup Using the Low Level
 API, and specifically this part:

 You can, however, omit from the backup dump the files within the
 cluster's pg_xlog/ subdirectory. This slight adjustment is worthwhile
 because it reduces the risk of mistakes when restoring. This is easy
 to arrange if pg_xlog/ is a symbolic link pointing to someplace
 outside the cluster directory, which is a common setup anyway for
 performance reasons. You might also want to exclude postmaster.pid and
 postmaster.opts, which record information about the running
 postmaster, not about the postmaster which will eventually use this
 backup. (These files can confuse pg_ctl.)

 What I'd propose is adding a second paragraph like this:

 It is often a good idea to also omit from the backup dump the files
 within the cluster's pg_replslot/ directory, so that replication slots
 that exist on the master do not become part of the backup.  Otherwise,
 the subsequent use of the backup to create a standby may result in
 indefinite retention of WAL files on the standby, and possibly bloat
 on the master if hot standby feedback is enabled, because the clients
 that are using those replication slots will still be connecting to and
 updating the slots on the master, not the standby.  Even if the backup
 is only intended for use in creating a new master, copying the
 replication slots isn't expected to be particularly useful, since the
 contents of those slots will likely be badly out of date by the time
 the new master comes on line.

This makes me think that it's safer to just remove replication slot files
at the beginning of the recovery when both backup_label and recovery.conf
exist.

Regards,

-- 
Fujii Masao


-- 
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] Including replication slot data in base backups

2014-04-14 Thread Robert Haas
On Mon, Apr 14, 2014 at 9:26 AM, Fujii Masao masao.fu...@gmail.com wrote:
 This makes me think that it's safer to just remove replication slot files
 at the beginning of the recovery when both backup_label and recovery.conf
 exist.

Well, we could do that, but that would preempt anyone who *does* want
to keep those replication slots around.  I'm not sure that's a good
idea, either.

-- 
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] Including replication slot data in base backups

2014-04-14 Thread Michael Paquier
On Tue, Apr 15, 2014 at 1:55 AM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Apr 14, 2014 at 9:26 AM, Fujii Masao masao.fu...@gmail.com wrote:
 This makes me think that it's safer to just remove replication slot files
 at the beginning of the recovery when both backup_label and recovery.conf
 exist.

 Well, we could do that, but that would preempt anyone who *does* want
 to keep those replication slots around.  I'm not sure that's a good
 idea, either.
Same here, I still see cases where people might want to keep the
replication slot information around, particularly if they take an
atomic snapshot of PGDATA, which is something that some of our users
do. Enforcing removal of such files when recovery begins would only
bring less flexibility to the way recovery can be done.
-- 
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] Including replication slot data in base backups

2014-04-13 Thread Michael Paquier
On Thu, Apr 10, 2014 at 12:36 AM, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Apr 8, 2014 at 3:08 AM, Michael Paquier
 michael.paqu...@gmail.com wrote:
 On Tue, Apr 8, 2014 at 1:18 AM, Robert Haas robertmh...@gmail.com wrote:
 Not sure if this is exactly the right way to do it, but I agree that
 something along those lines is a good idea.  I also think, maybe even
 importantly, that we should probably document that people using
 file-copy based hot backups should strongly consider removing the
 replication slots by hand before using the backup.
 Good point. Something here would be adapted in this case:
 http://www.postgresql.org/docs/devel/static/backup-file.html
 I am attaching an updated patch as well.

 What you've got here doesn't look like the right section to update -
 the section you've updated is on taking a cold backup.  The section I
 think you want to update is Making a Base Backup Using the Low Level
 API, and specifically this part:

 You can, however, omit from the backup dump the files within the
 cluster's pg_xlog/ subdirectory. This slight adjustment is worthwhile
 because it reduces the risk of mistakes when restoring. This is easy
 to arrange if pg_xlog/ is a symbolic link pointing to someplace
 outside the cluster directory, which is a common setup anyway for
 performance reasons. You might also want to exclude postmaster.pid and
 postmaster.opts, which record information about the running
 postmaster, not about the postmaster which will eventually use this
 backup. (These files can confuse pg_ctl.)

 What I'd propose is adding a second paragraph like this:

 It is often a good idea to also omit from the backup dump the files
 within the cluster's pg_replslot/ directory, so that replication slots
 that exist on the master do not become part of the backup.  Otherwise,
 the subsequent use of the backup to create a standby may result in
 indefinite retention of WAL files on the standby, and possibly bloat
 on the master if hot standby feedback is enabled, because the clients
 that are using those replication slots will still be connecting to and
 updating the slots on the master, not the standby.  Even if the backup
 is only intended for use in creating a new master, copying the
 replication slots isn't expected to be particularly useful, since the
 contents of those slots will likely be badly out of date by the time
 the new master comes on line.
Yes, that's far better than what I sent. Thanks!
-- 
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] Including replication slot data in base backups

2014-04-09 Thread Robert Haas
On Tue, Apr 8, 2014 at 3:08 AM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Tue, Apr 8, 2014 at 1:18 AM, Robert Haas robertmh...@gmail.com wrote:
 Not sure if this is exactly the right way to do it, but I agree that
 something along those lines is a good idea.  I also think, maybe even
 importantly, that we should probably document that people using
 file-copy based hot backups should strongly consider removing the
 replication slots by hand before using the backup.
 Good point. Something here would be adapted in this case:
 http://www.postgresql.org/docs/devel/static/backup-file.html
 I am attaching an updated patch as well.

What you've got here doesn't look like the right section to update -
the section you've updated is on taking a cold backup.  The section I
think you want to update is Making a Base Backup Using the Low Level
API, and specifically this part:

You can, however, omit from the backup dump the files within the
cluster's pg_xlog/ subdirectory. This slight adjustment is worthwhile
because it reduces the risk of mistakes when restoring. This is easy
to arrange if pg_xlog/ is a symbolic link pointing to someplace
outside the cluster directory, which is a common setup anyway for
performance reasons. You might also want to exclude postmaster.pid and
postmaster.opts, which record information about the running
postmaster, not about the postmaster which will eventually use this
backup. (These files can confuse pg_ctl.)

What I'd propose is adding a second paragraph like this:

It is often a good idea to also omit from the backup dump the files
within the cluster's pg_replslot/ directory, so that replication slots
that exist on the master do not become part of the backup.  Otherwise,
the subsequent use of the backup to create a standby may result in
indefinite retention of WAL files on the standby, and possibly bloat
on the master if hot standby feedback is enabled, because the clients
that are using those replication slots will still be connecting to and
updating the slots on the master, not the standby.  Even if the backup
is only intended for use in creating a new master, copying the
replication slots isn't expected to be particularly useful, since the
contents of those slots will likely be badly out of date by the time
the new master comes on line.

-- 
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] Including replication slot data in base backups

2014-04-08 Thread Michael Paquier
On Tue, Apr 8, 2014 at 1:18 AM, Robert Haas robertmh...@gmail.com wrote:
 Not sure if this is exactly the right way to do it, but I agree that
 something along those lines is a good idea.  I also think, maybe even
 importantly, that we should probably document that people using
 file-copy based hot backups should strongly consider removing the
 replication slots by hand before using the backup.
Good point. Something here would be adapted in this case:
http://www.postgresql.org/docs/devel/static/backup-file.html
I am attaching an updated patch as well.
-- 
Michael
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 854b5fd..d8286b0 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -448,6 +448,13 @@ tar -cf backup.tar /usr/local/pgsql/data
the contents of indexes for example, just the commands to recreate
them.)  However, taking a file system backup might be faster.
   /para
+
+  para
+   When doing a file system backup, it is recommended to drop replication
+   slots (see xref linkend=streaming-replication-slots) before using
+   it as it is not guaranteed that the WAL files needed by a slot will be
+   kept on the newly-created node.
+  /para
  /sect1
 
  sect1 id=continuous-archiving
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml 
b/doc/src/sgml/ref/pg_basebackup.sgml
index 6ce0c8c..b81ad8d 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -590,6 +590,13 @@ PostgreSQL documentation
or an older major version, down to 9.1. However, WAL streaming mode (-X
stream) only works with server version 9.3.
   /para
+
+  para
+   The backup will not include information about replication slots
+   (see xref linkend=streaming-replication-slots) as it is not
+   guaranteed that a node in recovery will have WAL files required for
+   a given slot.
+  /para
  /refsect1
 
  refsect1

-- 
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] Including replication slot data in base backups

2014-04-07 Thread Michael Paquier
On Fri, Apr 4, 2014 at 9:57 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
 For 9.4, clearly yes, this would change the semantic of recovery and
 this is not something wise to do at the end of a development cycle.
 For 9.5 though, this is a different story. It clearly depends on if
 this is though as useful enough to change how recovery fetches WAL
 files (in this case by scanning existing repslots). There are other
 things to consider as well like for example: do we reset the
 restart_lsn of a repslot if needed WAL files are not here anymore or
 abort recovery? I haven't worked much with repslots though...
Coming back to that, I am still wondering if for the time being it
would not be better to add in pg_basebackup documentation that
replication slot information is not added in a backup, per se the
patch attached.
Regards,
-- 
Michael
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml 
b/doc/src/sgml/ref/pg_basebackup.sgml
index 6ce0c8c..4305788 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -590,6 +590,13 @@ PostgreSQL documentation
or an older major version, down to 9.1. However, WAL streaming mode (-X
stream) only works with server version 9.3.
   /para
+
+  para
+   The backup will not include information about replication slots
+   (see xref linkend=streaming-replication-slots) as it is not
+   guaranteed that a node in recovery will have WAL files required for
+   a given slot.
+  /para
  /refsect1
 
  refsect1

-- 
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] Including replication slot data in base backups

2014-04-07 Thread Robert Haas
On Mon, Apr 7, 2014 at 3:04 AM, Michael Paquier
michael.paqu...@gmail.com wrote:
 On Fri, Apr 4, 2014 at 9:57 PM, Michael Paquier
 michael.paqu...@gmail.com wrote:
 For 9.4, clearly yes, this would change the semantic of recovery and
 this is not something wise to do at the end of a development cycle.
 For 9.5 though, this is a different story. It clearly depends on if
 this is though as useful enough to change how recovery fetches WAL
 files (in this case by scanning existing repslots). There are other
 things to consider as well like for example: do we reset the
 restart_lsn of a repslot if needed WAL files are not here anymore or
 abort recovery? I haven't worked much with repslots though...
 Coming back to that, I am still wondering if for the time being it
 would not be better to add in pg_basebackup documentation that
 replication slot information is not added in a backup, per se the
 patch attached.

Not sure if this is exactly the right way to do it, but I agree that
something along those lines is a good idea.  I also think, maybe even
importantly, that we should probably document that people using
file-copy based hot backups should strongly consider removing the
replication slots by hand before using the backup.

-- 
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] Including replication slot data in base backups

2014-04-07 Thread Fabrízio de Royes Mello
Em segunda-feira, 7 de abril de 2014, Robert Haas robertmh...@gmail.com
escreveu:

 On Mon, Apr 7, 2014 at 3:04 AM, Michael Paquier
 michael.paqu...@gmail.com javascript:; wrote:
  On Fri, Apr 4, 2014 at 9:57 PM, Michael Paquier
  michael.paqu...@gmail.com javascript:; wrote:
  For 9.4, clearly yes, this would change the semantic of recovery and
  this is not something wise to do at the end of a development cycle.
  For 9.5 though, this is a different story. It clearly depends on if
  this is though as useful enough to change how recovery fetches WAL
  files (in this case by scanning existing repslots). There are other
  things to consider as well like for example: do we reset the
  restart_lsn of a repslot if needed WAL files are not here anymore or
  abort recovery? I haven't worked much with repslots though...
  Coming back to that, I am still wondering if for the time being it
  would not be better to add in pg_basebackup documentation that
  replication slot information is not added in a backup, per se the
  patch attached.

 Not sure if this is exactly the right way to do it, but I agree that
 something along those lines is a good idea.  I also think, maybe even
 importantly, that we should probably document that people using
 file-copy based hot backups should strongly consider removing the
 replication slots by hand before using the backup.


+1

Fabrízio


-- 
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
 Timbira: http://www.timbira.com.br
 Blog sobre TI: http://fabriziomello.blogspot.com
 Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
 Twitter: http://twitter.com/fabriziomello


Re: [HACKERS] Including replication slot data in base backups

2014-04-04 Thread Michael Paquier
On Wed, Apr 2, 2014 at 10:27 PM, Andres Freund and...@2ndquadrant.com wrote:
 The new master won't necessarily have all the neccessary WAL available, no?
No, they won't have it, and things begin to get bad once a base backup
includes a slot that has a non-null value of restart_lsn. I imagine
that if we want to guarantee the correctness of a replication slot we
would need to fetch from archives the necessary WAL files needed for
it when a node is in recovery, which is not something that this patch
does...
-- 
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] Including replication slot data in base backups

2014-04-04 Thread Andres Freund
On 2014-04-04 21:42:33 +0900, Michael Paquier wrote:
 On Wed, Apr 2, 2014 at 10:27 PM, Andres Freund and...@2ndquadrant.com wrote:
  The new master won't necessarily have all the neccessary WAL available, no?

 No, they won't have it, and things begin to get bad once a base backup
 includes a slot that has a non-null value of restart_lsn.

In other words, every slot that has been used.

 I imagine
 that if we want to guarantee the correctness of a replication slot we
 would need to fetch from archives the necessary WAL files needed for
 it when a node is in recovery, which is not something that this patch
 does...

Does that mean you retract the patch?

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, 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] Including replication slot data in base backups

2014-04-04 Thread Michael Paquier
On Fri, Apr 4, 2014 at 9:44 PM, Andres Freund and...@2ndquadrant.com wrote:
 I imagine
 that if we want to guarantee the correctness of a replication slot we
 would need to fetch from archives the necessary WAL files needed for
 it when a node is in recovery, which is not something that this patch
 does...

 Does that mean you retract the patch?
For 9.4, clearly yes, this would change the semantic of recovery and
this is not something wise to do at the end of a development cycle.
For 9.5 though, this is a different story. It clearly depends on if
this is though as useful enough to change how recovery fetches WAL
files (in this case by scanning existing repslots). There are other
things to consider as well like for example: do we reset the
restart_lsn of a repslot if needed WAL files are not here anymore or
abort recovery? I haven't worked much with repslots though...
-- 
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] Including replication slot data in base backups

2014-04-02 Thread Bernd Helmle



--On 1. April 2014 11:26:08 -0400 Robert Haas robertmh...@gmail.com wrote:



As a general comment, I think that replication slots, while a great
feature, have more than the usual potential for self-inflicted injury.
 A replication slot prevents the global xmin from advancing (so your
tables will bloat) and WAL from being removed (so your pg_xlog
directory will fill up and take down the server).  The very last thing
you want to do is to keep around a replication slot that should have
been dropped, and I suspect a decent number of users are going to make
that mistake, just as they do with prepared transactions and backends
left idle in transaction.


Oh yes, i saw this happening uncountless times now by customers when 
restoring a basebackup with in-progress prepared xacts (and was indeed 
fooled myself a few times, too). I always was under the impression that 
there should be a big big warning at least in the logs to hint the user to 
check any remaining prepared xacts...


--
Thanks

Bernd


--
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] Including replication slot data in base backups

2014-04-02 Thread Andres Freund
On 2014-04-02 09:59:28 +0900, Michael Paquier wrote:
 On Tue, Apr 1, 2014 at 11:59 PM, Andres Freund and...@2ndquadrant.com wrote:
  On 2014-04-01 16:45:46 +0200, Magnus Hagander wrote:
  On Tue, Apr 1, 2014 at 2:24 PM, Michael Paquier
  michael.paqu...@gmail.comwrote:
   As of now, pg_basebackup creates an empty repository for pg_replslot/
   in a base backup, forcing the user to recreate slots on other nodes of
   the cluster with pg_create_*_replication_slot, or copy pg_replslot
   from another node. This is not really user-friendly especially after a
   failover where a given slave may not have the replication slot
   information of the master that it is replacing.
 
  What exactly is your use case for copying the slots?

 I had in mind users that want to keep around base backups that could
 be used for recovery operations like PITR using a base backup and
 archives. It does not apply directly to a live standby, as it would
 mean that this standby would be defined to retain WAL for other slaves
 connected to the master.

I honestly can't follow why that implies copying the slots?

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, 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] Including replication slot data in base backups

2014-04-02 Thread Michael Paquier
On Wed, Apr 2, 2014 at 6:58 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-04-02 09:59:28 +0900, Michael Paquier wrote:
 On Tue, Apr 1, 2014 at 11:59 PM, Andres Freund and...@2ndquadrant.com 
 wrote:
  On 2014-04-01 16:45:46 +0200, Magnus Hagander wrote:
  On Tue, Apr 1, 2014 at 2:24 PM, Michael Paquier
  michael.paqu...@gmail.comwrote:
   As of now, pg_basebackup creates an empty repository for pg_replslot/
   in a base backup, forcing the user to recreate slots on other nodes of
   the cluster with pg_create_*_replication_slot, or copy pg_replslot
   from another node. This is not really user-friendly especially after a
   failover where a given slave may not have the replication slot
   information of the master that it is replacing.
 
  What exactly is your use case for copying the slots?

 I had in mind users that want to keep around base backups that could
 be used for recovery operations like PITR using a base backup and
 archives. It does not apply directly to a live standby, as it would
 mean that this standby would be defined to retain WAL for other slaves
 connected to the master.

 I honestly can't follow why that implies copying the slots?
You simply do not need to recreate manually the slots on the new master.
-- 
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] Including replication slot data in base backups

2014-04-02 Thread Andres Freund
On 2014-04-02 20:59:03 +0900, Michael Paquier wrote:
 On Wed, Apr 2, 2014 at 6:58 PM, Andres Freund and...@2ndquadrant.com wrote:
  On 2014-04-02 09:59:28 +0900, Michael Paquier wrote:
  On Tue, Apr 1, 2014 at 11:59 PM, Andres Freund and...@2ndquadrant.com 
  wrote:
   On 2014-04-01 16:45:46 +0200, Magnus Hagander wrote:
   On Tue, Apr 1, 2014 at 2:24 PM, Michael Paquier
   michael.paqu...@gmail.comwrote:
As of now, pg_basebackup creates an empty repository for pg_replslot/
in a base backup, forcing the user to recreate slots on other nodes of
the cluster with pg_create_*_replication_slot, or copy pg_replslot
from another node. This is not really user-friendly especially after a
failover where a given slave may not have the replication slot
information of the master that it is replacing.
  
   What exactly is your use case for copying the slots?
 
  I had in mind users that want to keep around base backups that could
  be used for recovery operations like PITR using a base backup and
  archives. It does not apply directly to a live standby, as it would
  mean that this standby would be defined to retain WAL for other slaves
  connected to the master.
 
  I honestly can't follow why that implies copying the slots?
 You simply do not need to recreate manually the slots on the new master.

That doesn't seem like a good justification. The new master won't
necessarily have all the neccessary WAL available, no?

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, 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


[HACKERS] Including replication slot data in base backups

2014-04-01 Thread Michael Paquier
Hi all,

As of now, pg_basebackup creates an empty repository for pg_replslot/
in a base backup, forcing the user to recreate slots on other nodes of
the cluster with pg_create_*_replication_slot, or copy pg_replslot
from another node. This is not really user-friendly especially after a
failover where a given slave may not have the replication slot
information of the master that it is replacing.

The simple patch attached adds a new option in pg_basebackup, called
--replication-slot, allowing to include replication slot information
in a base backup. This is done by extending the command BASE_BACKUP in
the replication protocol.

As it is too late for 9.4, I would like to add it to the first commit
fest of 9.5. Comments are welcome.

Regards,
-- 
Michael
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 36d16a5..4748d08 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -1837,7 +1837,7 @@ The commands accepted in walsender mode are:
   /varlistentry
 
   varlistentry
-termBASE_BACKUP [literalLABEL/literal 
replaceable'label'/replaceable] [literalPROGRESS/literal] 
[literalFAST/literal] [literalWAL/literal] [literalNOWAIT/literal] 
[literalMAX_RATE/literal replaceablerate/replaceable]/term
+termBASE_BACKUP [literalLABEL/literal 
replaceable'label'/replaceable] [literalPROGRESS/literal] 
[literalFAST/literal] [literalWAL/literal] [literalNOWAIT/literal] 
[literalREPLICATION_SLOT/literal] [literalMAX_RATE/literal 
replaceablerate/replaceable]/term
 listitem
  para
   Instructs the server to start streaming a base backup.
@@ -1909,6 +1909,18 @@ The commands accepted in walsender mode are:
/varlistentry
 
varlistentry
+termliteralREPLICATION_SLOT/literal/term
+listitem
+ para
+  By default, the backup will not include the replication slot
+  information in filenamepg_replslot/ and is created as an
+  empty repository. Specifying literalREPLICATION_SLOT/literal
+  permits to includes replication slot information in the backup.
+ /para
+/listitem
+   /varlistentry
+
+   varlistentry
 termliteralMAX_RATE/literal replaceablerate//term
 listitem
  para
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml 
b/doc/src/sgml/ref/pg_basebackup.sgml
index 6ce0c8c..be77f7b 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -210,6 +210,17 @@ PostgreSQL documentation
  /varlistentry
 
  varlistentry
+  termoption--replication-slot/option/term
+  listitem
+   para
+Include replication slot information in the base backup. This is added
+in filenamepg_replslot/filename by default empty if this option is
+not specified.
+   /para
+  /listitem
+ /varlistentry
+
+ varlistentry
   termoption-R/option/term
   termoption--write-recovery-conf/option/term
   listitem
@@ -254,7 +265,7 @@ PostgreSQL documentation
   termoption--xlogdir=replaceable 
class=parameterxlogdir/replaceable/option/term
   listitem
para
-Specifies the location for the transaction log directory. 
+Specifies the location for the transaction log directory.
 replaceablexlogdir/replaceable must be an absolute path.
 The transaction log directory can only be specified when
 the backup is in plain mode.
diff --git a/src/backend/replication/basebackup.c 
b/src/backend/replication/basebackup.c
index f611f59..4a86117 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -45,6 +45,7 @@ typedef struct
boolfastcheckpoint;
boolnowait;
boolincludewal;
+   boolreplication_slot;
uint32  maxrate;
 } basebackup_options;
 
@@ -71,6 +72,9 @@ static bool backup_started_in_recovery = false;
 /* Relative path of temporary statistics directory */
 static char *statrelpath = NULL;
 
+/* Include replication slot data in base backup? */
+static bool include_replication_slots = false;
+
 /*
  * Size of each block sent into the tar stream for larger files.
  */
@@ -131,6 +135,7 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
datadirpathlen = strlen(DataDir);
 
backup_started_in_recovery = RecoveryInProgress();
+   include_replication_slots = opt-replication_slot;
 
startptr = do_pg_start_backup(opt-label, opt-fastcheckpoint, 
starttli,
  labelfile);
@@ -548,6 +553,7 @@ parse_basebackup_options(List *options, basebackup_options 
*opt)
boolo_nowait = false;
boolo_wal = false;
boolo_maxrate = false;
+   boolo_replication_slot = false;
 
MemSet(opt, 0, sizeof(*opt));
foreach(lopt, options)
@@ -618,6 +624,15 @@ 

Re: [HACKERS] Including replication slot data in base backups

2014-04-01 Thread Magnus Hagander
On Tue, Apr 1, 2014 at 2:24 PM, Michael Paquier
michael.paqu...@gmail.comwrote:

 Hi all,

 As of now, pg_basebackup creates an empty repository for pg_replslot/
 in a base backup, forcing the user to recreate slots on other nodes of
 the cluster with pg_create_*_replication_slot, or copy pg_replslot
 from another node. This is not really user-friendly especially after a
 failover where a given slave may not have the replication slot
 information of the master that it is replacing.

 The simple patch attached adds a new option in pg_basebackup, called
 --replication-slot, allowing to include replication slot information
 in a base backup. This is done by extending the command BASE_BACKUP in
 the replication protocol.


--replication-slots would be a better name (plural), or probably
--include-replication-slots. (and that comment also goes for the
BASE_BACKUP syntax and variables)


But. If you want to solve the failover case, doesn't that mean you need to
include it in the *replication* stream and not just the base backup?
Otherwise, what you're sending over might well be out of date set of slots
once the failover happens? What if the set of replication slots change
between the time of the basebackup and the failover?

As it is too late for 9.4, I would like to add it to the first commit
 fest of 9.5. Comments are welcome.


It's not too late to fix omissions in 9.4 features though, if we consider
it that. Which I think this could probably be considered as - if we think
the solution is the right one.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] Including replication slot data in base backups

2014-04-01 Thread Andres Freund
On 2014-04-01 16:45:46 +0200, Magnus Hagander wrote:
 On Tue, Apr 1, 2014 at 2:24 PM, Michael Paquier
 michael.paqu...@gmail.comwrote:
 
  Hi all,
 
  As of now, pg_basebackup creates an empty repository for pg_replslot/
  in a base backup, forcing the user to recreate slots on other nodes of
  the cluster with pg_create_*_replication_slot, or copy pg_replslot
  from another node. This is not really user-friendly especially after a
  failover where a given slave may not have the replication slot
  information of the master that it is replacing.

What exactly is your usecase for copying the slots?

  The simple patch attached adds a new option in pg_basebackup, called
  --replication-slot, allowing to include replication slot information
  in a base backup. This is done by extending the command BASE_BACKUP in
  the replication protocol.

 --replication-slots would be a better name (plural), or probably
 --include-replication-slots. (and that comment also goes for the
 BASE_BACKUP syntax and variables)

I vote for --include-replication-slots.

 But. If you want to solve the failover case, doesn't that mean you need to
 include it in the *replication* stream and not just the base backup?

They pretty fundamentally can't be in the replication stream - there's
no way to make that work with cascading setups and such.

 Otherwise, what you're sending over might well be out of date set of slots
 once the failover happens? What if the set of replication slots change
 between the time of the basebackup and the failover?

An out of date slot doesn't seem really harmful for the failover
case. All that will happen is that it will reserve too many
resources. That should be fine.

 It's not too late to fix omissions in 9.4 features though, if we consider
 it that. Which I think this could probably be considered as - if we think
 the solution is the right one.

Yea, I think adding this would be fine if we deem it necessary.

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, 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] Including replication slot data in base backups

2014-04-01 Thread Robert Haas
On Tue, Apr 1, 2014 at 10:45 AM, Magnus Hagander mag...@hagander.net wrote:
 On Tue, Apr 1, 2014 at 2:24 PM, Michael Paquier michael.paqu...@gmail.com
 wrote:

 As of now, pg_basebackup creates an empty repository for pg_replslot/
 in a base backup, forcing the user to recreate slots on other nodes of
 the cluster with pg_create_*_replication_slot, or copy pg_replslot
 from another node. This is not really user-friendly especially after a
 failover where a given slave may not have the replication slot
 information of the master that it is replacing.

 The simple patch attached adds a new option in pg_basebackup, called
 --replication-slot, allowing to include replication slot information
 in a base backup. This is done by extending the command BASE_BACKUP in
 the replication protocol.

 --replication-slots would be a better name (plural), or probably
 --include-replication-slots. (and that comment also goes for the BASE_BACKUP
 syntax and variables)

 But. If you want to solve the failover case, doesn't that mean you need to
 include it in the *replication* stream and not just the base backup?
 Otherwise, what you're sending over might well be out of date set of slots
 once the failover happens? What if the set of replication slots change
 between the time of the basebackup and the failover?

As a general comment, I think that replication slots, while a great
feature, have more than the usual potential for self-inflicted injury.
 A replication slot prevents the global xmin from advancing (so your
tables will bloat) and WAL from being removed (so your pg_xlog
directory will fill up and take down the server).  The very last thing
you want to do is to keep around a replication slot that should have
been dropped, and I suspect a decent number of users are going to make
that mistake, just as they do with prepared transactions and backends
left idle in transaction.

So I view this proposal with a bit of skepticism for that reason.  If
you end up copying the replication slots when you didn't really want
to, or when you only wanted some of them, you will be sad.  In
particular, suppose you have a master and 2 standbys, each of which
has a replication slot.  The master fails; a standby is promoted.  If
the standby has the master's replication slots, that's wrong: perhaps
the OTHER standby's slot should stick around for the standby to
connect to, but the standby's OWN slot on the master shouldn't be kept
around.

It's also part of the idea here that a cascading standby should be
able to have its own slots for its downstream standbys.  It should
retain WAL locally for those standbys, but it should NOT retain WAL
for the master's other standbys.  This definitely doesn't work yet for
logical slots; I'm not sure about physical slots.  But it's part of
the plan, for sure.  Here again, copying the slots from the master is
the wrong thing.

Now, it would be great to have some more technology in this area.  It
would be pretty nifty if we could set things up so that the promotion
process could optionally assume and activate a configurable subset of
the master's slots at failover/switchover time - but the administrator
would need to also make sure those machines were going to reconnect to
the new master.  Or maybe we could find a way to automate that, too,
but either way I think we're going to need something a *lot* more
sophisticated than just copying all the slots.

-- 
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] Including replication slot data in base backups

2014-04-01 Thread Josh Berkus
On 04/01/2014 05:24 AM, Michael Paquier wrote:
 Hi all,
 
 As of now, pg_basebackup creates an empty repository for pg_replslot/
 in a base backup, forcing the user to recreate slots on other nodes of
 the cluster with pg_create_*_replication_slot, or copy pg_replslot
 from another node. This is not really user-friendly especially after a
 failover where a given slave may not have the replication slot
 information of the master that it is replacing.
 
 The simple patch attached adds a new option in pg_basebackup, called
 --replication-slot, allowing to include replication slot information
 in a base backup. This is done by extending the command BASE_BACKUP in
 the replication protocol.
 
 As it is too late for 9.4, I would like to add it to the first commit
 fest of 9.5. Comments are welcome.

Assuming it works, this seems like something we need to fix for 9.4.  No?


-- 
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com


-- 
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] Including replication slot data in base backups

2014-04-01 Thread Michael Paquier
On Tue, Apr 1, 2014 at 11:59 PM, Andres Freund and...@2ndquadrant.com wrote:
 On 2014-04-01 16:45:46 +0200, Magnus Hagander wrote:
 On Tue, Apr 1, 2014 at 2:24 PM, Michael Paquier
 michael.paqu...@gmail.comwrote:
  As of now, pg_basebackup creates an empty repository for pg_replslot/
  in a base backup, forcing the user to recreate slots on other nodes of
  the cluster with pg_create_*_replication_slot, or copy pg_replslot
  from another node. This is not really user-friendly especially after a
  failover where a given slave may not have the replication slot
  information of the master that it is replacing.

 What exactly is your use case for copying the slots?
I had in mind users that want to keep around base backups that could
be used for recovery operations like PITR using a base backup and
archives. It does not apply directly to a live standby, as it would
mean that this standby would be defined to retain WAL for other slaves
connected to the master.
Regards
-- 
Michael


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