From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Fri, 22 Aug 2014 07:16:26 +0000 (+0300)
Subject: Change the way pg_basebackup's tablespace mapping is implemented.
X-Git-Tag: REL9_4_BETA3~87
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=8a22633f52f2399118d6ad3c2c4da6a31f07e907;p=postgresql

Change the way pg_basebackup's tablespace mapping is implemented.

Previously, we would first create the symlinks the way they are in the
original system, and at the end replace them with the mapped symlinks.
That never really made much sense, so now we create the symlink pointing
to the correct location to begin with, so that there's no need to fix
them at the end.

The old coding didn't work correctly on Windows, because Windows junction
points look more like directories than files, and ought to be removed with
rmdir rather than unlink. Also, it incorrectly used "%d" rather than "%u"
to print an Oid, but that's gone now.

Report and patch by Amit Kapila, with minor changes by me. Reviewed by
MauMau. Backpatch to 9.4, where the --tablespace feature was added.
---

diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 85f18a8982..ec22ac2354 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -109,7 +109,6 @@ static bool reached_end_position(XLogRecPtr segendpos, uint32 timeline,
 					 bool segment_finished);
 
 static const char *get_tablespace_mapping(const char *dir);
-static void update_tablespace_symlink(Oid oid, const char *old_dir);
 static void tablespace_list_append(const char *arg);
 
 
@@ -1109,34 +1108,6 @@ get_tablespace_mapping(const char *dir)
 }
 
 
-/*
- * Update symlinks to reflect relocated tablespace.
- */
-static void
-update_tablespace_symlink(Oid oid, const char *old_dir)
-{
-	const char *new_dir = get_tablespace_mapping(old_dir);
-
-	if (strcmp(old_dir, new_dir) != 0)
-	{
-		char	   *linkloc = psprintf("%s/pg_tblspc/%d", basedir, oid);
-
-		if (unlink(linkloc) != 0 && errno != ENOENT)
-		{
-			fprintf(stderr, _("%s: could not remove symbolic link \"%s\": %s\n"),
-					progname, linkloc, strerror(errno));
-			disconnect_and_exit(1);
-		}
-		if (symlink(new_dir, linkloc) != 0)
-		{
-			fprintf(stderr, _("%s: could not create symbolic link \"%s\": %s\n"),
-					progname, linkloc, strerror(errno));
-			disconnect_and_exit(1);
-		}
-	}
-}
-
-
 /*
  * Receive a tar format stream from the connection to the server, and unpack
  * the contents of it into a directory. Only files, directories and
@@ -1151,16 +1122,20 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 {
 	char		current_path[MAXPGPATH];
 	char		filename[MAXPGPATH];
+	const char *mapped_tblspc_path;
 	int			current_len_left;
 	int			current_padding = 0;
-	bool		basetablespace = PQgetisnull(res, rownum, 0);
+	bool		basetablespace;
 	char	   *copybuf = NULL;
 	FILE	   *file = NULL;
 
+	basetablespace = PQgetisnull(res, rownum, 0);
 	if (basetablespace)
 		strlcpy(current_path, basedir, sizeof(current_path));
 	else
-		strlcpy(current_path, get_tablespace_mapping(PQgetvalue(res, rownum, 1)), sizeof(current_path));
+		strlcpy(current_path,
+				get_tablespace_mapping(PQgetvalue(res, rownum, 1)),
+				sizeof(current_path));
 
 	/*
 	 * Get the COPY data
@@ -1284,13 +1259,25 @@ ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum)
 				{
 					/*
 					 * Symbolic link
+					 *
+					 * It's most likely a link in pg_tblspc directory, to
+					 * the location of a tablespace. Apply any tablespace
+					 * mapping given on the command line (--tablespace).
+					 * (We blindly apply the mapping without checking that
+					 * the link really is inside pg_tblspc. We don't expect
+					 * there to be other symlinks in a data directory, but
+					 * if there are, you can call it an undocumented feature
+					 * that you can map them too.)
 					 */
 					filename[strlen(filename) - 1] = '\0';		/* Remove trailing slash */
-					if (symlink(&copybuf[157], filename) != 0)
+
+					mapped_tblspc_path = get_tablespace_mapping(&copybuf[157]);
+					if (symlink(mapped_tblspc_path, filename) != 0)
 					{
 						fprintf(stderr,
 								_("%s: could not create symbolic link from \"%s\" to \"%s\": %s\n"),
-						 progname, filename, &copybuf[157], strerror(errno));
+								progname, filename, mapped_tblspc_path,
+								strerror(errno));
 						disconnect_and_exit(1);
 					}
 				}
@@ -1793,17 +1780,6 @@ BaseBackup(void)
 		fprintf(stderr, "\n");	/* Need to move to next line */
 	}
 
-	if (format == 'p' && tablespace_dirs.head != NULL)
-	{
-		for (i = 0; i < PQntuples(res); i++)
-		{
-			Oid			tblspc_oid = atooid(PQgetvalue(res, i, 0));
-
-			if (tblspc_oid)
-				update_tablespace_symlink(tblspc_oid, PQgetvalue(res, i, 1));
-		}
-	}
-
 	PQclear(res);
 
 	/*