On Fri, May 29, 2015 at 6:49 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Fri, May 29, 2015 at 2:28 PM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>>
>> On Fri, May 29, 2015 at 5:01 PM, Amit Kapila <amit.kapila16@gmail.com>
>> wrote:
>> >
>> > Test-3 - Symlinks in pg_tblspc.
>> > 1. Create couple of tablespaces which creates symlinks
>> > in pg_tblspc
>> > 2. Crash the server
>> > 3. Restart Server - It works fine.
>> >
>> > I am not sure Test-2 is a valid thing and we should support it or
>> > not, but the current patch is sane w.r.t that form of symlinks
>> > as well.
>>
>> It is always good to check, but is that relevant to the bug fixed? I
>> mean, you need to symlink a file in PGDATA that server has no write
>> permission on... For example tablespaces can be written to in test 3,
>> so that would work even with 9.4.2 after crashing the server with -m
>> immediate.
>
> I have just tried to cover the paths which has symlink related code
> in the new commit (in functions SyncDataDirectory() and walkdir()),
> because thats what I understood from Tom's mail. Without test-3, it
> won't cover walkdir symlink test path, I didn't knew that there was any
> confusion about verification of permissions problem on Windows
> and I haven't looked to verify the whole patch.
Reading once again this thread (after some good red wine + alpha), it
looks that you are right: not many checks with Windows junctions have
actually been with what has been committed...
> Test - 2 - Directory Symlink for pg_xlog
> First 4 steps are same as Test-1
> 5. mklink /D E:\ PGData\pg_xlog E:\TempLog\xlog_symlink_loc
> 6. Restart Server - Error
> - FATAL: required WAL directory "pg_xlog" does not exist
> This error occurs in
> ValidateXLOGDirectoryStructure()->stat(XLOGDIR, &stat_buf)
> 7. If I manually step-through that line, it proceeds and in
> SyncDataDirectory(), it detects the symlink;
> 8. After that in SyncDataDirectory(), when it does walkdir for
> datadir, for ./pg_xlog it reports the log message and then
> proceeds normal and the server is started.
Regarding this test, I just tested initdb -X and it works correctly
after crashing the server, so there is at least a workaround for the
error you are triggering here. Now, shouldn't we improve things as
well with mklink? That's a rather narrow case and we have a workaround
for it at initialization with initdb. I haven't looked much at the
code so I don't have a patch at hand but thoughts on this matter are
welcome.
--
Michael
