Postgres Pro
Downloads
Home   >   mailing lists

Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file
Date
Msg-id CAA4eK1+fLqQYWKNM+arNC0QryGGQkUJ0VHUOhoLZjTRB6xJfCQ@mail.gmail.com
Whole thread Raw
In response to Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file  (Andrew Dunstan <andrew@dunslane.net>)
Responses Re: Re: [COMMITTERS] pgsql: Map basebackup tablespaces using a tablespace_map file  (Andrew Dunstan <andrew@dunslane.net>)
List pgsql-hackers
Tree view
On Fri, Jun 5, 2015 at 7:29 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

On 06/04/2015 09:23 AM, Amit Kapila wrote:



    Okay, as we both seem to agree that it can be mostly used in
    tablespace symlinks context, so I have changed the name to
    remove_tablespace_symlink() and moved the function to
    tablespace.c.  S_ISLINK check is used for non-windows code,
    so not sure adding it here makes any real difference now that
    we have made it specific to tablespace and we might need to
    write small port specific code if we want to add S_ISLINK check.



 Where is it used? I can't see it called at all in tablespace.c or xlog.c.


Below files use S_ISLINK check
basebackup.c, fd.c, initdb.c, copy_fetch.c, pg_rewind/filemap.c

and all these places use it with #ifndef WIN32

 
Perhaps I'm being overcautious, but here's more or less what I had in mind.

What is making you feel nervous, if it is that we should not
use unlink call without checking S_ISLINK, then we are
already doing the same at many other places (rewriteheap.c,
slru.c, timeline.c, xlog.c).  It is already defined for Windows
as pgunlink.

Theoretically, I don't see much problem by changing the checks
way you have done in patch, but it becomes different than what
we have in destroy_tablespace_directories() and it is slightly
changing the way check was originally done in
create_tablespace_directories(), basically original check will try
unlink if lstat returns non-zero return code.  If you want to proceed
with the changed checks as in v3, then may be we can modify
comments on top of function remove_tablespace_symlink() which
indicates that it works like destroy_tablespace_directories().
 


With Regards,
Amit Kapila.

pgsql-hackers by date:

Previous
From: Kouhei Kaigai
Date:
Subject: Re: [idea] more aggressive join pushdown on postgres_fdw
Next
From: Michael Paquier
Date:
Subject: Re: Memory leak with XLogFileCopy since de768844 (WAL file with .partial)