Re: [bug fix] PostgreSQL fails to start on Windows if it crashes after tablespace creation

I did worked on testing the patch on Windows, test scenario mentioned above
thread is reproducible and the provided patch resolve the issue. In case of
junction or directory unlink function
(deprecated<http://msdn.microsoft.com/en-us/library/ms235350.aspx>)
returns -1 with errno “EACCES” (i.e. Permission denied). As you have
followed destroy_tablespace_directories() function, Is there any specific
reason not to use same logic to detect type of the file/link i.e.
“(lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))”, It also seems have
more appropriate error message i.e.

src/backend/commands/tablespace.c

> ….
> 745 /*
> 746 * Try to remove the symlink. We must however deal with
> the possibility
> 747 * that it's a directory instead of a symlink --- this
> could happen during
> 748 * WAL replay (see TablespaceCreateDbspace), and it is
> also the case on
> 749 * Windows where junction points lstat() as directories.
> 750 *
> 751 * Note: in the redo case, we'll return true if this final
> step fails;
> 752 * there's no point in retrying it. Also, ENOENT should
> provoke no more
> 753 * than a warning.
> 754 */
> 755 remove_symlink:
> 756 linkloc = pstrdup(linkloc_with_version_dir);
> 757 get_parent_directory(linkloc);
> 758 if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
> 759 {
> 760 if (rmdir(linkloc) < 0)
> 761 ereport(redo ? LOG : ERROR,
> 762 (errcode_for_file_access(),
> 763 errmsg("could not remove
> directory \"%s\": %m",
> 764 linkloc)));
> 765 }
> 766 else
> 767 {
> 768 if (unlink(linkloc) < 0)
> 769 ereport(redo ? LOG : (errno == ENOENT ?
> WARNING : ERROR),
> 770 (errcode_for_file_access(),
> 771 errmsg("could not remove
> symbolic link \"%s\": %m",
> 772 linkloc)));
> 773 }
> ….

Other than this patch looks good to me.

Regards,
Muhammad Asif Naeem

On Thu, Oct 31, 2013 at 8:03 PM, MauMau <maumau307(at)gmail(dot)com> wrote:

> From: "MauMau" <maumau307(at)gmail(dot)com>
>
> I thought the same situation could happen as in
>> destroy_tablespace_directories(), but it doesn't seem to apply on second
>> thought. Revised patch attached, which is very simple based on compile
>> time
>> check.
>>
>
> Sorry, I was careless to leave an old comment. Please use this version.
>
> Regards
> MauMau
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>