Horton

Re: Improve pg_sync_replication_slots() to wait for primary to advance

shveta malik <shveta.malik@gmail.com>

View thread
From: shveta malik <shveta.malik@gmail.com>
To: Ajin Cherian <itsajin@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>, Japin Li <japinli@hotmail.com>, Ashutosh Sharma <ashu.coek88@gmail.com>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>, shveta malik <shveta.malik@gmail.com>
Date: 2025-12-09T11:45:24Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Enhance slot synchronization API to respect promotion signal.

  2. Fix inconsistent elevel in pg_sync_replication_slots() retry logic.

  3. Refactor slot synchronization logic in slotsync.c.

  4. Fix intermittent BF failure in 040_standby_failover_slots_sync.

  5. Add retry logic to pg_sync_replication_slots().

  6. Fix LOCK_TIMEOUT handling in slotsync worker.

  7. Add slotsync skip statistics.

On Tue, Dec 9, 2025 at 4:04 PM Ajin Cherian <itsajin@gmail.com> wrote:
>
> Hi all,
>
> Since commit 04396ea [1] has been pushed, which included part of the
> changes this patch set was addressing, I have updated and rebased the
> patch set to incorporate those changes.
>
> The patch set now contains two patches:
>
> 0001 – Modify the pg_sync_replication_slots API to also handle
> promotion signals and stop synchronization, similar to the slot sync
> worker.
> 0002 – Improve pg_sync_replication_slots to wait for and persist slots
> until they are sync-ready.
>
> Please review the updated patch set (v31).
>

Thanks. Please find a few comments on 001:

1)
Commit message:
"That meant backends
that perform slot synchronization via the pg_sync_replication_slots()
SQL function were not signalled at all because their PIDs were not
recorded in the slot-sync context."

We should phrase it in the singular ('backend'), since multiple
backends cannot run simultaneously because concurrent sync is not
allowed.
Using the term 'their PIDs' gives an impression that there could be
multiple PIDs, which is not the case.

2)
  primary_slotname_changed = strcmp(old_primary_slotname, PrimarySlotName) != 0;
+
  pfree(old_primary_conninfo);

This change to put blank line is not needed.

3)

+ /* Check for sync_replication_slots change */
+ /* Check for parameter changes common to both API and worker */

IMO, these comments are not needed as it is self-explanatory. Even if
we plan to put these, both should be same, either both mentioning API
and worker or neither.

4)
- * receives a stop request from the startup process, or when there is an
+ * because receives a stop request from the startup process, or when
there is an

I think, this change is done by mistake.

5)
+ * Signal slotsync worker or backend process running
pg_sync_replication_slots()
+ * if running. The process will stop upon detecting that the stopSignaled
+ * flag is set to true.

Comment looks slightly odd. Shall we say:

Signal the slotsync worker or the backend process running
pg_sync_replication_slots(), if either one is active. The process...

thanks
Shveta