Re: [HACKERS] pg_basebackup, tablespace mapping and path canonicalization
On 29/04/15 09:12, Bruce Momjian wrote: > On Fri, Feb 6, 2015 at 08:25:42AM -0500, Robert Haas wrote: >> On Thu, Feb 5, 2015 at 10:21 PM, Ian Barwick wrote: >>> I stumbled on what appears to be inconsistent handling of double slashes >>> in tablespace paths when using pg_basebackup with the >>> -T/--tablespace-mapping >>> option: >>> >>> ibarwick:postgresql (master)$ mkdir /tmp//foo-old >> [...] >>> The attached patch adds the missing canonicalization; I can't see any >>> reason not to do this. Thoughts? >> >> Seems OK to me. Anyone think differently? > > Patch applied. Thanks! Regards Ian Barwick -- Ian Barwick http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup, tablespace mapping and path canonicalization
On Fri, Feb 6, 2015 at 08:25:42AM -0500, Robert Haas wrote: > On Thu, Feb 5, 2015 at 10:21 PM, Ian Barwick wrote: > > I stumbled on what appears to be inconsistent handling of double slashes > > in tablespace paths when using pg_basebackup with the > > -T/--tablespace-mapping > > option: > > > > ibarwick:postgresql (master)$ mkdir /tmp//foo-old > [...] > > The attached patch adds the missing canonicalization; I can't see any > > reason not to do this. Thoughts? > > Seems OK to me. Anyone think differently? Patch applied. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_basebackup, tablespace mapping and path canonicalization
On Thu, Feb 5, 2015 at 10:21 PM, Ian Barwick wrote: > I stumbled on what appears to be inconsistent handling of double slashes > in tablespace paths when using pg_basebackup with the -T/--tablespace-mapping > option: > > ibarwick:postgresql (master)$ mkdir /tmp//foo-old [...] > The attached patch adds the missing canonicalization; I can't see any > reason not to do this. Thoughts? Seems OK to me. Anyone think differently? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_basebackup, tablespace mapping and path canonicalization
Hi I stumbled on what appears to be inconsistent handling of double slashes in tablespace paths when using pg_basebackup with the -T/--tablespace-mapping option: ibarwick:postgresql (master)$ mkdir /tmp//foo-old ibarwick:postgresql (master)$ $PG_HEAD/psql 'dbname=postgres port=9595' psql (9.5devel) Type "help" for help. postgres=# CREATE TABLESPACE foo LOCATION '/tmp//foo-old'; CREATE TABLESPACE postgres=# \db List of tablespaces Name| Owner | Location +--+-- foo| ibarwick | /tmp/foo-old pg_default | ibarwick | pg_global | ibarwick | (3 rows) So far so good. However attempting to take a base backup (on the same machine) and remap the tablespace directory: ibarwick:postgresql (master)$ $PG_HEAD/pg_basebackup -p9595 --pgdata=/tmp//backup --tablespace-mapping=/tmp//foo-old=/tmp//foo-new produces the following message: pg_basebackup: directory "/tmp/foo-old" exists but is not empty which, while undeniably true, is unexpected and could potentially encourage someone to hastily delete "/tmp/foo-old" after confusing it with the new directory. The double-slash in the old tablespace path is the culprit: ibarwick:postgresql (master)$ $PG_HEAD/pg_basebackup -p9595 --pgdata=/tmp//backup --tablespace-mapping=/tmp/foo-old=/tmp//foo-new NOTICE: pg_stop_backup complete, all required WAL segments have been archived The documentation does state: To be effective, olddir must exactly match the path specification of the tablespace as it is currently defined. which I understood to mean that e.g. tildes would not be expanded, but it's somewhat surprising that the path is not canonicalized in the same way it is pretty much everywhere else (including in "CREATE TABLESPACE"). The attached patch adds the missing canonicalization; I can't see any reason not to do this. Thoughts? Regards Ian Barwick -- Ian Barwick http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, RemoteDBA, Training & Services diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c new file mode 100644 index fbf7106..349bd90 *** a/src/bin/pg_basebackup/pg_basebackup.c --- b/src/bin/pg_basebackup/pg_basebackup.c *** tablespace_list_append(const char *arg) *** 199,204 --- 199,207 exit(1); } + canonicalize_path(cell->old_dir); + canonicalize_path(cell->new_dir); + if (tablespace_dirs.tail) tablespace_dirs.tail->next = cell; else -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
