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

> 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/