Hi everyone! Apologies for the long silence — I was temporarily pulled away from this work and didn’t want to send an update until I could properly address the feedback.
I’m now back on track and have refined the implementation based on earlier discussions. The current version fully adopts the `include_if_exists` approach, writes temporary recovery settings to a separate config file, and disables it on exit by renaming to `.disabled`. Thank you for your patience — I appreciate any further review! --- Alena Vinter
From fc1a4bdf8ee44f4b7a2da81bd6a1164eef6855f4 Mon Sep 17 00:00:00 2001 From: Alena Vinter <[email protected]> Date: Tue, 30 Dec 2025 13:56:18 +0700 Subject: [PATCH 1/2] pg_createsubscriber: use include_if_exists for recovery config and disable on exit Write temporary recovery parameters to pg_createsubscriber.conf and include it from postgresql.auto.conf via include_if_exists. Upon completion or failure, rename the file to pg_createsubscriber.conf.disabled so it is ignored on subsequent restarts. --- src/bin/pg_basebackup/pg_createsubscriber.c | 77 ++++++++++++++----- .../t/040_pg_createsubscriber.pl | 34 ++++++++ 2 files changed, 90 insertions(+), 21 deletions(-) diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index dab4dfb3a52..81ab4641a31 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -29,9 +29,13 @@ #include "fe_utils/string_utils.h" #include "fe_utils/version.h" #include "getopt_long.h" +#include "common/file_utils.h" #define DEFAULT_SUB_PORT "50432" #define OBJECTTYPE_PUBLICATIONS 0x0001 +#define PG_AUTOCONF_FILENAME "postgresql.auto.conf" +#define INCLUDED_CONF_FILE "pg_createsubscriber.conf" +#define INCLUDED_CONF_FILE_DISABLED "pg_createsubscriber.conf.disabled" /* Command-line options */ struct CreateSubscriberOptions @@ -156,22 +160,43 @@ static char *subscriber_dir = NULL; static bool recovery_ended = false; static bool standby_running = false; - +static bool recovery_params_set = false; /* - * Cleanup objects that were created by pg_createsubscriber if there is an - * error. + * Clean up objects created by pg_createsubscriber. * - * Publications and replication slots are created on primary. Depending on the - * step it failed, it should remove the already created objects if it is - * possible (sometimes it won't work due to a connection issue). - * There is no cleanup on the target server. The steps on the target server are - * executed *after* promotion, hence, at this point, a failure means recreate - * the physical replica and start again. + * Publications and replication slots are created on the primary. Depending on + * the step where it failed, already created objects should be removed if + * possible (sometimes this won't work due to a connection issue). + * There is no cleanup on the target server *after* its promotion, because any + * failure at this point means recreating the physical replica and starting + * again. */ static void cleanup_objects_atexit(void) { + /* + * Disable the recovery configuration by renaming the + * "pg_createsubscriber.conf" file. Since "postgresql.auto.conf" uses + * `include_if_exists`, postgres will silently ignore the missing file. + * The renamed file is preserved fo debugging. + */ + if (recovery_params_set) + { + char conf_filename[MAXPGPATH]; + char conf_filename_disabled[MAXPGPATH]; + int err; + + snprintf(conf_filename, MAXPGPATH, "%s/%s", subscriber_dir, INCLUDED_CONF_FILE); + snprintf(conf_filename_disabled, MAXPGPATH, "%s/%s", subscriber_dir, INCLUDED_CONF_FILE_DISABLED); + + if ((err = durable_rename(conf_filename, conf_filename_disabled)) != 0) + { + pg_log_warning("recovery parameters were set but not properly reset on the target"); + pg_log_warning_hint("manual removal of recovery parameters is required from"); + } + } + if (success) return; @@ -1322,23 +1347,33 @@ setup_recovery(const struct LogicalRepInfo *dbinfo, const char *datadir, const c appendPQExpBufferStr(recoveryconfcontents, "recovery_target_name = ''\n"); appendPQExpBufferStr(recoveryconfcontents, "recovery_target_time = ''\n"); appendPQExpBufferStr(recoveryconfcontents, "recovery_target_xid = ''\n"); + appendPQExpBuffer(recoveryconfcontents, "recovery_target_lsn = '%s'\n", lsn); - if (dry_run) - { - appendPQExpBufferStr(recoveryconfcontents, "# dry run mode\n"); - appendPQExpBuffer(recoveryconfcontents, - "recovery_target_lsn = '%X/%08X'\n", - LSN_FORMAT_ARGS((XLogRecPtr) InvalidXLogRecPtr)); - } - else + pg_log_debug("recovery parameters:\n%s", recoveryconfcontents->data); + + if (!dry_run) { - appendPQExpBuffer(recoveryconfcontents, "recovery_target_lsn = '%s'\n", - lsn); + char conf_filename[MAXPGPATH]; + FILE *fd; + + recovery_params_set = true; + + snprintf(conf_filename, MAXPGPATH, "%s/%s", datadir, INCLUDED_CONF_FILE); + + fd = fopen(conf_filename, "w"); + if (fd == NULL) + pg_fatal("could not open file \"%s\": %m", conf_filename); + + if (fwrite(recoveryconfcontents->data, recoveryconfcontents->len, 1, fd) != 1) + pg_fatal("could not write to file \"%s\": %m", conf_filename); + + fclose(fd); + + resetPQExpBuffer(recoveryconfcontents); + appendPQExpBufferStr(recoveryconfcontents, "include_if_exists '" INCLUDED_CONF_FILE "'"); WriteRecoveryConfig(conn, datadir, recoveryconfcontents); } disconnect_database(conn, false); - - pg_log_debug("recovery parameters:\n%s", recoveryconfcontents->data); } /* diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl index 4657172c9ac..8a9200aead7 100644 --- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl +++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl @@ -160,6 +160,7 @@ primary_slot_name = '$slotname' primary_conninfo = '$pconnstr dbname=postgres' hot_standby_feedback = on ]); +my $sconnstr = $node_s->connstr; $node_s->set_standby_mode(); $node_s->start; @@ -579,10 +580,43 @@ is($result, qq($db1|{test_pub3} $db2|{pub2}), "subscriptions use the correct publications"); + +# Create a physical standby from the promoted subscriber +$node_s->safe_psql('postgres', + "SELECT pg_create_physical_replication_slot('$slotname');"); + +# Create backup from promoted subscriber +$node_s->backup('backup_3'); + +# Initialize new physical standby +my $node_k = PostgreSQL::Test::Cluster->new('node_k'); +$node_k->init_from_backup($node_s, 'backup_3', + has_streaming => 1); + +# Configure the new standby +$node_k->append_conf('postgresql.conf', qq[ +primary_slot_name = '$slotname' +primary_conninfo = '$sconnstr dbname=postgres' +hot_standby_feedback = on +]); + +$node_k->set_standby_mode(); +my $node_k_name = $node_s->name; +command_ok( + [ + 'pg_ctl', '--wait', + '--pgdata' => $node_k->data_dir, + '--log' => $node_k->logfile, + '--options' => "--cluster-name=$node_k_name", + 'start' + ], + "node_k has started"); + # clean up $node_p->teardown_node; $node_s->teardown_node; $node_t->teardown_node; $node_f->teardown_node; +$node_k->teardown_node; done_testing(); -- 2.52.0
From daa7126c061f39f1e345fbb0e98b33db07366885 Mon Sep 17 00:00:00 2001 From: Alena Vinter <[email protected]> Date: Tue, 30 Dec 2025 14:12:06 +0700 Subject: [PATCH 2/2] Doc: configuration file handling in pg_createsubscriber Update pg_createsubscriber.sgml to explain that recovery parameters are written to a separate pg_createsubscriber.conf file included via include_if_exists, and that the file is renamed to .disabled upon completion or failure to prevent reloading on restart. --- doc/src/sgml/ref/pg_createsubscriber.sgml | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml index e450c6a5b37..268fc414216 100644 --- a/doc/src/sgml/ref/pg_createsubscriber.sgml +++ b/doc/src/sgml/ref/pg_createsubscriber.sgml @@ -518,8 +518,11 @@ PostgreSQL documentation <step> <para> - Write recovery parameters into the target data directory and restart the - target server. It specifies an LSN (<xref + Write recovery parameters into the separate configuration file + <filename>pg_createsubscriber.conf</filename> that is included from + <filename>postgresql.auto.conf</filename> using + <literal>include_if_exists</literal> in the target data directory, then + restart the target server. It specifies an LSN (<xref linkend="guc-recovery-target-lsn"/>) of the write-ahead log location up to which recovery will proceed. It also specifies <literal>promote</literal> as the action that the server should take @@ -532,7 +535,10 @@ PostgreSQL documentation server ends standby mode and is accepting read-write transactions. If <option>--recovery-timeout</option> option is set, <application>pg_createsubscriber</application> terminates if recovery - does not end until the given number of seconds. + does not end until the given number of seconds. Upon completion or + failure, the included configuration file is renamed to + <filename>pg_createsubscriber.conf.disabled</filename> so it is no + longer loaded on subsequent restarts. </para> </step> -- 2.52.0
