On Fri, Jun 5, 2015 at 10:51 AM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Fri, Jun 5, 2015 at 9:57 AM, Andrew Dunstan <and...@dunslane.net> > wrote: > >> >> On 06/04/2015 11:35 PM, Amit Kapila wrote: >> >>> >>> 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(). >>> >>> >> >> The difference is that here we're getting the list from a base backup and >> it seems to me the risk of having a file we don't really want to unlink is >> significantly greater. >> > > 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? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
