diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 6b320b1..bb6aa8e 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -726,7 +726,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, recordDependencyOnOwner(SubscriptionRelationId, subid, owner); /* - * XXX todo: If the subscription is for a sequence-only publication, + * XXX: If the subscription is for a sequence-only publication, * creating this origin is unnecessary at this point. It can be created * later during the ALTER SUBSCRIPTION ... REFRESH command, if the * publication is updated to include tables or tables in schemas. @@ -756,7 +756,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, PG_TRY(); { - bool hastables = false; + bool has_tables; List *relations; char table_state; @@ -771,16 +771,14 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, table_state = opts.copy_data ? SUBREL_STATE_INIT : SUBREL_STATE_READY; /* - * Get the table list from publisher and build local table status - * info. + * Build local relation status info. Relations are for both tables and + * sequences from the publisher. */ relations = fetch_table_list(wrconn, publications); - if (relations != NIL) - hastables = true; - - /* Include the sequence list from publisher. */ + has_tables = relations != NIL; relations = list_concat(relations, fetch_sequence_list(wrconn, publications)); + foreach_ptr(RangeVar, rv, relations) { Oid relid; @@ -800,7 +798,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, * won't use the initial snapshot for anything, so no need to * export it. * - * XXX todo: If the subscription is for a sequence-only publication, + * XXX: If the subscription is for a sequence-only publication, * creating this slot is not necessary at the moment. It can be * created during the ALTER SUBSCRIPTION ... REFRESH command if the * publication is updated to include tables or tables in schema. @@ -827,7 +825,7 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, * PENDING, to allow ALTER SUBSCRIPTION ... REFRESH * PUBLICATION to work. */ - if (opts.twophase && !opts.copy_data && hastables) + if (opts.twophase && !opts.copy_data && has_tables) twophase_enabled = true; walrcv_create_slot(wrconn, opts.slot_name, false, twophase_enabled, @@ -2475,7 +2473,7 @@ fetch_sequence_list(WalReceiverConn *wrconn, List *publications) " LATERAL pg_get_publication_sequences(p.pubname::text) gps(relid), pg_class c\n" " JOIN pg_namespace n ON n.oid = c.relnamespace\n" " JOIN pg_sequence s ON c.oid = s.seqrelid\n" - "WHERE c.oid = gps.relid AND p.pubname IN (\n"); + "WHERE c.oid = gps.relid AND p.pubname IN ("); get_publications_str(publications, &cmd, true); appendStringInfoChar(&cmd, ')'); diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c index 2935c53..a79c8a3 100644 --- a/src/backend/replication/logical/sequencesync.c +++ b/src/backend/replication/logical/sequencesync.c @@ -18,8 +18,8 @@ * ALTER SUBSCRIPTION ... REFRESH PUBLICATION * ALTER SUBSCRIPTION ... REFRESH PUBLICATION SEQUENCES * - * Apply worker will periodically check if there are any sequences in INIT - * state and start a sequencesync worker. + * The apply worker will periodically check if there are any sequences in INIT + * state and will start a sequencesync worker if needed. * * The sequencesync worker retrieves the sequences to be synchronized from the * pg_subscription_rel catalog table. It synchronizes multiple sequences per @@ -35,16 +35,18 @@ * (100) sequences are synchronized per transaction. The locks on the sequence * relation will be periodically released at each transaction commit. * - * An alternative design was considered where the launcher process would + * XXX: An alternative design was considered where the launcher process would * periodically check for sequences that need syncing and then start the - * sequence sync worker. However, the approach of having the apply worker - * manage the sequence sync worker was chosen for the following reasons: a) It - * avoids overloading the launcher, which handles various other subscription - * requests. b) It offers a more straightforward path for extending support for - * incremental sequence synchronization. c) It utilizes the existing tablesync - * worker code to start the sequencesync process, thus preventing code - * duplication in the launcher. d) It simplifies code maintenance by - * consolidating changes to a single location rather than multiple components. + * sequencesync worker. However, the approach of having the apply worker + * manage the sequencesync worker was chosen for the following reasons: + * a) It avoids overloading the launcher, which handles various other + * subscription requests. + * b) It offers a more straightforward path for extending support for + * incremental sequence synchronization. + * c) It utilizes the existing tablesync worker code to start the sequencesync + * process, thus preventing code duplication in the launcher. + * d) It simplifies code maintenance by consolidating changes to a single + * location rather than multiple components. *------------------------------------------------------------------------- */ diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c index 5800f21..011c579 100644 --- a/src/backend/replication/logical/tablesync.c +++ b/src/backend/replication/logical/tablesync.c @@ -1700,7 +1700,7 @@ copy_table_done: static bool FetchTableStates(void) { - static bool has_subrels = false; + static bool has_subtables = false; bool started_tx = false; if (relation_states_validity != SYNC_TABLE_STATE_VALID) @@ -1752,7 +1752,7 @@ FetchTableStates(void) * if table_states_not_ready was empty we still need to check again to * see if there are 0 tables. */ - has_subrels = (table_states_not_ready != NIL) || + has_subtables = (table_states_not_ready != NIL) || HasSubscriptionTables(MySubscription->oid); /* @@ -1772,7 +1772,7 @@ FetchTableStates(void) pgstat_report_stat(true); } - return has_subrels; + return has_subtables; } /*