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