On Mon, Jul 22, 2024 at 09:46:29AM -0500, Nathan Bossart wrote: > On Mon, Jul 22, 2024 at 03:45:19PM +0530, Amit Kapila wrote: >> On Mon, Jul 22, 2024 at 7:35 AM Michael Paquier <mich...@paquier.xyz> wrote: >>> A comment in get_db_rel_and_slot_infos() becomes incorrect where >>> get_old_cluster_logical_slot_infos() is called; it is still referring >>> to the subscription count. > > I removed this comment since IMHO it doesn't add much.
WFM. >>> Actually, on the same grounds, couldn't we do the logical slot info >>> retrieval in get_old_cluster_logical_slot_infos() in a single pass as >>> well? pg_replication_slots reports some information about all the >>> slots, and the current code has a qual on current_database(). It >>> looks to me that this could be replaced by a single query, ordering >>> the slots by database names, assigning the slot infos in each >>> database's DbInfo at the end. >> >> Unlike subscriptions, logical slots are database-specific objects. We >> have some checks in the code like the one in CreateDecodingContext() >> for MyDatabaseId which may or may not create a problem for this case >> as we don't consume changes when checking >> LogicalReplicationSlotHasPendingWal via >> binary_upgrade_logical_slot_has_caught_up() but I think this needs >> more analysis than what Nathan has proposed. So, I suggest taking up >> this task for PG18 if we want to optimize this code path. > > I see what you mean. I am not sure to get the reason why get_old_cluster_logical_slot_infos() could not be optimized, TBH. LogicalReplicationSlotHasPendingWal() uses the fast forward mode where no changes are generated, hence there should be no need for a dependency to a connection to a specific database :) Combined to a hash table based on the database name and/or OID to know to which dbinfo to attach the information of a slot, then it should be possible to use one query, making the slot info gathering closer to O(N) rather than the current O(N^2). -- Michael
signature.asc
Description: PGP signature
