diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index b0233786d4..77ae17073a 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -1152,13 +1152,13 @@ drop_failover_replication_slots(struct LogicalRepInfo *dbinfo) conn = connect_database(dbinfo[0].subconninfo, false); if (conn != NULL) { - /* Get failover replication slot names. */ + /* Get failover replication slot names */ res = PQexec(conn, "SELECT slot_name FROM pg_catalog.pg_replication_slots WHERE failover"); if (PQresultStatus(res) == PGRES_TUPLES_OK) { - /* Remove failover replication slots from subscriber. */ + /* Remove failover replication slots from subscriber */ for (int i = 0; i < PQntuples(res); i++) drop_replication_slot(conn, &dbinfo[0], PQgetvalue(res, i, 0)); } @@ -1310,7 +1310,11 @@ start_standby_server(const struct CreateSubscriberOptions *opt, bool restricted_ PQExpBuffer pg_ctl_cmd = createPQExpBuffer(); int rc; - appendPQExpBuffer(pg_ctl_cmd, "\"%s\" start -D \"%s\" -s -o \"-c sync_replication_slots=off\"", + /* + * FIXME: appending "sync_replication_slots=off" is not trivial for reader. + * Please separate it into the later part with comments. + */ + appendPQExpBuffer(pg_ctl_cmd, "\"%s\" start -D \"%s\" -s", pg_ctl_path, subscriber_dir); if (restricted_access) { @@ -1333,6 +1337,24 @@ start_standby_server(const struct CreateSubscriberOptions *opt, bool restricted_ if (opt->config_file != NULL) appendPQExpBuffer(pg_ctl_cmd, " -o \"-c config_file=%s\"", opt->config_file); + + /* + * FIXME: please describe why the sync_replication_slots is set to false. + * I.e., + */ + + /* + * We set up some recovery parameters in the middle part of the + * pg_createsubscriber to ensure all changes are caught up. At that time, + * primary_conninfo will be set without the dbname attribute. Basically, it + * works well, but if the slot sync is enabled, the slot sync worker will + * exit with ERROR because the process requires dbname within + * primary_conninfo. To avoid the error, we override sync_replication_slots + * to off. The synchronization won't happen after the conversion, so it is + * acceptable to stop here as well. + */ + appendPQExpBuffer(pg_ctl_cmd, " -o \"-c sync_replication_slots=off\""); + pg_log_debug("pg_ctl command is: %s", pg_ctl_cmd->data); rc = system(pg_ctl_cmd->data); pg_ctl_status(pg_ctl_cmd->data, rc); @@ -2068,6 +2090,7 @@ main(int argc, char **argv) /* Check if the primary server is ready for logical replication */ pub_failed = check_publisher(dbinfo); + /* Exit if either of them is not ready */ if (pub_failed || sub_failed) exit(1); @@ -2111,8 +2134,8 @@ main(int argc, char **argv) setup_subscriber(dbinfo, consistent_lsn); /* - * Remove primary_slot_name if it exists on primary. This replication slot - * has no use after the transformation. + * Remove primary_slot_name if it exists on primary. This replication + * slot has no use after the transformation. */ drop_primary_replication_slot(dbinfo, primary_slot_name); diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl index 0516d4e17e..cc65942dc4 100644 --- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl +++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl @@ -266,28 +266,43 @@ $node_p->restart; # Create failover slot to test its removal my $fslotname = 'failover_slot'; $node_p->safe_psql('pg1', - "SELECT pg_create_logical_replication_slot('$fslotname', 'pgoutput', false, false, true)"); + "SELECT pg_create_logical_replication_slot('$fslotname', 'pgoutput', false, false, true)" +); $node_s->start; $node_s->safe_psql('postgres', "SELECT pg_sync_replication_slots()"); -my $result = $node_s->safe_psql('postgres', "SELECT slot_name FROM pg_replication_slots WHERE slot_name = '$fslotname' AND synced AND NOT temporary"); +my $result = $node_s->safe_psql('postgres', + "SELECT slot_name FROM pg_replication_slots WHERE slot_name = '$fslotname' AND synced AND NOT temporary" +); is($result, 'failover_slot', 'failover slot is synced'); $node_s->stop; # dry run mode on node S command_ok( [ - 'pg_createsubscriber', '--verbose', - '--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default", - '--dry-run', '--pgdata', - $node_s->data_dir, '--publisher-server', - $node_p->connstr('pg1'), '--socket-directory', - $node_s->host, '--subscriber-port', - $node_s->port, '--publication', - 'pub1', '--publication', - 'pub2', '--subscription', - 'sub1', '--subscription', - 'sub2', '--database', - 'pg1', '--database', + 'pg_createsubscriber', + '--verbose', + '--recovery-timeout', + "$PostgreSQL::Test::Utils::timeout_default", + '--dry-run', + '--pgdata', + $node_s->data_dir, + '--publisher-server', + $node_p->connstr('pg1'), + '--socket-directory', + $node_s->host, + '--subscriber-port', + $node_s->port, + '--publication', + 'pub1', + '--publication', + 'pub2', + '--subscription', + 'sub1', + '--subscription', + 'sub2', + '--database', + 'pg1', + '--database', 'pg2' ], 'run pg_createsubscriber --dry-run on node S'); @@ -314,18 +329,30 @@ command_ok( # Run pg_createsubscriber on node S command_ok( [ - 'pg_createsubscriber', '--verbose', - '--recovery-timeout', "$PostgreSQL::Test::Utils::timeout_default", - '--verbose', '--pgdata', - $node_s->data_dir, '--publisher-server', - $node_p->connstr('pg1'), '--socket-directory', - $node_s->host, '--subscriber-port', - $node_s->port, '--publication', - 'pub1', '--publication', - 'Pub2', '--replication-slot', - 'replslot1', '--replication-slot', - 'replslot2', '--database', - 'pg1', '--database', + 'pg_createsubscriber', + '--verbose', + '--recovery-timeout', + "$PostgreSQL::Test::Utils::timeout_default", + '--verbose', + '--pgdata', + $node_s->data_dir, + '--publisher-server', + $node_p->connstr('pg1'), + '--socket-directory', + $node_s->host, + '--subscriber-port', + $node_s->port, + '--publication', + 'pub1', + '--publication', + 'Pub2', + '--replication-slot', + 'replslot1', + '--replication-slot', + 'replslot2', + '--database', + 'pg1', + '--database', 'pg2' ], 'run pg_createsubscriber on node S'); @@ -358,7 +385,8 @@ $node_s->wait_for_subscription_sync($node_p, $subnames[1]); # Confirm the failover slot has been removed $result = $node_s->safe_psql('pg1', - "SELECT count(*) FROM pg_replication_slots WHERE slot_name = '$fslotname'"); + "SELECT count(*) FROM pg_replication_slots WHERE slot_name = '$fslotname'" +); is($result, qq(0), 'failover slot was removed'); # Check result on database pg1