Re: PATCH: add "--config-file=" option to pg_rewind - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: PATCH: add "--config-file=" option to pg_rewind
Date
Msg-id Yhy4aJFIW1cWBQXD@paquier.xyz
Whole thread Raw
In response to Re: PATCH: add "--config-file=" option to pg_rewind  ("Gunnar \"Nick\" Bluth" <gunnar.bluth@pro-open.de>)
Responses Re: PATCH: add "--config-file=" option to pg_rewind  ("Gunnar \"Nick\" Bluth" <gunnar.bluth@pro-open.de>)
List pgsql-hackers
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

Attachment

pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: logical decoding and replication of sequences
Next
From: Peter Eisentraut
Date:
Subject: Re: psql: Make SSL info display more compact