Re: PATCH: add "--config-file=" option to pg_rewind

On Sun, Feb 27, 2022 at 02:30:33PM +0100, Gunnar "Nick" Bluth wrote:
> That's universally true ;-)

-# Internal routine to enable archive recovery command on a standby node
+# Internal routine to enable archive recovery command on a standby node.
+# Returns generated restore_command.
 sub enable_restoring
 {
 	my ($self, $root_node, $standby) = @_;
@@ -1103,7 +1104,7 @@ restore_command = '$copy_command'
 	{
 		$self->set_recovery_mode();
 	}
-	return;
+	return $copy_command;

I don't like this change.  This makes an existing routine do some
extra work for something it is not designed for.  You could get this
information with a postgres -C command, for example.  Now, you cannot
use postgres -C because of..  Reasons (1a9d802).  But you could use a
pg_ctl command for the same purpose.  I guess that we'd better
refactor this into a new routine of Cluster.pm where we execute a
pg_ctl command and return both stdout and stderr back to the caller?

The TAP part could be refactored on its own.

+        In case the <filename>postgresql.conf</filename> of your
target cluster is not in the
+        target pgdata and you want to use the <option>-c /
--restore-target-wal</option> option,
+        provide a (relative or absolute) path to the
<filename>postgresql.conf</filename> using this option.

This could do a better job to explain in which cases this option can
be used for.  You could say something like that:
"This option specifies a path to a configuration file to be used when
starting the target cluster.  This makes easier the use of
-c/--restore-target-wal by setting restore_command in the extra
configuration file specified via this option."

Hmm.  What about the target cluster started in --single mode as of
ensureCleanShutdown()?  Should we try to apply this option also in
this case?
--
Michael