Hi,

Am Freitag, den 17.03.2017, 10:50 +0900 schrieb Michael Paquier:
> The comment block format is incorrect. I would think as well that this
> comment should say it is important to have the main tablespace listed
> last it includes the WAL segments, and those need to contain all the
> latest WAL segments for a consistent backup.

How about the attached? The comment now reads as follows:

|Add a node for the base directory. If WAL is included, the base
|directory has to be last as the WAL files get appended to it. If WAL
|is not included, send the base directory first, so that the
|backup_label file is the first file to be sent.

> FWIW, I have no issue with changing the ordering of backups the way
> you are proposing here as long as the comment of this code path is
> clear.

OK, great, let's see what the committers think then.


Michael

-- 
Michael Banck
Projektleiter / Senior Berater
Tel.: +49 2166 9901-171
Fax:  +49 2166 9901-100
Email: michael.ba...@credativ.de

credativ GmbH, HRB Mönchengladbach 12080
USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer

From 608ba9113e6d8f5752690c526051f61909c2e982 Mon Sep 17 00:00:00 2001
From: Michael Banck <michael.ba...@credativ.de>
Date: Fri, 17 Mar 2017 11:10:53 +0100
Subject: [PATCH] Reorder tablespaces for non-WAL streaming basebackups.

Currently, the main tablespace is the last to be sent, in order for the WAL
files to be appended to it. This makes the backup_label file (which gets
prepended to the main tablespace) inconveniently end up in the middle of the
basebackup stream if other tablespaces are present.

Change this so that the main tablespace is the first to be sent in case
no WAL files are requested, ensuring that backup_label is the first file
in the stream in this case.
---
 src/backend/replication/basebackup.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index e3a7ad5..e03e902 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -233,10 +233,17 @@ perform_base_backup(basebackup_options *opt, DIR *tblspcdir)
 		else
 			statrelpath = pgstat_stat_directory;
 
-		/* Add a node for the base directory at the end */
+		/* Add a node for the base directory. If WAL is included, the base
+		 * directory has to be last as the WAL files get appended to it. If WAL
+		 * is not included, send the base directory first, so that the
+		 * backup_label file is the first file to be sent.
+		 */
 		ti = palloc0(sizeof(tablespaceinfo));
 		ti->size = opt->progress ? sendDir(".", 1, true, tablespaces, true) : -1;
-		tablespaces = lappend(tablespaces, ti);
+		if (opt->includewal)
+			tablespaces = lappend(tablespaces, ti);
+		else
+			tablespaces = lcons(ti, tablespaces);
 
 		/* Send tablespace header */
 		SendBackupHeader(tablespaces);
-- 
2.1.4

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

Reply via email to