On Wednesday, September 13, 2017 6:01:43 PM CEST you wrote:
> > On 15 May 2017, at 07:26, Michael Paquier <michael.paqu...@gmail.com>
> > wrote:>
> > On Tue, May 2, 2017 at 2:28 AM, Pierre Ducroquet <p.p...@pinaraf.info>
wrote:
> >> I will submit this patch in the current commit fest.
> >
> > I have not spotted any flaws in the refactored logic.
>
> This patch no longer applies, could you take a look at it and submit a new
> version rebased on top of HEAD?
>
> cheers ./daniel
Hi
All my apologies for the schockingly long time with no answer on this topic.
Attached to this email is the new version of the patch, checked against HEAD
and REL_10_STABLE, with the small change required by Michael (affect true/
false to the boolean instead of 1/0) applied.
I will do my best to help review some patches in the current CF.
Pierre
>From cfb47eb5db398c1a30c5f83c680b59b4aa3d196a Mon Sep 17 00:00:00 2001
From: Pierre Ducroquet <p.p...@pinaraf.info>
Date: Wed, 13 Sep 2017 19:09:32 +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 dfb9b5ddcb..080468d846 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1310,6 +1310,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;
@@ -1403,7 +1404,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
@@ -1534,6 +1543,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);
@@ -1848,18 +1858,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.14.1
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
