pg_basebackup, tablespace mapping and path canonicalization - Mailing list pgsql-hackers

From Ian Barwick
Subject pg_basebackup, tablespace mapping and path canonicalization
Date
Msg-id 54D43345.2090608@2ndquadrant.com
Whole thread Raw
Responses Re: pg_basebackup, tablespace mapping and path canonicalization  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: Table-level log_autovacuum_min_duration
Next
From: Michael Paquier
Date:
Subject: Re: Patch: add recovery_timeout option to control timeout of restore_command nonzero status code