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

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/

In response to

Browse pgsql-hackers by date

  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