On Tue, Apr 05, 2022 at 11:10:30AM +0900, Michael Paquier wrote: > In the tests, the only difference between the modes "archive_cli" and > "archive" is the extra option given to the pg_rewind command, and > that's a simple redirect to what pg_rewind would choose by default > anyway. A more elegant solution would be to have an extra option in > RewindTest::run_pg_rewind(), where any caller of the routine can feed > extra options to the pg_rewind command. Now, in this case, we are > not going to lose any coverage if the existing "archive" mode is > changed so as we move away postgresql.conf from the target data folder > and just use --config-file by default, no? I would make the choice of > simplicity, by giving up on "archive_cli" and using > primary-postgresql.conf.tmp as value for --config-file. There could > be an argument for using --config-file for all the modes, as well.
The clock is ticking, so I have looked at this patch by myself. I have finished by tweaking the docs, the code and the tests as of the attached. One thing that I found annoying is the lack of description about the fact that this option affects pg_rewind when it internally starts the target cluster, as of when we retrieve restore_command and when we enforce crash recovery to have a target cluster with a clean shutdown state. The tests are heavily simplified, having no impact on the run-time while improving the coverage for the code paths changed here. -- Michael
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index b39b5c1aac..59838d6a07 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -61,6 +61,7 @@ char *datadir_target = NULL;
char *datadir_source = NULL;
char *connstr_source = NULL;
char *restore_command = NULL;
+char *config_file = NULL;
static bool debug = false;
bool showprogress = false;
@@ -87,6 +88,8 @@ usage(const char *progname)
printf(_("Options:\n"));
printf(_(" -c, --restore-target-wal use restore_command in target configuration to\n"
" retrieve WAL files from archives\n"));
+ printf(_(" --config-file=FILE use specified main server configuration\n"));
+ printf(_(" file when running target cluster\n"));
printf(_(" -D, --target-pgdata=DIRECTORY existing data directory to modify\n"));
printf(_(" --source-pgdata=DIRECTORY source data directory to synchronize with\n"));
printf(_(" --source-server=CONNSTR source server to synchronize with\n"));
@@ -115,6 +118,7 @@ main(int argc, char **argv)
{"source-pgdata", required_argument, NULL, 1},
{"source-server", required_argument, NULL, 2},
{"no-ensure-shutdown", no_argument, NULL, 4},
+ {"config-file", required_argument, NULL, 5},
{"version", no_argument, NULL, 'V'},
{"restore-target-wal", no_argument, NULL, 'c'},
{"dry-run", no_argument, NULL, 'n'},
@@ -205,6 +209,10 @@ main(int argc, char **argv)
case 4:
no_ensure_shutdown = true;
break;
+
+ case 5:
+ config_file = pg_strdup(optarg);
+ break;
}
}
@@ -1061,6 +1069,13 @@ getRestoreCommand(const char *argv0)
appendPQExpBufferStr(postgres_cmd, " -D ");
appendShellString(postgres_cmd, datadir_target);
+ /* add custom configuration file only if requested */
+ if (config_file != NULL)
+ {
+ appendPQExpBufferStr(postgres_cmd, " -c config_file=");
+ appendShellString(postgres_cmd, config_file);
+ }
+
/* add -C switch, for restore_command */
appendPQExpBufferStr(postgres_cmd, " -C restore_command");
@@ -1139,6 +1154,13 @@ ensureCleanShutdown(const char *argv0)
appendPQExpBufferStr(postgres_cmd, " --single -F -D ");
appendShellString(postgres_cmd, datadir_target);
+ /* add custom configuration file only if requested */
+ if (config_file != NULL)
+ {
+ appendPQExpBufferStr(postgres_cmd, " -c config_file=");
+ appendShellString(postgres_cmd, config_file);
+ }
+
/* finish with the database name, and a properly quoted redirection */
appendPQExpBufferStr(postgres_cmd, " template1 < ");
appendShellString(postgres_cmd, DEVNULL);
diff --git a/src/bin/pg_rewind/t/RewindTest.pm b/src/bin/pg_rewind/t/RewindTest.pm
index 1e34768e27..8fd1f4b9de 100644
--- a/src/bin/pg_rewind/t/RewindTest.pm
+++ b/src/bin/pg_rewind/t/RewindTest.pm
@@ -263,7 +263,9 @@ sub run_pg_rewind
"--debug",
"--source-pgdata=$standby_pgdata",
"--target-pgdata=$primary_pgdata",
- "--no-sync"
+ "--no-sync",
+ "--config-file",
+ "$tmp_folder/primary-postgresql.conf.tmp"
],
'pg_rewind local');
}
@@ -276,7 +278,8 @@ sub run_pg_rewind
'pg_rewind', "--debug",
"--source-server", $standby_connstr,
"--target-pgdata=$primary_pgdata", "--no-sync",
- "--write-recovery-conf"
+ "--write-recovery-conf", "--config-file",
+ "$tmp_folder/primary-postgresql.conf.tmp"
],
'pg_rewind remote');
@@ -323,7 +326,8 @@ sub run_pg_rewind
# Note the use of --no-ensure-shutdown here. WAL files are
# gone in this mode and the primary has been stopped
- # gracefully already.
+ # gracefully already. --config-file reuses the original
+ # postgresql.conf as restore_command has been enabled above.
command_ok(
[
'pg_rewind',
@@ -332,7 +336,9 @@ sub run_pg_rewind
"--target-pgdata=$primary_pgdata",
"--no-sync",
"--no-ensure-shutdown",
- "--restore-target-wal"
+ "--restore-target-wal",
+ "--config-file",
+ "$primary_pgdata/postgresql.conf"
],
'pg_rewind archive');
}
diff --git a/doc/src/sgml/ref/pg_rewind.sgml b/doc/src/sgml/ref/pg_rewind.sgml
index 33e6bb64ad..e808239aa5 100644
--- a/doc/src/sgml/ref/pg_rewind.sgml
+++ b/doc/src/sgml/ref/pg_rewind.sgml
@@ -241,6 +241,21 @@ PostgreSQL documentation
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--config-file=<replaceable class="parameter">filename</replaceable></option></term>
+ <listitem>
+ <para>
+ Use the specified main server configuration file for the target
+ cluster. This affects <application>pg_rewind</application> when
+ it uses internally the <application>postgres</application> command
+ for the rewind operation on this cluster (when retrieving
+ <varname>restore_command</varname> with the option
+ <option>-c/--restore-target-wal</option> and when forcing a
+ completion of crash recovery).
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--debug</option></term>
<listitem>
signature.asc
Description: PGP signature
