Thread: pgsql: Fix pg_rewind when pg_xlog is a symlink.
Fix pg_rewind when pg_xlog is a symlink. pg_xlog is often a symlink, typically to a different filesystem. Don't get confused and comlain about by that, and just always pretend that it's a normal directory, even if it's really a symlink. Also add a test case for this. Backpatch to 9.5. Branch ------ master Details ------- http://git.postgresql.org/pg/commitdiff/0e42397f42b370798461cbf3358185b520329f9f Modified Files -------------- src/bin/pg_rewind/RewindTest.pm | 14 +++-- src/bin/pg_rewind/filemap.c | 16 +++++- src/bin/pg_rewind/t/001_basic.pl | 1 + src/bin/pg_rewind/t/002_databases.pl | 1 + src/bin/pg_rewind/t/003_extrafiles.pl | 1 + src/bin/pg_rewind/t/004_pg_xlog_symlink.pl | 79 ++++++++++++++++++++++++++++ 6 files changed, 107 insertions(+), 5 deletions(-)
On Mon, Aug 3, 2015 at 9:34 PM, Heikki Linnakangas <heikki.linnakangas@iki.fi> wrote: > Fix pg_rewind when pg_xlog is a symlink. > > pg_xlog is often a symlink, typically to a different filesystem. Don't > get confused and comlain about by that, and just always pretend that it's a > normal directory, even if it's really a symlink. > > Also add a test case for this. + symlink($master_xlogdir, "$test_master_datadir/pg_xlog") or die; This will die on Windows, hence I think that this test should be skipped in this case. -- Michael
On 08/03/2015 03:54 PM, Michael Paquier wrote: > On Mon, Aug 3, 2015 at 9:34 PM, Heikki Linnakangas > <heikki.linnakangas@iki.fi> wrote: >> Fix pg_rewind when pg_xlog is a symlink. >> >> pg_xlog is often a symlink, typically to a different filesystem. Don't >> get confused and comlain about by that, and just always pretend that it's a >> normal directory, even if it's really a symlink. >> >> Also add a test case for this. > > + symlink($master_xlogdir, "$test_master_datadir/pg_xlog") or die; > > This will die on Windows, hence I think that this test should be > skipped in this case. Sure, I did have this at the top: +if ($windows_os) +{ + plan skip_all => 'symlinks not supported on Windows'; + exit; +} +else +{ + plan tests => 4; +} That's only on master, though. The TAP tests don't run on Windows in 9.5 anyway. I guess the pg_rewind tests used to work, but we didn't really advertise or make it easy to run it, so I'm not sure it's worth it to try to maintain that. Then again, we might want to backpatch all the TAP-test changes to make them work on Windows to 9.5, now that they've gotten some testing in the buildfarm and seem to work. BTW, I just noticed that the man page for perl's symlink says this: > On systems that don't support symbolic links, raises an exception. To check for that, use eval: > > $symlink_exists = eval { symlink("",""); 1 }; I wonder if we should be testing for that, instead of $windows_os. - Heikki
On Mon, Aug 3, 2015 at 10:37 PM, Heikki Linnakangas wrote: > That's only on master, though. The TAP tests don't run on Windows in 9.5 > anyway. Oops, yes I got mistaken by the commit on 9.5. > I guess the pg_rewind tests used to work, but we didn't really advertise or > make it easy to run it, so I'm not sure it's worth it to try to maintain > that. Then again, we might want to backpatch all the TAP-test changes to > make them work on Windows to 9.5, now that they've gotten some testing in > the buildfarm and seem to work. Usually new features are not backpatched, and the support for MSVC is one IMO. >> On systems that don't support symbolic links, raises an exception. To >> check for that, use eval: >> >> $symlink_exists = eval { symlink("",""); 1 }; > > > I wonder if we should be testing for that, instead of $windows_os. Yes, good point! The second platform referred as unsupported is RISC OS: http://perldoc.perl.org/perlport.html#symlink And Postgres can visibly work on it. It would be good to get for instance a RaspPI on the buildfarm with it, there is a development version. -- Michael
On 08/03/2015 09:05 PM, Michael Paquier wrote: > On Mon, Aug 3, 2015 at 10:37 PM, Heikki Linnakangas wrote: >> That's only on master, though. The TAP tests don't run on Windows in 9.5 >> anyway. > Oops, yes I got mistaken by the commit on 9.5. > >> I guess the pg_rewind tests used to work, but we didn't really advertise or >> make it easy to run it, so I'm not sure it's worth it to try to maintain >> that. Then again, we might want to backpatch all the TAP-test changes to >> make them work on Windows to 9.5, now that they've gotten some testing in >> the buildfarm and seem to work. > Usually new features are not backpatched, and the support for MSVC is one IMO. I think we can be rather more liberal with testing regimes that we can core server and client code. I have not yet tested these things with MSVC, but I can tell you: a) some more changes will be required to allow the tests to run on mingw/msys b) the rewind tests are currently failing very badly on my mingw/msys test rig. I'll be posting about that separately before long > >>> On systems that don't support symbolic links, raises an exception. To >>> check for that, use eval: >>> >>> $symlink_exists = eval { symlink("",""); 1 }; >> >> I wonder if we should be testing for that, instead of $windows_os. > Yes, good point! The second platform referred as unsupported is RISC OS: > http://perldoc.perl.org/perlport.html#symlink > And Postgres can visibly work on it. It would be good to get for > instance a RaspPI on the buildfarm with it, there is a development > version. The statement that Windows doesn't support symlinks isn't actually true. AS Perl in Windows doesn't (or might not, - I don't know about all versions). The test above doesn't fail on Msys DTK perl, but symlink calls in perl don't work either, So I think we'd better stay with the current test, or add the test above to the current test. cheers andrew