On Fri, Mar 22, 2019 at 04:25:53PM +0800, Shaoqi Bai wrote: > Deleted the test for group permissions in updated patch.
Well, there are a couple of things I am not really happy about in this patch: - There is not much point to have has_tablespace_mapping as it is not extensible. Instead I'd rather use a single "extra" parameter which can define a range of options. An example of that is within PostgresNode::init for "extra" and "auth_extra". - CREATE TABLESPACE is run once on the primary *before* promoting the standby, which causes the tablespace paths to map between both of them. This is not correct. Creating a tablespace on the primary before creating the standby, and use --tablespace-map would be the way to go... However per the next point... - standby_afterpromotion is created on the promoted standby, and then immediately dropped. pg_rewind is able to handle this case when working on different hosts. But with this test we finish by having the same problem as pg_basebackup: the source and the target server finish by eating each other. I think that this could actually be an interesting feature for pg_rewind. - A comment at the end refers to databases, and not tablespaces. You could work out the first problem with the backup by changing the backup()/init_from_backup() in RewindTest::create_standby by a pure call to pg_basebackup, but you still have the second problem, which we should still be able to test, and this requires more facility in pg_rewind so as it is basically possible to hijack create_target_symlink() to create a symlink to a different path than the initial one. > Checking the RecursiveCopy::copypath being called, only _backup_fs and > init_from_backup called it. > After runing cmd make -C src/bin check in updated patch, seeing no failure. Yes, I can see that. The issue is that even if we do a backup with --tablespace-mapping then we still need a tweak to allow the copy of symlinks. I am not sure that this is completely what we are looking for either, as it means that any test setting a primary with a tablespace and two standbys initialized from the same base backup would fail. That's not really portable. -- Michael
signature.asc
Description: PGP signature
