Re: Improve pg_sync_replication_slots() to wait for primary to advance - Mailing list pgsql-hackers

On Fri, Nov 7, 2025 at 10:36 AM Japin Li <japinli@hotmail.com> wrote:
>
> >
> > Attaching patch v22 addressing the above comments.
>
> @@ -62,8 +62,8 @@ LOGICAL_APPLY_MAIN    "Waiting in main loop of logical replication apply process."
>  LOGICAL_LAUNCHER_MAIN  "Waiting in main loop of logical replication launcher process."
>  LOGICAL_PARALLEL_APPLY_MAIN    "Waiting in main loop of logical replication parallel apply process."
>  RECOVERY_WAL_STREAM    "Waiting in main loop of startup process for WAL to arrive, during streaming recovery."
> -REPLICATION_SLOTSYNC_MAIN      "Waiting in main loop of slot sync worker."
>  REPLICATION_SLOTSYNC_SHUTDOWN  "Waiting for slot sync worker to shut down."
> +REPLICATION_SLOTSYNC_MAIN      "Waiting in main loop of slot synchronization."
>  SYSLOGGER_MAIN "Waiting in main loop of syslogger process."
>  WAL_RECEIVER_MAIN      "Waiting in main loop of WAL receiver process."
>  WAL_SENDER_MAIN        "Waiting in main loop of WAL sender process."
>
> I've noticed that all events are sorted alphabetically.  I think we should keep
> the order of REPLICATION_SLOTSYNC_MAIN unchanged.
>

+1.

Few trivial comments:

1)
Since we have always used the term 'SQL function' rather than API in
existing code, shall we change all references of API to 'SQL function'
in current patch:

+ * If the pg_sync_replication API is used to sync the slots, and if the slots
"If the SQL function pg_sync_replication_slots() is used.."

+ * the reasons mentioned above, then the API also waits and retries until the
API --> SQL function

+ * persist. It is utilized by the pg_sync_replication_slots() API.
 pg_sync_replication_slots() API --> SQL function pg_sync_replication_slots()

+ * the API can retry.
API --> SQL function

+ /* Set this, so that API can retry */
API --> SQL function

+ * persist. It is utilized by the pg_sync_replication_slots() API.
pg_sync_replication_slots() API --> SQL function pg_sync_replication_slots()

+ * slot_persistence_pending - boolean used by pg_sync_replication_slots
+ *   API to track if any slots could not be
pg_sync_replication_slots  API --> SQL function pg_sync_replication_slots()

+ * Interrupt handler for pg_sync_replication_slots() API.
pg_sync_replication_slots() API --> SQL function pg_sync_replication_slots()


2)
ProcessSlotSyncAPIInterrupts
slotsync_api_reread_config
-- These also have API in it, but I do not have any better name
suggestions here, we can retain the current ones and see what others
say.

3)
/*
 * Re-read the config file.
 *
 * Exit if any of the slot sync GUCs have changed. The postmaster will
 * restart it.
 */
static void
slotsync_reread_config(void)

Shall we change this existing comment to: Re-read the config file for
slot sync worker.

4)

+/*
+ * Re-read the config file and check for critical parameter changes.
+ *
+ */
+static void
+slotsync_api_reread_config(void)

Shall we change comment to:
/*
 * Re-read the config file for SQL function pg_sync_replication_slots()
 *
 * Emit error if any of the slot sync GUCs have changed.
 */

thanks
Shveta