On Friday, April 7, 2017 3:12:58 AM CEST, Kyotaro HORIGUCHI wrote:
Hi, Pierre.

Maybe you're the winner:p

At Thu, 06 Apr 2017 12:34:09 +0200, Pierre Ducroquet <p.p...@pinaraf.info> wrote in <1714428.BHRm6e8A2D@peanuts2>
On Thursday, April 6, 2017 2:00:55 PM CEST Kyotaro HORIGUCHI wrote: ...

That being said, it is a different matter if the behavior is
preferable. The discussion on the behavior is continued here.

https://www.postgresql.org/message-id/20170406.160844.120459562.horiguchi.kyot...@lab.ntt.co.jp

Right now, there is a conflict between pg_basebackup and the server since they do not allow the same behaviour. I can submit a patch either way, but I won't decide what is the right way to do it. I know tricks will allow to work around that issue, I found them hopefully and I guess most people affected by this issue would be able to find and use them, but nevertheless being able to build a server that can no longer be base-
backuped does not seem right.

regards,

Hi

I didn't have much time to spend on that issue until today, and I found a way to solve it that seems acceptable to me.

The biggest drawback will be that if the backup is interrupted, previous tablespaces already copied will stay on disk, but since that issue could already happen, I don't think it's too much a drawback. The patch simply delays the empty folder checking until we start reading the tablespace tarball. The first entry of the tarball being the PG_MAJOR_CATVER folder, that can not be found otherwise, there is no real alternative to that.

I will submit this patch in the current commit fest.


Regards

Pierre
From f3e6b078d4159c90237d966c56289cb59b54ede1 Mon Sep 17 00:00:00 2001
From: Pierre Ducroquet <pierre.ducroq...@people-doc.com>
Date: Mon, 1 May 2017 11:38:52 +0200
Subject: [PATCH] Allow a pg_basebackup when a tablespace is shared between two
 versions

When a tablespace folder is shared between two PostgreSQL versions,
pg_basebackup fails because its tablespace folder checking is stricter
than what is done in the server.
That behaviour makes it possible to create clusters that will then be
complicated to replicate whithout playing with symlinks.

This patch fixes this by delaying the tablespace folder verification.
The folder name is using the PG catalog version, that can not be
obtained from the server. It is a compile-time constant that is not
exposed publicly.
The fix is thus to simply delay the checking of folders and use the
folder name from the tablespace tarball.
---
 src/bin/pg_basebackup/pg_basebackup.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index e2a2ebb30f..2e687a2880 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1306,6 +1306,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 	pgoff_t		current_len_left = 0;
 	int			current_padding = 0;
 	bool		basetablespace;
+	bool		firstfile = 1;
 	char	   *copybuf = NULL;
 	FILE	   *file = NULL;
 
@@ -1399,7 +1400,15 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 					 * Directory
 					 */
 					filename[strlen(filename) - 1] = '\0';		/* Remove trailing slash */
-					if (mkdir(filename, S_IRWXU) != 0)
+					if (firstfile && !basetablespace)
+					{
+						/*
+						 * The first file in the tablespace is its main folder, whose name can not be guessed (PG_MAJORVER_CATVER)
+						 * So we must check here that this folder can be created or is empty.
+						 */
+						verify_dir_is_empty_or_create(filename, &made_tablespace_dirs, &found_tablespace_dirs);
+					}
+					else if (mkdir(filename, S_IRWXU) != 0)
 					{
 						/*
 						 * When streaming WAL, pg_wal (or pg_xlog for pre-9.6
@@ -1530,6 +1539,7 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 				continue;
 			}
 		}						/* continuing data in existing file */
+		firstfile = 0;			/* mark that we are done with the first file of the tarball */
 	}							/* loop over all data blocks */
 	progress_report(rownum, filename, true);
 
@@ -1844,18 +1854,6 @@ BaseBackup(void)
 	for (i = 0; i < PQntuples(res); i++)
 	{
 		totalsize += atol(PQgetvalue(res, i, 2));
-
-		/*
-		 * Verify tablespace directories are empty. Don't bother with the
-		 * first once since it can be relocated, and it will be checked before
-		 * we do anything anyway.
-		 */
-		if (format == 'p' && !PQgetisnull(res, i, 1))
-		{
-			char	   *path = (char *) get_tablespace_mapping(PQgetvalue(res, i, 1));
-
-			verify_dir_is_empty_or_create(path, &made_tablespace_dirs, &found_tablespace_dirs);
-		}
 	}
 
 	/*
-- 
2.11.0

-- 
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