> > > I went through the remaining two patches and they seem to be very clear > and concise. However, there are two points I could complain about: > > 1) Maybe I've missed it somewhere in the thread above, but currently > pg_rewind allows to run itself with -R and --source-pgdata. In that case > -R option is just swallowed and neither standby.signal, nor > postgresql.auto.conf is written, which is reasonable though. Should it > be stated somehow in the docs that -R option always has to go altogether > with --source-server? Or should pg_rewind notify user that options are > incompatible and no recovery configuration will be written? >
I modified code & doc to address this. In code, pg_rewind will error out for the local case. > 2) Are you going to leave -R option completely without tap-tests? > Attached is a small patch, which tests -R option along with the existing > 'remote' case. If needed it may be split into two separate cases. First, > it tests that pg_rewind is able to succeed with minimal permissions > according to the Michael's patch d9f543e [1]. Next, it checks presence > of standby.signal and adds REPLICATION permission to rewind_user to test > that new standby is able to start with generated recovery configuration. > > [1] > > https://github.com/postgres/postgres/commit/d9f543e9e9be15f92abdeaf870e57ef289020191 > > It seems that we could further disabling recovery info setting code for the 'remote' test case? - my $port_standby = $node_standby->port; - $node_master->append_conf( - 'postgresql.conf', qq( -primary_conninfo='port=$port_standby' -)); + if ($test_mode ne "remote") + { + my $port_standby = $node_standby->port; + $node_master->append_conf( + 'postgresql.conf', + qq(primary_conninfo='port=$port_standby')); - $node_master->set_standby_mode(); + $node_master->set_standby_mode(); + } Thanks.
v10-0001-Add-option-to-write-recovery-configuration-infor.patch
Description: Binary data
