From ac319b445cf501fe0349590d9a4d2225a419d660 Mon Sep 17 00:00:00 2001 From: Alena Vinter Date: Tue, 2 Sep 2025 18:15:13 +0700 Subject: [PATCH 2/3] Reseting recovery target parameters in pg_createsubscriber The utility sets recovery target params for correct recovery before conversion a physical replica to a logical one but does not reset them afterward. It may cause recovery failures in certain scenarios. For example, if recovery begins from a checkpoint where no WAL records need to be applied, the system may incorrectly determine that the recovery target was never reached because these parameters remain active. This change ensures all recovery parameters are properly reset after conversion to prevent such edge cases. --- src/bin/pg_basebackup/pg_createsubscriber.c | 56 ++++++++++++++----- .../t/040_pg_createsubscriber.pl | 31 ++++++++++ 2 files changed, 74 insertions(+), 13 deletions(-) diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index 3986882f042..dccf5dcbc87 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -154,6 +154,7 @@ static char *subscriber_dir = NULL; static bool recovery_ended = false; static bool standby_running = false; +static bool recovery_params_set = false; enum WaitPMResult { @@ -161,21 +162,51 @@ enum WaitPMResult POSTMASTER_STILL_STARTING }; +/* + * Buffer to preserve the original recovery conf contents before modifying + * recovery parameters. This allows restoration of the original configuration + * after the logical replication process completes, maintaining the system's + * previous recovery state. + */ +static PQExpBuffer savedrecoveryconfcontents = NULL; + +/* + * Cached version of the subscriber server, used for version-specific behavior + */ +static int sub_version = 0; /* - * Cleanup objects that were created by pg_createsubscriber if there is an - * error. + * Cleanup objects that were 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. + * There is no cleanup on the target server *after* its promotion because any + * failure at this point means recreate the physical replica and start again. */ static void cleanup_objects_atexit(void) { + /* + * Replace recovery config with its original contents. This ensures the + * subscriber doesn't try using outdated recovery parameters. + */ + if (recovery_params_set) + { + bool no_err = ReplaceRecoveryConfig(sub_version, subscriber_dir, + savedrecoveryconfcontents); + + if (!no_err) + { + bool use_recovery_conf = + sub_version < MINIMUM_VERSION_FOR_RECOVERY_GUC; + + 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 '%s'", + use_recovery_conf ? "recovery.conf" : "postgresql.auto.conf"); + } + } + if (success) return; @@ -1019,6 +1050,7 @@ check_subscriber(const struct LogicalRepInfo *dbinfo) pg_log_info("checking settings on subscriber"); conn = connect_database(dbinfo[0].subconninfo, true); + sub_version = PQserverVersion(conn); /* The target server must be a standby */ if (!server_is_in_recovery(conn)) @@ -1236,6 +1268,9 @@ setup_recovery(const struct LogicalRepInfo *dbinfo, const char *datadir, const c */ conn = connect_database(dbinfo[0].pubconninfo, true); + /* Before setting up the recovery parameters save the original content. */ + savedrecoveryconfcontents = GetRecoveryConfig(conn, datadir); + /* * Write recovery parameters. * @@ -1267,17 +1302,12 @@ setup_recovery(const struct LogicalRepInfo *dbinfo, const char *datadir, const c appendPQExpBufferStr(recoveryconfcontents, "recovery_target_time = ''\n"); appendPQExpBufferStr(recoveryconfcontents, "recovery_target_xid = ''\n"); - if (dry_run) - { - appendPQExpBufferStr(recoveryconfcontents, "# dry run mode"); - appendPQExpBuffer(recoveryconfcontents, - "recovery_target_lsn = '%X/%08X'\n", - LSN_FORMAT_ARGS((XLogRecPtr) InvalidXLogRecPtr)); - } - else + if (!dry_run) { appendPQExpBuffer(recoveryconfcontents, "recovery_target_lsn = '%s'\n", lsn); + + recovery_params_set = true; WriteRecoveryConfig(conn, datadir, recoveryconfcontents); } disconnect_database(conn, false); diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl index 229fef5b3b5..6dee0837ee9 100644 --- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl +++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl @@ -41,6 +41,30 @@ sub generate_db return $dbname; } +# Check if a parameter is absent from the server's configuration file. +# Uses 'recovery.conf' for PostgreSQL < 12, 'postgresql.auto.conf' for newer +# versions. +sub test_param_absent +{ + my ($node, $param) = @_; + my $version = $node->pg_version->major; + my $conf; + + if ($version < 12) + { + $conf = $node->data_dir . '/recovery.conf'; + } + else + { + $conf = $node->data_dir . '/postgresql.auto.conf'; + } + + return 1 unless -e $conf; + + my $content = slurp_file($conf); + return $content !~ /^\s*$param\s*=/m; +} + # # Test mandatory options command_fails(['pg_createsubscriber'], @@ -467,6 +491,13 @@ command_ok( ], 'run pg_createsubscriber on node S'); +# Verify that recovery parameters have been reset after pg_createsubscriber +# We check recovery_target_lsn as a representative parameter - since all +# recovery parameters are managed as a group, the absence of one indicates +# that the entire set has been properly cleared from the configuration. +ok( test_param_absent($node_s, 'recovery_target_lsn'), + 'recovery_target_lsn parameter was removed'); + # Confirm the physical replication slot has been removed $result = $node_p->safe_psql($db1, "SELECT count(*) FROM pg_replication_slots WHERE slot_name = '$slotname'" -- 2.51.0