Re: base backup client as auxiliary backend process
On 2020-Jan-14, Peter Eisentraut wrote: > On 2020-01-14 07:32, Masahiko Sawada wrote: > > - Replication slot name used by this WAL receiver > > + > > + Replication slot name used by this WAL receiver. This is only set > > if a > > + permanent replication slot is set using > + linkend="guc-primary-slot-name"/>. Otherwise, the WAL receiver may > > use > > + a temporary replication slot (determined by > + linkend="guc-wal-receiver-create-temp-slot"/>), but these are not > > shown > > + here. > > + > > > > Now that the slot name is shown even if it's a temp slot the above > > documentation changes needs to be changed. Other changes look good to > > me. > > committed, thanks Sergei has just proposed a change in semantics: if primary_slot_name is specified as well as wal_receiver_create_temp_slot, then a temp slot is used and it uses the specified name, instead of ignoring the temp-slot option as currently. Patch is at https://postgr.es/m/3109511585392...@myt6-887fb48a9c29.qloud-c.yandex.net (To clarify: the current semantics if both options are set is that an existing permanent slot is sought with the given name, and an error is raised if it doesn't exist.) What do you think? Preliminarly I think the proposed semantics are saner. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
I have set this patch to "returned with feedback" in the upcoming commit fest, because I will not be able to finish it. Unsurprisingly, the sequencing of startup actions in postmaster.c is extremely tricky and needs more thinking. All the rest worked pretty well, I thought. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
On 2020-02-17 18:42, Peter Eisentraut wrote: On 2020-02-03 13:47, Andres Freund wrote: Comment: - It'd be good to split out the feature independent refactorings, like the introduction of InitControlFile(), into their own commit. Right now it's hard to separate out what should just should be moved code, and what should be behavioural changes. Especially when there's stuff like just reindenting comments as part of the patch. Agreed. Here are three refactoring patches extracted that seem useful on their own. These have been committed. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
On 2020-02-03 13:47, Andres Freund wrote:
Comment:
- It'd be good to split out the feature independent refactorings, like
the introduction of InitControlFile(), into their own commit. Right
now it's hard to separate out what should just should be moved code,
and what should be behavioural changes. Especially when there's stuff
like just reindenting comments as part of the patch.
Agreed. Here are three refactoring patches extracted that seem useful
on their own.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 49b51b362b4d86103c74057186154a42b9ef335b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 17 Feb 2020 17:35:48 +0100
Subject: [PATCH 1/3] pg_resetwal: Rename function to avoid potential conflict
ReadControlFile() here conflicts with a function of the same name in
xlog.c. There is no actual conflict right now, but since
pg_resetwal.c reaches deep inside backend headers, it's possible in
the future.
---
src/bin/pg_resetwal/pg_resetwal.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/bin/pg_resetwal/pg_resetwal.c
b/src/bin/pg_resetwal/pg_resetwal.c
index f9cfeae264..c9edeb54d3 100644
--- a/src/bin/pg_resetwal/pg_resetwal.c
+++ b/src/bin/pg_resetwal/pg_resetwal.c
@@ -76,7 +76,7 @@ static intWalSegSz;
static int set_wal_segsize;
static void CheckDataVersion(void);
-static bool ReadControlFile(void);
+static bool read_controlfile(void);
static void GuessControlValues(void);
static void PrintControlValues(bool guessed);
static void PrintNewControlValues(void);
@@ -393,7 +393,7 @@ main(int argc, char *argv[])
/*
* Attempt to read the existing pg_control file
*/
- if (!ReadControlFile())
+ if (!read_controlfile())
GuessControlValues();
/*
@@ -578,7 +578,7 @@ CheckDataVersion(void)
* to the current format. (Currently we don't do anything of the sort.)
*/
static bool
-ReadControlFile(void)
+read_controlfile(void)
{
int fd;
int len;
--
2.25.0
From 91c816533a9e79623576a9303b2a793ee713eaed Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 17 Feb 2020 17:46:37 +0100
Subject: [PATCH 2/3] Reformat code comment
---
src/backend/access/transam/xlog.c | 21 +++--
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 3813eadfb4..b017fd286f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6297,16 +6297,17 @@ StartupXLOG(void)
/*--
* If we previously crashed, perform a couple of actions:
-* - The pg_wal directory may still include some temporary WAL
segments
-* used when creating a new segment, so perform some clean up to not
-* bloat this path. This is done first as there is no point to sync
this
-* temporary data.
-* - There might be data which we had written, intending to fsync
it,
-* but which we had not actually fsync'd yet. Therefore, a power failure
-* in the near future might cause earlier unflushed writes to be lost,
-* even though more recent data written to disk from here on would be
-* persisted. To avoid that, fsync the entire data directory.
-*-
+*
+* - The pg_wal directory may still include some temporary WAL segments
+* used when creating a new segment, so perform some clean up to not
+* bloat this path. This is done first as there is no point to sync
+* this temporary data.
+*
+* - There might be data which we had written, intending to fsync it,
but
+* which we had not actually fsync'd yet. Therefore, a power failure
in
+* the near future might cause earlier unflushed writes to be lost,
even
+* though more recent data written to disk from here on would be
+* persisted. To avoid that, fsync the entire data directory.
*/
if (ControlFile->state != DB_SHUTDOWNED &&
ControlFile->state != DB_SHUTDOWNED_IN_RECOVERY)
--
2.25.0
From 0cebb19f719f1894140f13739f4a1851a929186d Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 17 Feb 2020 17:58:02 +0100
Subject: [PATCH 3/3] Factor out InitControlFile() from BootStrapXLOG()
Right now this only makes BootStrapXLOG() a bit more manageable, but
in the future there may be external callers.
---
src/backend/access/transam/xlog.c | 70 +--
1 file changed, 39 insertions(+), 31 deletions(-)
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index b017fd286f..fd527f20c5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -903,6 +903,7 @@ static void
Re: base backup client as auxiliary backend process
On Mon, 3 Feb 2020 at 20:06, Andres Freund wrote: > > Hi, > > On 2020-01-11 10:52:30 +0100, Peter Eisentraut wrote: > > On 2020-01-10 04:32, Masahiko Sawada wrote: > > > I agreed that these patches are useful on its own and 0001 patch and > > > > committed 0001 > > over on -committers Robert complained: > > On 2020-01-23 15:49:37 -0500, Robert Haas wrote: > > On Tue, Jan 14, 2020 at 8:57 AM Peter Eisentraut > > wrote: > > > walreceiver uses a temporary replication slot by default > > > > > > If no permanent replication slot is configured using > > > primary_slot_name, the walreceiver now creates and uses a temporary > > > replication slot. A new setting wal_receiver_create_temp_slot can be > > > used to disable this behavior, for example, if the remote instance is > > > out of replication slots. > > > > > > Reviewed-by: Masahiko Sawada > > > Discussion: > > > https://www.postgresql.org/message-id/CA%2Bfd4k4dM0iEPLxyVyme2RAFsn8SUgrNtBJOu81YqTY4V%2BnqZA%40mail.gmail.com > > > > Neither the commit message for this patch nor any of the comments in > > the patch seem to explain why this is a desirable change. > > > > I assume that's probably discussed on the thread that is linked here, > > but you shouldn't have to dig through the discussion thread to figure > > out what the benefits of a change like this are. > > which I fully agree with. > > > It's not at all clear to me that the potential downsides of this have > been fully thought through. And even if they have, they've not been > documented. > > Previously if a standby without a slot was slow receiving WAL, > e.g. because the network bandwidth was insufficient, it'd at some point > just fail because the required WAL is removed. But with this patch that > won't happen - instead the primary will just run out of space. At the > very least this would need to add documentation of this caveat to a few > places. +1 to add downsides to the documentation. It might not normally happen but with this parameter we will need to have enough setting of max_replication_slots because the standby will fail to start after failover due to full of slots. WAL required by the standby could be removed on the primary due to the standby delaying much, for example when the standby stopped for a long time or when the standby is running but delayed for some reason. This feature prevents WAL from removal for the latter case. That is, we can ensure that required WAL is not removed during replication running. For the former case we can use a permanent replication slot. Although there is a risk of running out of space but I personally think this behavior is better for most cases. Regards, -- Masahiko Sawadahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
On Mon, Feb 03, 2020 at 01:37:25AM -0800, Andres Freund wrote: > On 2020-01-23 15:49:37 -0500, Robert Haas wrote: >> I assume that's probably discussed on the thread that is linked here, >> but you shouldn't have to dig through the discussion thread to figure >> out what the benefits of a change like this are. > > which I fully agree with. > > It's not at all clear to me that the potential downsides of this have > been fully thought through. And even if they have, they've not been > documented. There is this, and please let me add a reference to another complaint I had about this commit: https://www.postgresql.org/message-id/20200122055510.gh174...@paquier.xyz -- Michael signature.asc Description: PGP signature
Re: base backup client as auxiliary backend process
Hi,
Comment:
- It'd be good to split out the feature independent refactorings, like
the introduction of InitControlFile(), into their own commit. Right
now it's hard to separate out what should just should be moved code,
and what should be behavioural changes. Especially when there's stuff
like just reindenting comments as part of the patch.
> @@ -886,12 +891,27 @@ PostmasterMain(int argc, char *argv[])
> /* Verify that DataDir looks reasonable */
> checkDataDir();
>
> - /* Check that pg_control exists */
> - checkControlFile();
> -
> /* And switch working directory into it */
> ChangeToDataDir();
>
> + if (stat(BASEBACKUP_SIGNAL_FILE, _buf) == 0)
> + {
> + int fd;
> +
> + fd = BasicOpenFilePerm(STANDBY_SIGNAL_FILE, O_RDWR | PG_BINARY,
> +S_IRUSR | S_IWUSR);
> + if (fd >= 0)
> + {
> + (void) pg_fsync(fd);
> + close(fd);
> + }
> + basebackup_signal_file_found = true;
> + }
> +
> + /* Check that pg_control exists */
> + if (!basebackup_signal_file_found)
> + checkControlFile();
> +
This should be moved into its own function, rather than open coded in
PostmasterMain(). Imagine how PostmasterMain() would look if all the
check/initialization functions weren't extracted into functions.
> /*
>* Check for invalid combinations of GUC settings.
>*/
> @@ -970,7 +990,8 @@ PostmasterMain(int argc, char *argv[])
>* processes will inherit the correct function pointer and not need to
>* repeat the test.
>*/
> - LocalProcessControlFile(false);
> + if (!basebackup_signal_file_found)
> + LocalProcessControlFile(false);
>
> /*
>* Initialize SSL library, if specified.
> @@ -1386,6 +1407,39 @@ PostmasterMain(int argc, char *argv[])
>*/
> AddToDataDirLockFile(LOCK_FILE_LINE_PM_STATUS, PM_STATUS_STARTING);
>
> + if (basebackup_signal_file_found)
> + {
This imo *really* should be a separate function.
> + BaseBackupPID = StartBaseBackup();
> +
> + /*
> + * Wait until done. Start WAL receiver in the meantime, once
> base
> + * backup has received the starting position.
> + */
> + while (BaseBackupPID != 0)
> + {
> + PG_SETMASK();
> + pg_usleep(100L);
> + PG_SETMASK();
> + MaybeStartWalReceiver();
> + }
Is there seriously no better signalling that we can use than just
looping for a couple hours?
Is it actully guaranteed that a compiler wouldn't just load
BaseBackupPID into a register, and never see a change to it done in a
signal handler?
There should be a note mentioning that we'll just FATAL out if the base
backup process fails. Otherwise it's the obvious question reading this
code. Also - we have handling to restart WAL receiver, but there's no
handling for the base backup temporarily failing: Is that just because
its easy to do in one, but not the other case?
> + /*
> + * Reread the control file that came in with the base backup.
> + */
> + ReadControlFile();
> + }
Is it actualy rereading? I'm just reading the diff, so maybe I'm missing
something, but you've made LocalProcessControlFile not enter this code
path...
> @@ -2824,6 +2880,8 @@ pmdie(SIGNAL_ARGS)
>
> if (StartupPID != 0)
> signal_child(StartupPID, SIGTERM);
> + if (BaseBackupPID != 0)
> + signal_child(BaseBackupPID, SIGTERM);
> if (BgWriterPID != 0)
> signal_child(BgWriterPID, SIGTERM);
> if (WalReceiverPID != 0)
> @@ -3062,6 +3120,23 @@ reaper(SIGNAL_ARGS)
> continue;
> }
>
> + /*
> + * Was it the base backup process?
> + */
> + if (pid == BaseBackupPID)
> + {
> + BaseBackupPID = 0;
> + if (EXIT_STATUS_0(exitstatus))
> + ;
> + else if (EXIT_STATUS_1(exitstatus))
> + ereport(FATAL,
> + (errmsg("base backup failed")));
> + else
> + HandleChildCrash(pid, exitstatus,
> + _("base backup
> process"));
> + continue;
> + }
> +
What's the error handling for the case we shut down either because of
SIGTERM above, or here? Does all the code just deal with that the next
start? If not, what makes this safe?
> +/*
>
Re: base backup client as auxiliary backend process
Hi, On 2020-01-11 10:52:30 +0100, Peter Eisentraut wrote: > On 2020-01-10 04:32, Masahiko Sawada wrote: > > I agreed that these patches are useful on its own and 0001 patch and > > committed 0001 over on -committers Robert complained: On 2020-01-23 15:49:37 -0500, Robert Haas wrote: > On Tue, Jan 14, 2020 at 8:57 AM Peter Eisentraut wrote: > > walreceiver uses a temporary replication slot by default > > > > If no permanent replication slot is configured using > > primary_slot_name, the walreceiver now creates and uses a temporary > > replication slot. A new setting wal_receiver_create_temp_slot can be > > used to disable this behavior, for example, if the remote instance is > > out of replication slots. > > > > Reviewed-by: Masahiko Sawada > > Discussion: > > https://www.postgresql.org/message-id/CA%2Bfd4k4dM0iEPLxyVyme2RAFsn8SUgrNtBJOu81YqTY4V%2BnqZA%40mail.gmail.com > > Neither the commit message for this patch nor any of the comments in > the patch seem to explain why this is a desirable change. > > I assume that's probably discussed on the thread that is linked here, > but you shouldn't have to dig through the discussion thread to figure > out what the benefits of a change like this are. which I fully agree with. It's not at all clear to me that the potential downsides of this have been fully thought through. And even if they have, they've not been documented. Previously if a standby without a slot was slow receiving WAL, e.g. because the network bandwidth was insufficient, it'd at some point just fail because the required WAL is removed. But with this patch that won't happen - instead the primary will just run out of space. At the very least this would need to add documentation of this caveat to a few places. Perhaps that's worth doing anyway, because it's probably more common for a standby to just temporarily run behind - but given that this feature doesn't actually provide any robustness, due to e.g. the possibility of temporary disconnections or restarts, I'm not sure it's providing all that much compared to the dangers, for a feature on by default. Greetings, Andres Freund
Re: base backup client as auxiliary backend process
On Thu, 16 Jan 2020 at 00:17, Peter Eisentraut
wrote:
>
> On 2020-01-15 01:40, Masahiko Sawada wrote:
> > Could you rebase the main patch that adds base backup client as
> > auxiliary backend process since the previous version patch (v3)
> > conflicts with the current HEAD?
>
> attached
Thanks. I used and briefly looked at this patch. Here are some comments:
1.
+/*
+ * Wait until done. Start WAL receiver in the meantime, once base
+ * backup has received the starting position.
+ */
+while (BaseBackupPID != 0)
+{
+PG_SETMASK();
+pg_usleep(100L);
+PG_SETMASK();
+MaybeStartWalReceiver();
+}
Since the postmaster is sleeping the new connection hangs without any
message whereas normally we can get the message like "the database
system is starting up" during not accepting new connections. I think
some programs that checks the connectivity of PostgreSQL starting up
might not work fine with this. So many we might want to refuse all new
connections while waiting for taking basebackup.
2.
+initStringInfo();
+appendStringInfo(, "BASE_BACKUP PROGRESS NOWAIT EXCLUDE_CONF");
+if (cluster_name && cluster_name[0])
While using this patch I realized that the standby server cannot start
when the master server has larger value of some GUC parameter such as
max_connections and max_prepared_transactions than the default values.
And unlike taking basebackup using pg_basebacup or other methods the
database cluster initialized by this feature use default values for
all configuration parameters regardless of values in the master. So I
think it's better to include .conf files but we will end up with
overwriting the local .conf files instead. So I thought that
basebackup process can fetch .conf files from the master server and
add primary_conninfo to postgresql.auto.conf but I'm not sure.
3.
+if (stat(BASEBACKUP_SIGNAL_FILE, _buf) == 0)
+{
+int fd;
+
+fd = BasicOpenFilePerm(STANDBY_SIGNAL_FILE, O_RDWR | PG_BINARY,
+ S_IRUSR | S_IWUSR);
+if (fd >= 0)
+{
+(void) pg_fsync(fd);
+close(fd);
+}
+basebackup_signal_file_found = true;
+}
+
Why do we open and just close the file?
Regards,
--
Masahiko Sawadahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
On 2020-01-09 11:57, Alexandra Wang wrote: Back to the base backup stuff, I don't quite understand all the benefits you mentioned above. It seems to me the greatest benefit with this patch is that postmaster takes care of pg_basebackup itself, which reduces the human wait in between running the pg_basebackup and pg_ctl/postgres commands. Is that right? I personally don't mind the --write-recovery-conf option because it helps me write the primary_conninfo and primary_slot_name gucs into postgresql.auto.conf, which to me as a developer is easier than editing postgres.conf without automation. Sorry about the dumb question but what's so bad about --write-recovery-conf? Making it easier to automate is one major appeal of my proposal. The current way of setting up a standby is very difficult to automate correctly. Are you planning to completely replace pg_basebackup with this? Is there any use case that a user just need a basebackup but not immediately start the backend process? I'm not planning to replace or change pg_basebackup. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
On 2020-01-15 01:40, Masahiko Sawada wrote:
Could you rebase the main patch that adds base backup client as
auxiliary backend process since the previous version patch (v3)
conflicts with the current HEAD?
attached
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 892ba431956c7d936555f758efc874f58b3679e8 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Wed, 15 Jan 2020 16:15:06 +0100
Subject: [PATCH v4] Base backup client as auxiliary backend process
Discussion:
https://www.postgresql.org/message-id/flat/61b8d18d-c922-ac99-b990-a31ba63cd...@2ndquadrant.com
---
doc/src/sgml/protocol.sgml| 12 +-
doc/src/sgml/ref/initdb.sgml | 17 +
src/backend/access/transam/xlog.c | 102 +++--
src/backend/bootstrap/bootstrap.c | 9 +
src/backend/postmaster/pgstat.c | 6 +
src/backend/postmaster/postmaster.c | 114 -
src/backend/replication/basebackup.c | 70 +++
.../libpqwalreceiver/libpqwalreceiver.c | 400 ++
src/backend/replication/repl_gram.y | 9 +-
src/backend/replication/repl_scanner.l| 1 +
src/backend/storage/file/fd.c | 36 +-
src/bin/initdb/initdb.c | 39 +-
src/bin/pg_resetwal/pg_resetwal.c | 6 +-
src/include/access/xlog.h | 8 +-
src/include/miscadmin.h | 2 +
src/include/pgstat.h | 1 +
src/include/replication/basebackup.h | 2 +
src/include/replication/walreceiver.h | 4 +
src/include/storage/fd.h | 2 +-
src/include/utils/guc.h | 2 +-
src/test/recovery/t/018_basebackup.pl | 29 ++
21 files changed, 783 insertions(+), 88 deletions(-)
create mode 100644 src/test/recovery/t/018_basebackup.pl
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 80275215e0..f54b820edf 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2466,7 +2466,7 @@ Streaming Replication Protocol
-BASE_BACKUP [ LABEL
'label' ] [ PROGRESS ] [
FAST ] [ WAL ] [
NOWAIT ] [ MAX_RATE
rate ] [ TABLESPACE_MAP ] [
NOVERIFY_CHECKSUMS ]
+BASE_BACKUP [ LABEL
'label' ] [ PROGRESS ] [
FAST ] [ WAL ] [
NOWAIT ] [ MAX_RATE
rate ] [ TABLESPACE_MAP ] [
NOVERIFY_CHECKSUMS ] [ EXCLUDE_CONF ]
BASE_BACKUP
@@ -2576,6 +2576,16 @@ Streaming Replication Protocol
+
+
+EXCLUDE_CONF
+
+
+ Do not copy configuration files, that is, files that end in
+ .conf.
+
+
+
diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml
index da5c8f5307..1261e02d59 100644
--- a/doc/src/sgml/ref/initdb.sgml
+++ b/doc/src/sgml/ref/initdb.sgml
@@ -286,6 +286,23 @@ Options
+
+ -r
+ --replica
+
+
+Initialize a data directory for a physical replication replica. The
+data directory will not be initialized with a full database system,
+but will instead only contain a minimal set of files. A server that
+is started on this data directory will first fetch a base backup and
+then switch to standby mode. The connection information for the base
+backup has to be configured by setting , and other parameters as desired,
+before the server is started.
+
+
+
+
-S
--sync-only
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 7f4f784c0e..36c6cdef82 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -903,8 +903,6 @@ static void CheckRecoveryConsistency(void);
static XLogRecord *ReadCheckpointRecord(XLogReaderState *xlogreader,
XLogRecPtr RecPtr, int whichChkpt, bool report);
static bool rescanLatestTimeLine(void);
-static void WriteControlFile(void);
-static void ReadControlFile(void);
static char *str_time(pg_time_t tnow);
static bool CheckForStandbyTrigger(void);
@@ -4494,7 +4492,7 @@ rescanLatestTimeLine(void)
* ReadControlFile() verifies they are correct. We could split out the
* I/O and compatibility-check functions, but there seems no need currently.
*/
-static void
+void
WriteControlFile(void)
{
int fd;
@@ -4585,7 +4583,7 @@ WriteControlFile(void)
XLOG_CONTROL_FILE)));
}
-static void
+void
ReadControlFile(void)
{
pg_crc32c crc;
@@ -5075,6 +5073,41 @@ XLOGShmemInit(void)
InitSharedLatch(>recoveryWakeupLatch);
}
+void
+InitControlFile(uint64 sysidentifier)
+{
+ charmock_aut
Re: base backup client as auxiliary backend process
On Tue, 14 Jan 2020 at 22:58, Peter Eisentraut wrote: > > On 2020-01-14 07:32, Masahiko Sawada wrote: > > - Replication slot name used by this WAL receiver > > + > > + Replication slot name used by this WAL receiver. This is only set > > if a > > + permanent replication slot is set using > + linkend="guc-primary-slot-name"/>. Otherwise, the WAL receiver may > > use > > + a temporary replication slot (determined by > + linkend="guc-wal-receiver-create-temp-slot"/>), but these are not > > shown > > + here. > > + > > > > Now that the slot name is shown even if it's a temp slot the above > > documentation changes needs to be changed. Other changes look good to > > me. > > committed, thanks Thank you for committing these patches. Could you rebase the main patch that adds base backup client as auxiliary backend process since the previous version patch (v3) conflicts with the current HEAD? Regards, -- Masahiko Sawadahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
On 2020-01-14 07:32, Masahiko Sawada wrote: - Replication slot name used by this WAL receiver + + Replication slot name used by this WAL receiver. This is only set if a + permanent replication slot is set using . Otherwise, the WAL receiver may use + a temporary replication slot (determined by ), but these are not shown + here. + Now that the slot name is shown even if it's a temp slot the above documentation changes needs to be changed. Other changes look good to me. committed, thanks -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
On Sat, 11 Jan 2020 at 18:52, Peter Eisentraut wrote: > > On 2020-01-10 04:32, Masahiko Sawada wrote: > > I agreed that these patches are useful on its own and 0001 patch and > > committed 0001 > > > 0002 patch look good to me. For 0003 patch, > > > > + linkend="guc-primary-slot-name"/>. Otherwise, the WAL receiver may > > use > > + a temporary replication slot (determined by > + linkend="guc-wal-receiver-create-temp-slot"/>), but these are not > > shown > > + here. > > > > I think it's better to show the temporary slot name on > > pg_stat_wal_receiver view. Otherwise user would have no idea about > > what wal receiver is using the temporary slot. > > Makes sense. It makes the code a bit more fiddly, but it seems worth > it. New patches attached. Thank you for updating the patch! - Replication slot name used by this WAL receiver + + Replication slot name used by this WAL receiver. This is only set if a + permanent replication slot is set using . Otherwise, the WAL receiver may use + a temporary replication slot (determined by ), but these are not shown + here. + Now that the slot name is shown even if it's a temp slot the above documentation changes needs to be changed. Other changes look good to me. Regards, -- Masahiko Sawadahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
On 2020-01-10 04:32, Masahiko Sawada wrote:
I agreed that these patches are useful on its own and 0001 patch and
committed 0001
0002 patch look good to me. For 0003 patch,
+ linkend="guc-primary-slot-name"/>. Otherwise, the WAL receiver may use
+ a temporary replication slot (determined by ), but these are not shown
+ here.
I think it's better to show the temporary slot name on
pg_stat_wal_receiver view. Otherwise user would have no idea about
what wal receiver is using the temporary slot.
Makes sense. It makes the code a bit more fiddly, but it seems worth
it. New patches attached.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 2a089e6bd34b04e17f7b2918057d8e8eb04c117f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Sat, 11 Jan 2020 10:22:08 +0100
Subject: [PATCH v2 1/2] Expose PQbackendPID() through walreceiver API
This will be used by a subsequent patch.
---
.../replication/libpqwalreceiver/libpqwalreceiver.c | 11 +++
src/include/replication/walreceiver.h | 4
2 files changed, 15 insertions(+)
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 658af71fec..b731d3fd04 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -74,6 +74,7 @@ static char *libpqrcv_create_slot(WalReceiverConn *conn,
bool
temporary,
CRSSnapshotAction snapshot_action,
XLogRecPtr
*lsn);
+static pid_t libpqrcv_get_backend_pid(WalReceiverConn *conn);
static WalRcvExecResult *libpqrcv_exec(WalReceiverConn *conn,
const char *query,
const int nRetTypes,
@@ -93,6 +94,7 @@ static WalReceiverFunctionsType PQWalReceiverFunctions = {
libpqrcv_receive,
libpqrcv_send,
libpqrcv_create_slot,
+ libpqrcv_get_backend_pid,
libpqrcv_exec,
libpqrcv_disconnect
};
@@ -858,6 +860,15 @@ libpqrcv_create_slot(WalReceiverConn *conn, const char
*slotname,
return snapshot;
}
+/*
+ * Return PID of remote backend process.
+ */
+static pid_t
+libpqrcv_get_backend_pid(WalReceiverConn *conn)
+{
+ return PQbackendPID(conn->streamConn);
+}
+
/*
* Convert tuple query result to tuplestore.
*/
diff --git a/src/include/replication/walreceiver.h
b/src/include/replication/walreceiver.h
index a276237477..172cfa2862 100644
--- a/src/include/replication/walreceiver.h
+++ b/src/include/replication/walreceiver.h
@@ -226,6 +226,7 @@ typedef char *(*walrcv_create_slot_fn) (WalReceiverConn
*conn,
const char *slotname, bool temporary,
CRSSnapshotAction snapshot_action,
XLogRecPtr *lsn);
+typedef pid_t (*walrcv_get_backend_pid_fn) (WalReceiverConn *conn);
typedef WalRcvExecResult *(*walrcv_exec_fn) (WalReceiverConn *conn,
const char *query,
const int nRetTypes,
@@ -246,6 +247,7 @@ typedef struct WalReceiverFunctionsType
walrcv_receive_fn walrcv_receive;
walrcv_send_fn walrcv_send;
walrcv_create_slot_fn walrcv_create_slot;
+ walrcv_get_backend_pid_fn walrcv_get_backend_pid;
walrcv_exec_fn walrcv_exec;
walrcv_disconnect_fn walrcv_disconnect;
} WalReceiverFunctionsType;
@@ -276,6 +278,8 @@ extern PGDLLIMPORT WalReceiverFunctionsType
*WalReceiverFunctions;
WalReceiverFunctions->walrcv_send(conn, buffer, nbytes)
#define walrcv_create_slot(conn, slotname, temporary, snapshot_action, lsn) \
WalReceiverFunctions->walrcv_create_slot(conn, slotname, temporary,
snapshot_action, lsn)
+#define walrcv_get_backend_pid(conn) \
+ WalReceiverFunctions->walrcv_get_backend_pid(conn)
#define walrcv_exec(conn, exec, nRetTypes, retTypes) \
WalReceiverFunctions->walrcv_exec(conn, exec, nRetTypes, retTypes)
#define walrcv_disconnect(conn) \
--
2.24.1
From c6ebd1275f8e2490c8a1a5dba981bdce53aafe20 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Sat, 11 Jan 2020 10:24:49 +0100
Subject: [PATCH v2 2/2] walreceiver uses a temporary replication slot by
default
If no permanent replication slot is configured using
primary_slot_name, the walreceiver now
Re: base backup client as auxiliary backend process
On Fri, 22 Nov 2019 at 19:22, Peter Eisentraut wrote: > > On 2019-11-15 14:52, Sergei Kornilov wrote: > >> I looked into this. It seems trivial to make walsender create and use a > >> temporary replication slot by default if no permanent replication slot > >> is specified. This is basically the logic that pg_basebackup has but > >> done server-side. See attached patch for a demonstration. Any reason > >> not to do that? > > Seems this would break pg_basebackup --no-slot option? > > After thinking about this a bit more, doing the temporary slot stuff on > the walsender side might lead to too many complications in practice. > > Here is another patch set that implements the temporary slot use on the > walreceiver side, essentially mirroring what pg_basebackup already does. > > I think this patch set might be useful on its own, even without the base > backup stuff to follow. > I agreed that these patches are useful on its own and 0001 patch and 0002 patch look good to me. For 0003 patch, + linkend="guc-primary-slot-name"/>. Otherwise, the WAL receiver may use + a temporary replication slot (determined by ), but these are not shown + here. I think it's better to show the temporary slot name on pg_stat_wal_receiver view. Otherwise user would have no idea about what wal receiver is using the temporary slot. Regards, -- Masahiko Sawadahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
Hi Peter, On Fri, Nov 22, 2019 at 6:22 PM Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > Here is another patch set that implements the temporary slot use on the > walreceiver side, essentially mirroring what pg_basebackup already does. > > I think this patch set might be useful on its own, even without the base > backup stuff to follow. I very much like this idea of every replication connection should have a replication slot, either permanent or temporary if user didn't specify. I agree that this patch is useful on its own. > This makes a whole bunch of things much nicer: The connection > information for where to get the base backup from comes from > postgresql.conf, so you only need to specify it in one place. > pg_basebackup is completely out of the picture; no need to deal with > command-line options, --recovery-conf, screen, monitoring for > completion, etc. If something fails, the base backup process can > automatically be restarted (maybe). Operating system integration is > much easier: You only call initdb and then pg_ctl or postgres, as you > are already doing. Automated deployment systems don't need to wait for > pg_basebackup to finish: You only call initdb, then start the server, > and then you're done -- waiting for the base backup to finish can be > done by the regular monitoring system. Back to the base backup stuff, I don't quite understand all the benefits you mentioned above. It seems to me the greatest benefit with this patch is that postmaster takes care of pg_basebackup itself, which reduces the human wait in between running the pg_basebackup and pg_ctl/postgres commands. Is that right? I personally don't mind the --write-recovery-conf option because it helps me write the primary_conninfo and primary_slot_name gucs into postgresql.auto.conf, which to me as a developer is easier than editing postgres.conf without automation. Sorry about the dumb question but what's so bad about --write-recovery-conf? Are you planning to completely replace pg_basebackup with this? Is there any use case that a user just need a basebackup but not immediately start the backend process? Also the patch doesn't apply to master any more, need a rebase. -- Alexandra
Re: base backup client as auxiliary backend process
On Fri, Nov 22, 2019 at 11:21:53AM +0100, Peter Eisentraut wrote: > After thinking about this a bit more, doing the temporary slot stuff on the > walsender side might lead to too many complications in practice. > > Here is another patch set that implements the temporary slot use on the > walreceiver side, essentially mirroring what pg_basebackup already does. I have not looked at the patch, but controlling the generation of the slot from the client feels much more natural to me. This reuses the existing interface, which is consistent, and we avoid a new class of bugs if there is any need to deal with the cleanup of the slot on the WAL sender side it itself created. -- Michael signature.asc Description: PGP signature
Re: base backup client as auxiliary backend process
On 2019-11-15 14:52, Sergei Kornilov wrote:
I looked into this. It seems trivial to make walsender create and use a
temporary replication slot by default if no permanent replication slot
is specified. This is basically the logic that pg_basebackup has but
done server-side. See attached patch for a demonstration. Any reason
not to do that?
Seems this would break pg_basebackup --no-slot option?
After thinking about this a bit more, doing the temporary slot stuff on
the walsender side might lead to too many complications in practice.
Here is another patch set that implements the temporary slot use on the
walreceiver side, essentially mirroring what pg_basebackup already does.
I think this patch set might be useful on its own, even without the base
backup stuff to follow.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 00c816f2ee9b6b8c0668d17a596470a18c6092e1 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Fri, 22 Nov 2019 11:04:26 +0100
Subject: [PATCH 1/3] Make lsn argument walrcv_create_slot() optional
Some callers are not using it, so it's wasteful to have to specify it.
---
src/backend/commands/subscriptioncmds.c | 3 +--
src/backend/replication/libpqwalreceiver/libpqwalreceiver.c | 6 --
2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c
b/src/backend/commands/subscriptioncmds.c
index 5408edcfc2..198aa6f4b1 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -428,7 +428,6 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
*/
if (connect)
{
- XLogRecPtr lsn;
char *err;
WalReceiverConn *wrconn;
List *tables;
@@ -479,7 +478,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool
isTopLevel)
Assert(slotname);
walrcv_create_slot(wrconn, slotname, false,
-
CRS_NOEXPORT_SNAPSHOT, );
+
CRS_NOEXPORT_SNAPSHOT, NULL);
ereport(NOTICE,
(errmsg("created replication
slot \"%s\" on publisher",
slotname)));
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index 545d2fcd05..befedb811d 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -844,8 +844,10 @@ libpqrcv_create_slot(WalReceiverConn *conn, const char
*slotname,
slotname,
pchomp(PQerrorMessage(conn->streamConn);
}
- *lsn = DatumGetLSN(DirectFunctionCall1Coll(pg_lsn_in, InvalidOid,
-
CStringGetDatum(PQgetvalue(res, 0, 1;
+ if (lsn)
+ *lsn = DatumGetLSN(DirectFunctionCall1Coll(pg_lsn_in,
InvalidOid,
+
CStringGetDatum(PQgetvalue(res, 0, 1;
+
if (!PQgetisnull(res, 0, 2))
snapshot = pstrdup(PQgetvalue(res, 0, 2));
else
base-commit: 4a0aab14dcb35550b55e623a3c194442c5666084
--
2.24.0
From 3e42ec83d06dacf92063111e9dc1f033ef415e8a Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Fri, 22 Nov 2019 11:07:48 +0100
Subject: [PATCH 2/3] Expose PQbackendPID() through walreceiver API
This will be used by a subsequent patch.
---
.../replication/libpqwalreceiver/libpqwalreceiver.c | 11 +++
src/include/replication/walreceiver.h | 4
2 files changed, 15 insertions(+)
diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
index befedb811d..ccc31f3cee 100644
--- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
+++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c
@@ -74,6 +74,7 @@ static char *libpqrcv_create_slot(WalReceiverConn *conn,
bool
temporary,
CRSSnapshotAction snapshot_action,
XLogRecPtr
*lsn);
+static pid_t libpqrcv_get_backend_pid(WalReceiverConn *conn);
static WalRcvExecResult *libpqrcv_exec(WalReceiverConn *conn,
const char *query,
Re: base backup client as auxiliary backend process
Hello Could you rebase patch please? I have errors during patch apply. CFbot checks latest demonstration patch. > I looked into this. It seems trivial to make walsender create and use a > temporary replication slot by default if no permanent replication slot > is specified. This is basically the logic that pg_basebackup has but > done server-side. See attached patch for a demonstration. Any reason > not to do that? Seems this would break pg_basebackup --no-slot option? > + Do not copy configuration files, that is, files that end in > + .conf. possible we need ignore *.signal files too? > +/* > + * XXX copied from pg_basebackup.c > + */ > + > +unsigned long long totaldone; > +unsigned long long totalsize_kb; > +int tablespacenum; > +int tablespacecount; Variable declaration in the middle of file is correct for coding style? Not a problem for me, I just want to clarify. Should not be declared "static"? Also how about tablespacedone instead of tablespacenum? > The updated has support for tablespaces without mapping. I'm thinking > about putting the mapping specification into a GUC list somehow. > Shouldn't be too hard. I think we can leave tablespace mapping for pg_basebackup only. More powerful tool for less common scenarios. Or for another future patch. regards, Sergei
Re: base backup client as auxiliary backend process
On 2019-11-07 05:16, Michael Paquier wrote:
The current defaults of pg_basebackup have been thought so as the
backups taken have a good stability and so as monitoring is eased
thanks to --wal-method=stream and the use of replication slots.
Shouldn't the use of a least a temporary replication slot be mandatory
for the stability of the copy? It seems to me that there is a good
argument for having a second process which streams WAL on top of the
main backup process, and just use a WAL receiver for that.
Is this something that the walreceiver should be doing independent of this
patch?
There could be an argument for switching a WAL receiver to use a
temporary replication slot by default. Still, it seems to me that
this backup solution suffers from the same set of problems we have
spent years to fix with pg_basebackup with missing WAL files caused by
concurrent checkpoints removing things needed while the copy of the
main data folder and other tablespaces happens.
I looked into this. It seems trivial to make walsender create and use a
temporary replication slot by default if no permanent replication slot
is specified. This is basically the logic that pg_basebackup has but
done server-side. See attached patch for a demonstration. Any reason
not to do that?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 6ae76011ece6a5900cc06e2350b0ccb930eb41a0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Sat, 9 Nov 2019 22:10:19 +0100
Subject: [PATCH] walsender uses a temporary replication slot by default
---
src/backend/replication/walsender.c | 9 +++--
src/bin/pg_basebackup/pg_basebackup.c | 8 +++-
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/src/backend/replication/walsender.c
b/src/backend/replication/walsender.c
index 7f5671504f..c16455f3f0 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -563,6 +563,12 @@ StartReplication(StartReplicationCmd *cmd)
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
(errmsg("cannot use a logical
replication slot for physical replication";
}
+ else
+ {
+ char *slotname = psprintf("pg_walsender_%d", MyProcPid);
+
+ ReplicationSlotCreate(slotname, false, RS_TEMPORARY);
+ }
/*
* Select the timeline. If it was given explicitly by the client, use
@@ -703,8 +709,7 @@ StartReplication(StartReplicationCmd *cmd)
Assert(streamingDoneSending && streamingDoneReceiving);
}
- if (cmd->slotname)
- ReplicationSlotRelease();
+ ReplicationSlotRelease();
/*
* Copy is finished now. Send a single-row result set indicating the
next
diff --git a/src/bin/pg_basebackup/pg_basebackup.c
b/src/bin/pg_basebackup/pg_basebackup.c
index a9d162a7da..687bd331dd 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -68,6 +68,11 @@ typedef struct TablespaceList
*/
#define MINIMUM_VERSION_FOR_TEMP_SLOTS 10
+/*
+ * Version 13 creates temporary slots automatically.
+ */
+#define MINIMUM_VERSION_FOR_AUTO_TEMP_SLOTS 13
+
/*
* Different ways to include WAL
*/
@@ -569,7 +574,8 @@ StartLogStreamer(char *startpos, uint32 timeline, char
*sysidentifier)
"pg_xlog" : "pg_wal");
/* Temporary replication slots are only supported in 10 and newer */
- if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_TEMP_SLOTS)
+ if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_TEMP_SLOTS ||
+ PQserverVersion(conn) >= MINIMUM_VERSION_FOR_AUTO_TEMP_SLOTS)
temp_replication_slot = false;
/*
--
2.24.0
Re: base backup client as auxiliary backend process
On Mon, Oct 28, 2019 at 09:30:52AM +0100, Peter Eisentraut wrote: > On 2019-09-18 10:31, Michael Paquier wrote: >> -* Verify XLOG status looks valid. >> +* Check that contents look valid. >> */ >> - if (ControlFile->state < DB_SHUTDOWNED || >> - ControlFile->state > DB_IN_PRODUCTION || >> - !XRecOffIsValid(ControlFile->checkPoint)) >> + if (!XRecOffIsValid(ControlFile->checkPoint)) >> ereport(FATAL, >> Doesn't seem like a good idea to me to remove this sanity check for >> normal deployments, but actually you moved that down in StartupXLOG(). >> It seems to me tha this is unrelated and could be a separate patch so >> as the errors produced are more verbose. I think that we should also >> change that code to use a switch/case on ControlFile->state. > > Done. Yes, this was really a change made to get more precise error messaged > during debugging. It could be committed separately. If you wish to do so now, that's fine by me. >> The current defaults of pg_basebackup have been thought so as the >> backups taken have a good stability and so as monitoring is eased >> thanks to --wal-method=stream and the use of replication slots. >> Shouldn't the use of a least a temporary replication slot be mandatory >> for the stability of the copy? It seems to me that there is a good >> argument for having a second process which streams WAL on top of the >> main backup process, and just use a WAL receiver for that. > > Is this something that the walreceiver should be doing independent of this > patch? There could be an argument for switching a WAL receiver to use a temporary replication slot by default. Still, it seems to me that this backup solution suffers from the same set of problems we have spent years to fix with pg_basebackup with missing WAL files caused by concurrent checkpoints removing things needed while the copy of the main data folder and other tablespaces happens. >> One problem which is not tackled here is what to do for the tablespace >> map. pg_basebackup has its own specific trick for that, and with that >> new feature we may want something equivalent? Not something to >> consider as a first stage of course. > > The updated has support for tablespaces without mapping. I'm thinking about > putting the mapping specification into a GUC list somehow. Shouldn't be too > hard. That may become ugly if there are many tablespaces to take care of. Another idea I can come up with would be to pass the new mapping to initdb, still this requires an extra intermediate step to store the new map, and then compare it with the mapping received at BASE_BACKUP time. But perhaps you are looking for an experience different than pg_basebackup. The first version of the patch does not actually require that anyway.. >> No more strtol with base 10 stuff please :) > > Hmm, why not? What's the replacement? I was referring to this patch: https://commitfest.postgresql.org/25/2272/ It happens that all our calls of strtol in core use a base of 10. But please just ignore this part. ReceiveAndUnpackTarFile() is in both libpqwalreceiver.c and pg_basebackup.c. It would be nice to refactor that. -- Michael signature.asc Description: PGP signature
Re: base backup client as auxiliary backend process
Updated patch attached.
On 2019-09-18 10:31, Michael Paquier wrote:
-* Verify XLOG status looks valid.
+* Check that contents look valid.
*/
- if (ControlFile->state < DB_SHUTDOWNED ||
- ControlFile->state > DB_IN_PRODUCTION ||
- !XRecOffIsValid(ControlFile->checkPoint))
+ if (!XRecOffIsValid(ControlFile->checkPoint))
ereport(FATAL,
Doesn't seem like a good idea to me to remove this sanity check for
normal deployments, but actually you moved that down in StartupXLOG().
It seems to me tha this is unrelated and could be a separate patch so
as the errors produced are more verbose. I think that we should also
change that code to use a switch/case on ControlFile->state.
Done. Yes, this was really a change made to get more precise error
messaged during debugging. It could be committed separately.
The current defaults of pg_basebackup have been thought so as the
backups taken have a good stability and so as monitoring is eased
thanks to --wal-method=stream and the use of replication slots.
Shouldn't the use of a least a temporary replication slot be mandatory
for the stability of the copy? It seems to me that there is a good
argument for having a second process which streams WAL on top of the
main backup process, and just use a WAL receiver for that.
Is this something that the walreceiver should be doing independent of
this patch?
One problem which is not tackled here is what to do for the tablespace
map. pg_basebackup has its own specific trick for that, and with that
new feature we may want something equivalent? Not something to
consider as a first stage of course.
The updated has support for tablespaces without mapping. I'm thinking
about putting the mapping specification into a GUC list somehow.
Shouldn't be too hard.
*/
-static void
+void
WriteControlFile(void)
[...]
-static void
+void
ReadControlFile(void)
[...]
If you begin to publish those routines, it seems to me that there
could be more consolidation with controldata_utils.c which includes
now a routine to update a control file.
Hmm, maybe long-term, but it seems too much dangerous surgery for this
patch.
+#ifndef FRONTEND
+extern void InitControlFile(uint64 sysidentifier);
+extern void WriteControlFile(void);
+extern void ReadControlFile(void);
+#endif
It would be nice to avoid that.
Fixed by renaming a function in pg_resetwal.c.
+ /*
+* Wait until done. Start WAL receiver in the meantime, once
base
+* backup has received the starting position.
+*/
+ while (BaseBackupPID != 0)
+ {
+ PG_SETMASK();
+ pg_usleep(100L);
+ PG_SETMASK();
+ primary_sysid = strtoull(walrcv_identify_system(wrconn,
), NULL, 10);
No more strtol with base 10 stuff please :)
Hmm, why not? What's the replacement?
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From ac34ece7665b62d542653cf12238973a1a45a18b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Mon, 28 Oct 2019 09:23:43 +0100
Subject: [PATCH v3] Base backup client as auxiliary backend process
Discussion:
https://www.postgresql.org/message-id/flat/61b8d18d-c922-ac99-b990-a31ba63cd...@2ndquadrant.com
---
doc/src/sgml/protocol.sgml| 12 +-
doc/src/sgml/ref/initdb.sgml | 17 +
src/backend/access/transam/xlog.c | 184
src/backend/bootstrap/bootstrap.c | 9 +
src/backend/postmaster/pgstat.c | 6 +
src/backend/postmaster/postmaster.c | 114 -
src/backend/replication/basebackup.c | 70 +++
.../libpqwalreceiver/libpqwalreceiver.c | 400 ++
src/backend/replication/repl_gram.y | 9 +-
src/backend/replication/repl_scanner.l| 1 +
src/backend/storage/file/fd.c | 36 +-
src/bin/initdb/initdb.c | 39 +-
src/bin/pg_resetwal/pg_resetwal.c | 6 +-
src/include/access/xlog.h | 8 +-
src/include/miscadmin.h | 2 +
src/include/pgstat.h | 1 +
src/include/replication/basebackup.h | 2 +
src/include/replication/walreceiver.h | 4 +
src/include/storage/fd.h | 2 +-
src/include/utils/guc.h | 2 +-
src/test/recovery/t/018_basebackup.pl | 29 ++
21 files changed, 831 insertions(+), 122 deletions(-)
create mode 100644 src/test/recovery/t/018_basebackup.pl
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 80275215e0..f54b820edf 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2466,7 +2466,7 @@ Streaming Replication Protocol
-BASE_BACKUP [ LABEL
'label' ] [ PROGRESS ] [
FAST ] [ WAL ] [
NOWAIT ] [ MAX_RATE
rate ] [ TABLESPACE
Re: base backup client as auxiliary backend process
Hello, thanks for pinging. At Wed, 11 Sep 2019 19:15:24 -0300, Alvaro Herrera wrote in <20190911221524.GA16563@alvherre.pgsql> > On 2019-Aug-30, Peter Eisentraut wrote: > > > Attached is an updated patch for this. I have changed the initdb option > > name per suggestion. The WAL receiver is now started concurrently with > > the base backup. There is progress reporting (ps display), fsyncing. > > Configuration files are not copied anymore. There is a simple test > > suite. Tablespace support is still missing, but it would be > > straightforward. > > This is an amazing feature. How come we don't have people cramming to > review this? I love it, too. As for me, the reason for hesitating review this is the patch is said to be experimental. That means 'the details don't matter, let's discuss it's design/outline.'. So I wanted to see what the past reviewers comment on the revised shape before I would stir up the discussion by maybe-pointless comment. (Then forgotten..) I'll re-look on this. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: base backup client as auxiliary backend process
On 2019-Aug-30, Peter Eisentraut wrote: > Attached is an updated patch for this. I have changed the initdb option > name per suggestion. The WAL receiver is now started concurrently with > the base backup. There is progress reporting (ps display), fsyncing. > Configuration files are not copied anymore. There is a simple test > suite. Tablespace support is still missing, but it would be > straightforward. This is an amazing feature. How come we don't have people cramming to review this? -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
> Attached is a very hackish patch to implement this. It works like this:
>
> # (assuming you have a primary already running somewhere)
> initdb -D data2 --replica
> $EDITOR data2/postgresql.conf # set primary_conninfo
> pg_ctl -D data2 start
Attached is an updated patch for this. I have changed the initdb option
name per suggestion. The WAL receiver is now started concurrently with
the base backup. There is progress reporting (ps display), fsyncing.
Configuration files are not copied anymore. There is a simple test
suite. Tablespace support is still missing, but it would be
straightforward.
It's still all to be considered experimental, but it's taking shape and
working pretty well.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From aae4640acbb2a1ae4ff5d2e80abce0798799fe73 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut
Date: Fri, 30 Aug 2019 20:42:51 +0200
Subject: [PATCH v2] Base backup client as auxiliary backend process
Discussion:
https://www.postgresql.org/message-id/flat/61b8d18d-c922-ac99-b990-a31ba63cd...@2ndquadrant.com
---
doc/src/sgml/protocol.sgml| 12 +-
doc/src/sgml/ref/initdb.sgml | 17 +
src/backend/access/transam/xlog.c | 84 ++--
src/backend/bootstrap/bootstrap.c | 9 +
src/backend/postmaster/pgstat.c | 6 +
src/backend/postmaster/postmaster.c | 114 -
src/backend/replication/basebackup.c | 68 +++
.../libpqwalreceiver/libpqwalreceiver.c | 419 ++
src/backend/replication/repl_gram.y | 9 +-
src/backend/replication/repl_scanner.l| 1 +
src/bin/initdb/initdb.c | 39 +-
src/include/access/xlog.h | 6 +
src/include/miscadmin.h | 2 +
src/include/pgstat.h | 1 +
src/include/replication/basebackup.h | 2 +
src/include/replication/walreceiver.h | 4 +
src/include/utils/guc.h | 2 +-
src/test/recovery/t/018_basebackup.pl | 29 ++
18 files changed, 768 insertions(+), 56 deletions(-)
create mode 100644 src/test/recovery/t/018_basebackup.pl
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index b20f1690a7..81f43b5c00 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2466,7 +2466,7 @@ Streaming Replication Protocol
-BASE_BACKUP [ LABEL
'label' ] [ PROGRESS ] [
FAST ] [ WAL ] [
NOWAIT ] [ MAX_RATE
rate ] [ TABLESPACE_MAP ] [
NOVERIFY_CHECKSUMS ]
+BASE_BACKUP [ LABEL
'label' ] [ PROGRESS ] [
FAST ] [ WAL ] [
NOWAIT ] [ MAX_RATE
rate ] [ TABLESPACE_MAP ] [
NOVERIFY_CHECKSUMS ] [ EXCLUDE_CONF ]
BASE_BACKUP
@@ -2576,6 +2576,16 @@ Streaming Replication Protocol
+
+
+EXCLUDE_CONF
+
+
+ Do not copy configuration files, that is, files that end in
+ .conf.
+
+
+
diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml
index da5c8f5307..1261e02d59 100644
--- a/doc/src/sgml/ref/initdb.sgml
+++ b/doc/src/sgml/ref/initdb.sgml
@@ -286,6 +286,23 @@ Options
+
+ -r
+ --replica
+
+
+Initialize a data directory for a physical replication replica. The
+data directory will not be initialized with a full database system,
+but will instead only contain a minimal set of files. A server that
+is started on this data directory will first fetch a base backup and
+then switch to standby mode. The connection information for the base
+backup has to be configured by setting , and other parameters as desired,
+before the server is started.
+
+
+
+
-S
--sync-only
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index e651a841bb..7ab8ab45f5 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -905,8 +905,6 @@ static void CheckRecoveryConsistency(void);
static XLogRecord *ReadCheckpointRecord(XLogReaderState *xlogreader,
XLogRecPtr RecPtr, int whichChkpt, bool report);
static bool rescanLatestTimeLine(void);
-static void WriteControlFile(void);
-static void ReadControlFile(void);
static char *str_time(pg_time_t tnow);
static bool CheckForStandbyTrigger(void);
@@ -4481,7 +4479,7 @@ rescanLatestTimeLine(void)
* ReadControlFile() verifies they are correct. We could split out the
* I/O and compatibility-check functions, but there seems no need currently.
*/
-static void
+void
WriteControlFile(void)
{
int fd;
@@ -4573,7 +4571,7 @@
Re: base backup client as auxiliary backend process
Hello. At Sat, 29 Jun 2019 22:05:22 +0200, Peter Eisentraut wrote in <61b8d18d-c922-ac99-b990-a31ba63cd...@2ndquadrant.com> > Setting up a standby instance is still quite complicated. You need to > run pg_basebackup with all the right options. You need to make sure > pg_basebackup has the right permissions for the target directories. The > created instance has to be integrated into the operating system's start > scripts. There is this slightly awkward business of the --recovery-conf > option and how it interacts with other features. And you should > probably run pg_basebackup under screen. And then how do you get > notified when it's done. And when it's done you have to log back in and > finish up. Too many steps. > > My idea is that the postmaster can launch a base backup worker, wait > till it's done, then proceed with the rest of the startup. initdb gets > a special option to create a "minimal" data directory with only a few > files, directories, and the usual configuration files. Then you create > a $PGDATA/basebackup.signal, start the postmaster as normal. It sees > the signal file, launches an auxiliary process that runs the base > backup, then proceeds with normal startup in standby mode. > > This makes a whole bunch of things much nicer: The connection > information for where to get the base backup from comes from > postgresql.conf, so you only need to specify it in one place. > pg_basebackup is completely out of the picture; no need to deal with > command-line options, --recovery-conf, screen, monitoring for > completion, etc. If something fails, the base backup process can > automatically be restarted (maybe). Operating system integration is > much easier: You only call initdb and then pg_ctl or postgres, as you > are already doing. Automated deployment systems don't need to wait for > pg_basebackup to finish: You only call initdb, then start the server, > and then you're done -- waiting for the base backup to finish can be > done by the regular monitoring system. > > Attached is a very hackish patch to implement this. It works like this: > > # (assuming you have a primary already running somewhere) > initdb -D data2 --minimal > $EDITOR data2/postgresql.conf # set primary_conninfo > pg_ctl -D data2 start Nice idea! > (Curious side note: If you don’t set primary_conninfo in these steps, > then libpq defaults apply, so the default behavior might end up being > that a given instance attempts to replicate from itself.) We may be able to have different setting for primary and replica for other settings if we could have sections in the configuration file, defining, say, [replica] section gives us more frexibility. Though it is a bit far from the topic, dedicate command-line configuration editor that can find and replace specified parameter would elimite the sublte editing step. It is annoying that finding specific separator in conf file then trim then add new contnet. > It works for basic cases. It's missing tablespace support, proper > fsyncing, progress reporting, probably more. Those would be pretty While catching up master, connections to replica are once accepted then result in FATAL error. I now and then receive inquiries for that. With the new feature, we get FATAL also while basebackup phase. That can let users fear more frequently. > straightforward I think. The interesting bit is the delicate ordering > of the postmaster startup: Normally, the pg_control file is read quite > early, but if starting from a minimal data directory, we need to wait > until the base backup is done. There is also the question what you do > if the base backup fails halfway through. Currently you probably need > to delete the whole data directory and start again with initdb. Better > might be a way to start again and overwrite any existing files, but that > can clearly also be dangerous. All this needs some careful analysis, > but I think it's doable. > > Any thoughts? Just overwriting won't work since files removed just before retrying are left alon in replica. I think it should work similarly to initdb, that is, removing all then retrying. It's easy if we don't consider reducing startup time. Just do initdb then start exising postmaster internally. But melding them together makes room for reducing the startup time. We even could redirect read-only queries to master while setting up the server. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
Re: base backup client as auxiliary backend process
Peter Eisentraut writes: > On 2019-07-11 22:20, Robert Haas wrote: >> On Thu, Jul 11, 2019 at 4:10 PM Tom Lane wrote: >>> How's the postmaster to know that it's supposed to run pg_basebackup >>> rather than start normally? Where will it get the connection information? >>> Seem to need configuration data *somewhere*. >> >> Maybe just: >> >> ./postgres --replica='connstr' -D createme > What you are describing is of course theoretically possible, but it > doesn't really fit with how existing tooling normally deals with this, > which is one of the problems I want to address. I don't care for Robert's suggestion for a different reason: it presumes that all data that can possibly be needed to set up a new replica is feasible to cram onto the postmaster command line, and always will be. An immediate counterexample is that's not where you want to be specifying the password for a replication connection. But even without that sort of security issue, this approach won't scale. It also does not work even a little bit nicely for tooling in which the postmaster is not supposed to be started directly by the user. (Which is to say, all postgres-service tooling everywhere.) regards, tom lane
Re: base backup client as auxiliary backend process
On 2019-07-11 22:20, Robert Haas wrote: > On Thu, Jul 11, 2019 at 4:10 PM Tom Lane wrote: >> Robert Haas writes: >>> On Thu, Jul 11, 2019 at 10:36 AM Tom Lane wrote: Gotta have config files in place already, no? >> >>> Why? >> >> How's the postmaster to know that it's supposed to run pg_basebackup >> rather than start normally? Where will it get the connection information? >> Seem to need configuration data *somewhere*. > > Maybe just: > > ./postgres --replica='connstr' -D createme What you are describing is of course theoretically possible, but it doesn't really fit with how existing tooling normally deals with this, which is one of the problems I want to address. initdb has all the knowledge of how to create the data *directory*, how to set permissions, deal with existing and non-empty directories, how to set up a separate WAL directory. Packaged environments might wrap this further by using the correct OS users, creating the directory first as root, then changing owner, etc. This is all logic that we can reuse and probably don't want to duplicate elsewhere. Furthermore, we have for the longest time encouraged packagers *not* to create data directories automatically when a service is started, because this might store data in places that will be hidden by a later mount. Keeping this property requires making the initialization of the data directory a separate step somehow. That step doesn't have to be called "initdb", it could be a new "pg_mkdirs", but for the reasons described above, this would create a fair mount of code duplication and not really gain anything. Finally, many installations want to have the configuration files under control of some centralized configuration management system. The way those want to work is usually: (1) create file system structures, (2) install configuration files from some templates, (3) start service. This is of course how setting up a primary works. Having such a system set up a standby is currently seemingly impossible in an elegant way, because the order and timing of how things work is all wrong. My proposed change would fix this because things would be set up in the same three-step process. (As has been pointed out, this would require that the base backup does not copy over the configuration files from the remote, which my patch currently doesn't do correctly.) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: base backup client as auxiliary backend process
On Thu, Jul 11, 2019 at 4:10 PM Tom Lane wrote: > Robert Haas writes: > > On Thu, Jul 11, 2019 at 10:36 AM Tom Lane wrote: > >> Gotta have config files in place already, no? > > > Why? > > How's the postmaster to know that it's supposed to run pg_basebackup > rather than start normally? Where will it get the connection information? > Seem to need configuration data *somewhere*. Maybe just: ./postgres --replica='connstr' -D createme ? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: base backup client as auxiliary backend process
Robert Haas writes: > On Thu, Jul 11, 2019 at 10:36 AM Tom Lane wrote: >> Gotta have config files in place already, no? > Why? How's the postmaster to know that it's supposed to run pg_basebackup rather than start normally? Where will it get the connection information? Seem to need configuration data *somewhere*. regards, tom lane
Re: base backup client as auxiliary backend process
On Thu, Jul 11, 2019 at 10:36 AM Tom Lane wrote: > Gotta have config files in place already, no? Why? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: base backup client as auxiliary backend process
Robert Haas writes: > On Sat, Jun 29, 2019 at 4:05 PM Peter Eisentraut > wrote: >> My idea is that the postmaster can launch a base backup worker, wait >> till it's done, then proceed with the rest of the startup. initdb gets >> a special option to create a "minimal" data directory with only a few >> files, directories, and the usual configuration files. > Why do we even have to do that much? Can we remove the need for an > initdb altogether? Gotta have config files in place already, no? regards, tom lane
Re: base backup client as auxiliary backend process
On Sat, Jun 29, 2019 at 4:05 PM Peter Eisentraut wrote: > My idea is that the postmaster can launch a base backup worker, wait > till it's done, then proceed with the rest of the startup. initdb gets > a special option to create a "minimal" data directory with only a few > files, directories, and the usual configuration files. Why do we even have to do that much? Can we remove the need for an initdb altogether? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: base backup client as auxiliary backend process
Em sáb, 29 de jun de 2019 às 17:05, Peter Eisentraut escreveu: > > Setting up a standby instance is still quite complicated. You need to > run pg_basebackup with all the right options. You need to make sure > Attached is a very hackish patch to implement this. It works like this: > > # (assuming you have a primary already running somewhere) > initdb -D data2 --minimal > $EDITOR data2/postgresql.conf # set primary_conninfo > pg_ctl -D data2 start > Great! The main complaints about pg_basebackup usage in TB clusters are: (a) it can't be restarted and (b) it can't be parallelized. AFAICS your proposal doesn't solve them. It would be nice if it can be solved in future releases (using rsync or another in-house tool is as fragile as using pg_basebackup). -- Euler Taveira Timbira - http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento
Re: base backup client as auxiliary backend process
Hello >> Attached is a very hackish patch to implement this. It works like this: >> >> # (assuming you have a primary already running somewhere) >> initdb -D data2 --minimal >> $EDITOR data2/postgresql.conf # set primary_conninfo >> pg_ctl -D data2 start > > +1, very nice. How about --replica? +1 Also not works with -DEXEC_BACKEND for me. > There is also the question what you do > if the base backup fails halfway through. Currently you probably need > to delete the whole data directory and start again with initdb. Better > might be a way to start again and overwrite any existing files, but that > can clearly also be dangerous. I think the need for delete directory and rerun initdb is better than overwrite files. - we need check major version. Basebackup can works with different versions, but would be useless to copying cluster which we can not run - basebackup silently overwrite configs (pg_hba.conf, postgresql.conf, postgresql.auto.conf) in $PGDATA. This is ok for pg_basebackup but not for backend process - I think we need start walreceiver. At best, without interruption during startup replay (if possible) > XXX Is there a use for >* switching into (non-standby) recovery here? I think not. regards, Sergei
Re: base backup client as auxiliary backend process
On Sun, Jun 30, 2019 at 8:05 AM Peter Eisentraut wrote: > Attached is a very hackish patch to implement this. It works like this: > > # (assuming you have a primary already running somewhere) > initdb -D data2 --minimal > $EDITOR data2/postgresql.conf # set primary_conninfo > pg_ctl -D data2 start +1, very nice. How about --replica? FIY Windows doesn't like your patch: src/backend/postmaster/postmaster.c(1396): warning C4013: 'sleep' undefined; assuming extern returning int [C:\projects\postgresql\postgres.vcxproj] https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.45930 -- Thomas Munro https://enterprisedb.com
base backup client as auxiliary backend process
Setting up a standby instance is still quite complicated. You need to run pg_basebackup with all the right options. You need to make sure pg_basebackup has the right permissions for the target directories. The created instance has to be integrated into the operating system's start scripts. There is this slightly awkward business of the --recovery-conf option and how it interacts with other features. And you should probably run pg_basebackup under screen. And then how do you get notified when it's done. And when it's done you have to log back in and finish up. Too many steps. My idea is that the postmaster can launch a base backup worker, wait till it's done, then proceed with the rest of the startup. initdb gets a special option to create a "minimal" data directory with only a few files, directories, and the usual configuration files. Then you create a $PGDATA/basebackup.signal, start the postmaster as normal. It sees the signal file, launches an auxiliary process that runs the base backup, then proceeds with normal startup in standby mode. This makes a whole bunch of things much nicer: The connection information for where to get the base backup from comes from postgresql.conf, so you only need to specify it in one place. pg_basebackup is completely out of the picture; no need to deal with command-line options, --recovery-conf, screen, monitoring for completion, etc. If something fails, the base backup process can automatically be restarted (maybe). Operating system integration is much easier: You only call initdb and then pg_ctl or postgres, as you are already doing. Automated deployment systems don't need to wait for pg_basebackup to finish: You only call initdb, then start the server, and then you're done -- waiting for the base backup to finish can be done by the regular monitoring system. Attached is a very hackish patch to implement this. It works like this: # (assuming you have a primary already running somewhere) initdb -D data2 --minimal $EDITOR data2/postgresql.conf # set primary_conninfo pg_ctl -D data2 start (Curious side note: If you don’t set primary_conninfo in these steps, then libpq defaults apply, so the default behavior might end up being that a given instance attempts to replicate from itself.) It works for basic cases. It's missing tablespace support, proper fsyncing, progress reporting, probably more. Those would be pretty straightforward I think. The interesting bit is the delicate ordering of the postmaster startup: Normally, the pg_control file is read quite early, but if starting from a minimal data directory, we need to wait until the base backup is done. There is also the question what you do if the base backup fails halfway through. Currently you probably need to delete the whole data directory and start again with initdb. Better might be a way to start again and overwrite any existing files, but that can clearly also be dangerous. All this needs some careful analysis, but I think it's doable. Any thoughts? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services From 1cf36db2514b04428570496fc9d1145591fda0fc Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Sat, 29 Jun 2019 21:52:21 +0200 Subject: [PATCH v1] Base backup client as auxiliary backend process --- src/backend/access/transam/xlog.c | 14 +- src/backend/bootstrap/bootstrap.c | 9 + src/backend/postmaster/pgstat.c | 6 + src/backend/postmaster/postmaster.c | 95 +- src/backend/replication/Makefile | 2 +- src/backend/replication/basebackup_client.c | 33 ++ .../libpqwalreceiver/libpqwalreceiver.c | 308 ++ src/bin/initdb/initdb.c | 39 ++- src/include/access/xlog.h | 4 + src/include/miscadmin.h | 2 + src/include/pgstat.h | 1 + src/include/replication/basebackup_client.h | 1 + src/include/replication/walreceiver.h | 4 + 13 files changed, 502 insertions(+), 16 deletions(-) create mode 100644 src/backend/replication/basebackup_client.c create mode 100644 src/include/replication/basebackup_client.h diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index e08320e829..da97970703 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -905,7 +905,6 @@ static XLogRecord *ReadCheckpointRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr, int whichChkpti, bool report); static bool rescanLatestTimeLine(void); static void WriteControlFile(void); -static void ReadControlFile(void); static char *str_time(pg_time_t tnow); static bool CheckForStandbyTrigger(void); @@ -4572,7 +4571,7 @@ WriteCo
