Re: [PATCH] Relocation of tablespaces in pg_basebackup - Mailing list pgsql-hackers

From Steeve Lennmark
Subject Re: [PATCH] Relocation of tablespaces in pg_basebackup
Date
Msg-id CADAK8w4UPXBruJ4EQ_Jmn1GPjC7WAZZiBsxGrEe3cNBuM4cqcw@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Relocation of tablespaces in pg_basebackup  (Steeve Lennmark <steevel@handeldsbanken.se>)
Responses Re: [PATCH] Relocation of tablespaces in pg_basebackup
Re: [PATCH] Relocation of tablespaces in pg_basebackup
List pgsql-hackers
New patch attached with the following changes:

On Thu, Jan 23, 2014 at 11:01 AM, Steeve Lennmark <steevel@handeldsbanken.se> wrote:
On Thu, Jan 23, 2014 at 2:06 AM, Peter Eisentraut <peter_e@gmx.net> wrote:
I'm not totally happy with the choice of ":" as the mapping separator,
because that would always require escaping on Windows.  We could make it
analogous to the path handling and make ";" the separator on Windows.
Then again, this is not a path, so maybe it should look like one.  We
pick something else altogether, like "=".

The option name "--tablespace" isn't very clear.  It ought to be
something like "--tablespace-mapping".

This design choice I made about -T (--tablespace) and colon as
separator was copied from pg_barman, which is the way I back up my
clusters at the moment. Renaming the option to --tablespace-mapping and
changing the syntax to something like "=" is totally fine by me.

I changed the directory separator from ":" to "=" but made it
configurable in the code.
  
I don't think we should require the new directory to be an absolute
path.  It should be relative to the current directory, just like the -D
option does it.

Accepting a relative path should be fine, I made a failed attempt using
realpath(3) initially but I guess checking for [0] != '/' and
prepending getcwd(3) would suffice.

Relative paths are now accepted. This code will probably not work on
windows though. I tried setting up Windows 8 with the git version of
postgres but was unsuccessful, so I can't really test any of this.
Help on this subject (Windows) would be much appreciated.
 
Somehow, I had the subconscious assumption that this feature would do
prefix matching on the directories, not only complete string matching.
So if I had tablespaces in /mnt/data1, /mnt/data2, /mnt/data3, I could
map them all with -T /mnt:mnt and be done.  Not sure if that is useful
to many, but it's worth a thought.

I like that a lot, but I'm afraid the code would have to get a bit more
complex to add that functionality. It would be an easier rewrite if we
added a hint character, something like -T '/mnt/*:mnt'.

This is not implemented as suggested by Peter in his previous comment.
-T /mnt:mnt now relocates all tablespaces under /mnt to the relative
path mnt.


I've updated error messages according to the style guide.

We need to think about how to handle this on platforms without symlinks.
I don't like just printing an error message and moving on.  It should be
either pass or fail or an option to choose between them.

I hope someone with experience with those kind of systems can come up
with suggestions on how to solve that. I only run postgres on Linux.

I would still love some input on this.

Please put the options in the getopt handling in alphabetical order.
It's not very alphabetical now, but between D and F is probably not the
best place. ;-)

Done.

//Steeve 
Attachment

pgsql-hackers by date:

Previous
From: Ian Lawrence Barwick
Date:
Subject: Re: Review: Patch FORCE_NULL option for copy COPY in CSV mode
Next
From: Pavel Stehule
Date:
Subject: Re: patch: option --if-exists for pg_dump