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

Hi, Ajin

Thanks for updating the patch.

On Mon, 27 Oct 2025 at 18:47, Ajin Cherian <itsajin@gmail.com> wrote:
> On Fri, Oct 24, 2025 at 8:29 PM shveta malik <shveta.malik@gmail.com> wrote:
>>
>> On Wed, Oct 22, 2025 at 10:25 AM Ajin Cherian <itsajin@gmail.com> wrote:
>> >
>> >
>> > I've modified the comments to reflect the new changes.
>> >
>> > attaching patch v18 with the above changes.
>> >
>>
>> Thanks for the patch. The test is still not clear. Can we please add
>> the test after the test of "Test logical failover slots corresponding
>> to different plugins" finishes instead of adding it in between?
>>
>
> I've rewritten the tests again to make this possible. Attaching v19
> which has the modified tap test.

Here are some comments on the new patch.

1. Given the existence of the foreach_ptr macro, we can switch the usage of
foreach to foreach_ptr.

diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index 1b78ffc5ff1..5db51407a82 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -872,7 +872,6 @@ fetch_remote_slots(WalReceiverConn *wrconn, List *slot_names)
 
 	if (slot_names != NIL)
 	{
-		ListCell   *lc;
 		bool		first_slot = true;
 
 		/*
@@ -880,10 +879,8 @@ fetch_remote_slots(WalReceiverConn *wrconn, List *slot_names)
 		 */
 		appendStringInfoString(&query, " AND slot_name IN (");
 
-		foreach(lc, slot_names)
+		foreach_ptr(char, slot_name, slot_names)
 		{
-			char *slot_name = (char *) lfirst(lc);
-
 			if (!first_slot)
 				appendStringInfoString(&query, ", ");
 
@@ -1872,15 +1869,13 @@ static List *
 extract_slot_names(List *remote_slots)
 {
 	List		*slot_names = NIL;
-	ListCell	*lc;
 	MemoryContext oldcontext;
 
 	/* Switch to long-lived TopMemoryContext to store slot names */
 	oldcontext = MemoryContextSwitchTo(TopMemoryContext);
 
-	foreach(lc, remote_slots)
+	foreach_ptr(RemoteSlot, remote_slot, remote_slots)
 	{
-		RemoteSlot *remote_slot = (RemoteSlot *) lfirst(lc);
 		char       *slot_name;
 
 		slot_name = pstrdup(remote_slot->name);

2. To append a signal character, switch from appendStringInfoString() to the
more efficient appendStringInfoChar().

+ 		appendStringInfoString(&query, ")");

3. The query memory can be released immediately after walrcv_exec() because
there are no subsequent references.

@@ -895,6 +892,7 @@ fetch_remote_slots(WalReceiverConn *wrconn, List *slot_names)
 
 	/* Execute the query */
 	res = walrcv_exec(wrconn, query.data, SLOTSYNC_COLUMN_COUNT, slotRow);
+	pfree(query.data);
 	if (res->status != WALRCV_OK_TUPLES)
 		ereport(ERROR,
 				errmsg("could not fetch failover logical slots info from the primary server: %s",
@@ -975,7 +973,6 @@ fetch_remote_slots(WalReceiverConn *wrconn, List *slot_names)
 	}
 
 	walrcv_clear_result(res);
-	pfree(query.data);
 
 	return remote_slot_list;
 }

> 
> Fujitsu Australia
>
> [2. text/x-diff; v19-0001-Improve-initial-slot-synchronization-in-pg_sync_.patch]...

-- 
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.