From 8a22633f52f2399118d6ad3c2c4da6a31f07e907 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Fri, 22 Aug 2014 10:16:26 +0300 Subject: [PATCH] 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. --- src/bin/pg_basebackup/pg_basebackup.c | 64 +++++++++------------------ 1 file changed, 20 insertions(+), 44 deletions(-) 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(©buf[157], filename) != 0) + + mapped_tblspc_path = get_tablespace_mapping(©buf[157]); + if (symlink(mapped_tblspc_path, filename) != 0) { fprintf(stderr, _("%s: could not create symbolic link from \"%s\" to \"%s\": %s\n"), - progname, filename, ©buf[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); /*