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

On 06/08/2015 11:16 AM, Amit Kapila wrote:
> On Mon, Jun 8, 2015 at 6:39 PM, Andrew Dunstan <andrew(at)dunslane(dot)net
> <mailto:andrew(at)dunslane(dot)net>> wrote:
>
>
> On 06/08/2015 12:08 AM, Amit Kapila wrote:
>
> 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 have to retry that operation, but for me unlink hasn't deleted
> the file on Windows, may be I am not doing properly, but in
> anycase why we want to throw error for such a case, why
> can't we just ignore and create a symlink with the same name.

1. You realize that in Windows postgres, unlink is actually pgunlink(),
right? See port.h. If your experiments weren't using that then they
weren't testing the same thing.

2. If the unlink fails and the file is still there (i.e. pretty much
everything except the ENOENT case) then creation of the symlink is bound
to fail anyway.

> 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.
>
>
> I agree with you that deleting unrelated file with the same name as
> symlink is not the right thing to do, but not sure throwing error for
> such a case is better either.
>
>

What else would you suggest? Closing our eyes and wishing it weren't so
doesn't seem like a solution.

cheers

andrew