| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Ajin Cherian <itsajin(at)gmail(dot)com>, Yilin Zhang <jiezhilove(at)126(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Improve pg_sync_replication_slots() to wait for primary to advance |
| Date: | 2025-12-11 22:49:03 |
| Message-ID: | 3CB2CD6B-520D-4605-9633-2F304D47B65D@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Dec 11, 2025, at 20:23, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Thu, Dec 11, 2025 at 10:45 AM Ajin Cherian <itsajin(at)gmail(dot)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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jacob Champion | 2025-12-11 22:59:36 | Re: Few untranslated error messages in OAuth |
| Previous Message | Nathan Bossart | 2025-12-11 22:39:58 | Re: Proposal: Add a callback data parameter to GetNamedDSMSegment |