Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file

On 06/08/2015 12:08 AM, Amit Kapila wrote:
> On Mon, Jun 8, 2015 at 5:52 AM, Andrew Dunstan <andrew(at)dunslane(dot)net
> <mailto:andrew(at)dunslane(dot)net>> wrote:
> >
> > On 06/05/2015 11:08 PM, Amit Kapila wrote:
> >>
> >>
> >> Okay, I think I can understand why you want to be cautious for
> >> having a different check for this path, but in that case there is a
> >> chance that recovery might fail when it will try to create a
> symlink
> >> for that file. Shouldn't we then try to call this new function
> only
> >> when we are trying to restore from tablespace_map file and also
> >> is there a need of ifdef S_ISLINK in remove_tablespace_link?
> >>
> >>
> >> Shall I send an updated patch on these lines or do you want to
> >> proceed with v3 version?
> >>
> >>
> >
> > The point seems to me that we should not be removing anything that's
> not an empty directory or symlink, and that nothing else has any
> business being in pg_tblspc. If we encounter such a thing whose name
> conflicts with the name of a tablespace link we wish to create, rather
> than quietly blowing it away we should complain loudly, and error out,
> making it the user's responsibility to clean up their mess. Am I
> missing something?
> >
>
> How about if it is just a flat file with same name as tablespace link,
> why we want to give error for that case? I think now it just don't do
> anything with that file (unlink will fail with ENOENT and it will be
> ignored, atleast thats the way currently it behaves in Windows) and
> create a separate symlink with same name which seems okay to
> me and in the change proposed by you it will give error, do you see
> any reason for doing so?
>
>

This is surely wrong. unlink won't fail with ENOENT if the file is
present; ENOENT means that the file is NOT present. It will succeed if
the file is present, which is exactly what I'm saying is wrong.

I realize our existing code just more or less assumes that that it's a
symlink. I think we've probably been a bit careless there.

cheers

andrew