Horton

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

Chao Li <li.evan.chao@gmail.com>

View thread
From: Chao Li <li.evan.chao@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>, Yilin Zhang <jiezhilove@126.com>, shveta malik <shveta.malik@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>
Date: 2025-12-11T22:49:03Z
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 Dec 11, 2025, at 20:23, Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Thu, Dec 11, 2025 at 10:45 AM Ajin Cherian <itsajin@gmail.com> wrote:
>> 
>> As patch 0001 has been pushed. I've rebased and created a new version
>> v36 with the remaining patch.
>> 
> 
> I have made a number of changes in code comments and docs. Kindly
> review and if you are okay with these then include them in the next
> version.
> 

This diff enhanced docs and comments, overall looks good to me. A few nit comments:

1
```
- * Returns:
- *	List of remote slot information structures. Returns NIL if no slot
- *	is found.
- *
+ * Returns list of remote slot information structures, if any, otherwise,
+ * NIL if no slot is found.
```

I think “a” is needed before “list”, and “if any, otherwise,” looks rarely seen in code comments. So suggesting: 
```
 * Returns a list of remote slot information structures, or NIL if none
 * are found.
```

2
```
- * Parameters:
- * wrconn - Connection to the primary server
- * remote_slot_list - List of RemoteSlot structures to synchronize.
- * slot_persistence_pending - boolean used by SQL function
- * 							  pg_sync_replication_slots() to track if any slots
- * 							  could not be persisted and need to be retried.
+ * If slot_persistence_pending is not NULL, it will be set to true if one or
+ * more slots could not be persisted.
```

The changed version loses the meaning of retry. So, suggesting:
```
 * If slot_persistence_pending is not NULL, it will be set to true if one
 * or more slots could not be persisted.  This allows callers such as
 * pg_sync_replication_slots() to retry those slots.
```

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/