From 7442d53ed32d8846a5aa018210a538d843df83c6 Mon Sep 17 00:00:00 2001 From: Khanna Date: Thu, 20 Mar 2025 13:52:27 +0530 Subject: [PATCH 1/3] Enhance 'pg_createsubscriber' to fetch and append all databases This patch enhances the 'pg_createsubscriber' utility by adding the '--all' option. When '--all' is specified, the tool queries the source server (publisher) for all databases and creates subscriptions on the target server (subscriber) for databases with matching names. This simplifies the process of converting a physical standby to a logical subscriber, particularly during upgrades. The options '--database', '--publication', '--subscription', and '--replication-slot' cannot be used when '--all' is specified. --- doc/src/sgml/ref/pg_createsubscriber.sgml | 37 ++++-- src/bin/pg_basebackup/pg_createsubscriber.c | 106 +++++++++++++++++- .../t/040_pg_createsubscriber.pl | 57 ++++++++++ 3 files changed, 189 insertions(+), 11 deletions(-) diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml index d011b79e5e6d..1f0ddd7f9f2c 100644 --- a/doc/src/sgml/ref/pg_createsubscriber.sgml +++ b/doc/src/sgml/ref/pg_createsubscriber.sgml @@ -87,6 +87,24 @@ PostgreSQL documentation command-line arguments: + + + + + + Create one subscription per database on the target server. Exceptions + are template databases and databases that don't allow connections. + If the database name is not specified in publisher-server, the postgres + database will be used, or if that does not exist, template1 will be used. + Automatically generated names for subscriptions, publications, and + replication slots are used when this option is specified. + This option cannot be used along with , + , , or + . + + + + @@ -94,10 +112,12 @@ PostgreSQL documentation The name of the database in which to create a subscription. Multiple databases can be selected by writing multiple - switches. If option is not provided, the database - name will be obtained from option. If the database - name is not specified in either the option or - option, an error will be reported. + switches. This option cannot be used together with . + If option is not provided, the database name will be + obtained from option. If the database name is not + specified in either the option, or the + option, and option is not + specified, an error will be reported. @@ -253,7 +273,8 @@ PostgreSQL documentation names must match the number of specified databases, otherwise an error is reported. The order of the multiple publication name switches must match the order of database switches. If this option is not specified, - a generated name is assigned to the publication name. + a generated name is assigned to the publication name. This option cannot + be used together with . @@ -269,7 +290,8 @@ PostgreSQL documentation otherwise an error is reported. The order of the multiple replication slot name switches must match the order of database switches. If this option is not specified, the subscription name is assigned to the - replication slot name. + replication slot name. This option cannot be used together with + . @@ -284,7 +306,8 @@ PostgreSQL documentation names must match the number of specified databases, otherwise an error is reported. The order of the multiple subscription name switches must match the order of database switches. If this option is not specified, - a generated name is assigned to the subscription name. + a generated name is assigned to the subscription name. This option cannot + be used together with . diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c index e2d6b7544bfe..d0cff80d2b4d 100644 --- a/src/bin/pg_basebackup/pg_createsubscriber.c +++ b/src/bin/pg_basebackup/pg_createsubscriber.c @@ -45,6 +45,7 @@ struct CreateSubscriberOptions SimpleStringList sub_names; /* list of subscription names */ SimpleStringList replslot_names; /* list of replication slot names */ int recovery_timeout; /* stop recovery after this time */ + bool all_dbs; /* all option */ SimpleStringList objecttypes_to_remove; /* list of object types to remove */ }; @@ -124,6 +125,8 @@ static void check_and_drop_existing_subscriptions(PGconn *conn, const struct LogicalRepInfo *dbinfo); static void drop_existing_subscriptions(PGconn *conn, const char *subname, const char *dbname); +static void get_publisher_databases(struct CreateSubscriberOptions *opt, + bool dbnamespecified); #define USEC_PER_SEC 1000000 #define WAIT_INTERVAL 1 /* 1 second */ @@ -243,6 +246,8 @@ usage(void) printf(_("Usage:\n")); printf(_(" %s [OPTION]...\n"), progname); printf(_("\nOptions:\n")); + printf(_(" -a, --all create subscriptions for all databases except template\n" + " databases or databases with connection restrictions\n")); printf(_(" -d, --database=DBNAME database in which to create a subscription\n")); printf(_(" -D, --pgdata=DATADIR location for the subscriber data directory\n")); printf(_(" -n, --dry-run dry run, just show what would be done\n")); @@ -1959,11 +1964,67 @@ enable_subscription(PGconn *conn, const struct LogicalRepInfo *dbinfo) destroyPQExpBuffer(str); } +/* + * Fetch a list of all not-template databases from the source server. + * Internally, this is treated as if the user specified multiple --database + * options, one for each source database. + */ +static void +get_publisher_databases(struct CreateSubscriberOptions *opt, + bool dbnamespecified) +{ + PGconn *conn; + PGresult *res; + + /* If a database name was specified, just connect to it. */ + if (dbnamespecified) + conn = connect_database(opt->pub_conninfo_str, true); + else + { + /* Otherwise, try postgres first and then template1. */ + char *conninfo; + + conninfo = concat_conninfo_dbname(opt->pub_conninfo_str, "postgres"); + conn = connect_database(conninfo, false); + pg_free(conninfo); + if (!conn) + { + conninfo = concat_conninfo_dbname(opt->pub_conninfo_str, "template1"); + conn = connect_database(conninfo, true); + pg_free(conninfo); + } + } + + res = PQexec(conn, "SELECT datname FROM pg_database WHERE datistemplate = false AND datallowconn AND datconnlimit <> -2 ORDER BY 1"); + + /* Check for errors during query execution */ + if (PQresultStatus(res) != PGRES_TUPLES_OK) + { + pg_log_error("could not obtain a list of databases: %s", PQresultErrorMessage(res)); + PQclear(res); + disconnect_database(conn, true); + } + + for (int i = 0; i < PQntuples(res); i++) + { + const char *dbname = PQgetvalue(res, i, 0); + + simple_string_list_append(&opt->database_names, dbname); + + /* Increment num_dbs to reflect multiple --database options */ + num_dbs++; + } + + PQclear(res); + disconnect_database(conn, false); +} + int main(int argc, char **argv) { static struct option long_options[] = { + {"all", no_argument, NULL, 'a'}, {"database", required_argument, NULL, 'd'}, {"pgdata", required_argument, NULL, 'D'}, {"dry-run", no_argument, NULL, 'n'}, @@ -2034,6 +2095,7 @@ main(int argc, char **argv) 0 }; opt.recovery_timeout = 0; + opt.all_dbs = false; /* * Don't allow it to be run as root. It uses pg_ctl which does not allow @@ -2051,11 +2113,14 @@ main(int argc, char **argv) get_restricted_token(); - while ((c = getopt_long(argc, argv, "d:D:np:P:R:s:t:TU:v", + while ((c = getopt_long(argc, argv, "ad:D:np:P:R:s:t:TU:v", long_options, &option_index)) != -1) { switch (c) { + case 'a': + opt.all_dbs = true; + break; case 'd': if (!simple_string_list_member(&opt.database_names, optarg)) { @@ -2149,6 +2214,28 @@ main(int argc, char **argv) } } + /* Validate that --all is not used with incompatible options */ + if (opt.all_dbs) + { + char *bad_switch = NULL; + + if (num_dbs > 0) + bad_switch = "--database"; + else if (num_pubs > 0) + bad_switch = "--publication"; + else if (num_replslots > 0) + bad_switch = "--replication-slot"; + else if (num_subs > 0) + bad_switch = "--subscription"; + + if (bad_switch) + { + pg_log_error("%s cannot be used with --all", bad_switch); + pg_log_error_hint("Try \"%s --help\" for more information.", progname); + exit(1); + } + } + /* Any non-option arguments? */ if (optind < argc) { @@ -2202,14 +2289,25 @@ main(int argc, char **argv) pg_log_info("validating subscriber connection string"); sub_base_conninfo = get_sub_conninfo(&opt); + /* + * Fetch all databases from the source (publisher) if --all is specified. + * This is treated as if the user specified multiple --database options, + * one for each source database. + */ + if (opt.all_dbs) + { + bool dbnamespecified = (dbname_conninfo != NULL); + + get_publisher_databases(&opt, dbnamespecified); + } + if (opt.database_names.head == NULL) { pg_log_info("no database was specified"); /* - * If --database option is not provided, try to obtain the dbname from - * the publisher conninfo. If dbname parameter is not available, error - * out. + * Try to obtain the dbname from the publisher conninfo. If dbname + * parameter is not available, error out. */ if (dbname_conninfo) { diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl index f7a980ec7991..80153f7d77e8 100644 --- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl +++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl @@ -386,6 +386,63 @@ sub generate_db ], 'run pg_createsubscriber without --databases'); +# run pg_createsubscriber with '--database' and '--all' without '--dry-run' +# and verify the failure +command_fails_like( + [ + 'pg_createsubscriber', + '--verbose', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--database' => $db1, + '--all', + ], + qr/--database cannot be used with --all/, + 'fail if --database is used with --all'); + +# run pg_createsubscriber with '--publication' and '--all' and verify +# the failure +command_fails_like( + [ + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--all', + '--publication' => 'pub1', + ], + qr/--publication cannot be used with --all/, + 'fail if --publication is used with --all'); + +# run pg_createsubscriber with '--all' option +my ($stdout, $stderr) = run_command( + [ + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default, + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr, + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--all', + ], + 'run pg_createsubscriber with --all'); + +# Verify that the required logical replication objects are output. +# The expected count 3 refers to postgres, $db1 and $db2 databases. +is(scalar(() = $stderr =~ /creating publication/g), + 3, "verify publications are created for all databases"); +is(scalar(() = $stderr =~ /creating the replication slot/g), + 3, "verify replication slots are created for all databases"); +is(scalar(() = $stderr =~ /creating subscription/g), + 3, "verify subscriptions are created for all databases"); + # Run pg_createsubscriber on node S. --verbose is used twice # to show more information. # In passing, also test the --enable-two-phase option and From 57368743be6718538094576e81cdbacbf77a6675 Mon Sep 17 00:00:00 2001 From: Khanna Date: Fri, 21 Mar 2025 12:29:41 +0530 Subject: [PATCH 2/3] Synopsis for --all option This patch contains the synopsis for the --all option. --- doc/src/sgml/ref/pg_createsubscriber.sgml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/doc/src/sgml/ref/pg_createsubscriber.sgml b/doc/src/sgml/ref/pg_createsubscriber.sgml index 1f0ddd7f9f2c..8d8574d7b5f1 100644 --- a/doc/src/sgml/ref/pg_createsubscriber.sgml +++ b/doc/src/sgml/ref/pg_createsubscriber.sgml @@ -20,6 +20,27 @@ PostgreSQL documentation + + pg_createsubscriber + option + + + + + + + + + + datadir + + + + + connstr + + + pg_createsubscriber option From e5ee2abb92ab890af90a682a5e0dccceff891464 Mon Sep 17 00:00:00 2001 From: Khanna Date: Sat, 22 Mar 2025 19:08:30 +0530 Subject: [PATCH 3/3] Additional test cases This patch contains the additional test cases related to the --all option. --- .../t/040_pg_createsubscriber.pl | 151 ++++++++++++++++-- 1 file changed, 141 insertions(+), 10 deletions(-) diff --git a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl index 80153f7d77e8..f6da373ff598 100644 --- a/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl +++ b/src/bin/pg_basebackup/t/040_pg_createsubscriber.pl @@ -41,6 +41,25 @@ sub generate_db return $dbname; } +# Wait for subscriptions on the subscriber to catch up all changes. +sub wait_for_all_subscriptions_caught_up +{ + my ($node_p, $node_s) = @_; + + # Get subscription names + my $result = $node_s->safe_psql( + 'postgres', qq( + SELECT subname FROM pg_subscription WHERE subname ~ '^pg_createsubscriber_' + )); + my @subnames = split("\n", $result); + + # Wait for all subscriptions to catch up + foreach my $subname (@subnames) + { + $node_p->wait_for_catchup($subname); + } +} + # # Test mandatory options command_fails(['pg_createsubscriber'], @@ -386,6 +405,23 @@ sub generate_db ], 'run pg_createsubscriber without --databases'); +# run pg_createsubscriber with '--all' and '--database' and verify the +# failure +command_fails_like( + [ + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--all', + '--database' => $db1, + ], + qr/--database cannot be used with --all/, + 'fail if --database is used with --all'); + # run pg_createsubscriber with '--database' and '--all' without '--dry-run' # and verify the failure command_fails_like( @@ -419,6 +455,40 @@ sub generate_db qr/--publication cannot be used with --all/, 'fail if --publication is used with --all'); +# run pg_createsubscriber with '--replication-slot' and '--all' and +# verify the failure +command_fails_like( + [ + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--replication-slot' => 'replslot1', + '--all', + ], + qr/--replication-slot cannot be used with --all/, + 'fail if --replication-slot is used with --all'); + +# run pg_createsubscriber with '--subscription' and '--all' and +# verify the failure +command_fails_like( + [ + 'pg_createsubscriber', + '--verbose', + '--dry-run', + '--pgdata' => $node_s->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_s->host, + '--subscriber-port' => $node_s->port, + '--all', + '--subscription' => 'sub1', + ], + qr/--subscription cannot be used with --all/, + 'fail if --subscription is used with --all'); + # run pg_createsubscriber with '--all' option my ($stdout, $stderr) = run_command( [ @@ -502,16 +572,7 @@ sub generate_db )); is($result, qq(0), 'pre-existing subscription was dropped'); -# Get subscription names -$result = $node_s->safe_psql( - 'postgres', qq( - SELECT subname FROM pg_subscription WHERE subname ~ '^pg_createsubscriber_' -)); -my @subnames = split("\n", $result); - -# Wait subscriber to catch up -$node_s->wait_for_subscription_sync($node_p, $subnames[0]); -$node_s->wait_for_subscription_sync($node_p, $subnames[1]); +wait_for_all_subscriptions_caught_up($node_p, $node_s); # Confirm the failover slot has been removed $result = $node_s->safe_psql($db1, @@ -537,10 +598,80 @@ sub generate_db 'SELECT system_identifier FROM pg_control_system()'); ok($sysid_p != $sysid_s, 'system identifier was changed'); +$node_s->stop; + +# Drop the database $db2 to verify subscriptions are handled correctly +$node_p->safe_psql('postgres', "DROP DATABASE \"$db2\""); + +# On node P create a test table +$node_p->safe_psql('postgres', 'CREATE TABLE tbl1 (a text)'); + +# Set up node U as standby linking to node P +$node_p->backup('backup_3'); +my $node_u = PostgreSQL::Test::Cluster->new('node_u'); +$node_u->init_from_backup($node_p, 'backup_3', has_streaming => 1); +$node_u->set_standby_mode(); + +# run pg_createsubscriber with '--all' option without '--dry-run' +command_ok( + [ + 'pg_createsubscriber', + '--verbose', + '--recovery-timeout' => $PostgreSQL::Test::Utils::timeout_default, + '--pgdata' => $node_u->data_dir, + '--publisher-server' => $node_p->connstr($db1), + '--socketdir' => $node_u->host, + '--subscriber-port' => $node_u->port, + '--all', + ], + 'run pg_createsubscriber with --all'); + +$node_u->start; + +# Verify that user databases (postgres, $db1) got subscriptions. +$result = $node_u->safe_psql( + 'postgres', + 'SELECT datname FROM pg_subscription, + pg_database WHERE subdbid = pg_database.oid and datistemplate = \'f\' ORDER BY pg_database.oid' +); +is( $result, "postgres +$db1", 'subscription is created on the required databases'); + +# Verify template databases do not have subscriptions +$result = $node_u->safe_psql( + 'postgres', + "SELECT count(*) FROM pg_subscription, pg_database + WHERE subdbid = pg_database.oid and datistemplate = 't';" +); +is($result, '0', 'subscription is not created on template databases'); + +# Verify logical replication works for all databases +# Insert rows on node P +$node_p->safe_psql('postgres', + "INSERT INTO tbl1 VALUES('row in database postgres')"); +$node_p->safe_psql($db1, "INSERT INTO tbl1 VALUES('fourth row')"); + +wait_for_all_subscriptions_caught_up($node_p, $node_u); + +# Check result in database 'postgres' of node U +$result = $node_u->safe_psql('postgres', 'SELECT * FROM tbl1'); +is( $result, + qq(row in database postgres), + "logical replication works in database postgres"); + +# Check result in database $db1 of node U +$result = $node_u->safe_psql($db1, 'SELECT * FROM tbl1'); +is( $result, qq(first row +second row +third row +fourth row), + "logical replication works in database $db1"); + # clean up $node_p->teardown_node; $node_s->teardown_node; $node_t->teardown_node; +$node_u->teardown_node; $node_f->teardown_node; done_testing();