public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: Ajin Cherian <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: shveta malik <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: [PATCH] Support automatic sequence replication
Date: Fri, 27 Feb 2026 16:37:28 +0530
Message-ID: <CAA4eK1JTau3fV7br6xwAV+LXXwM65RuGCuM2J3PQpCONtL1KXA@mail.gmail.com> (raw)
In-Reply-To: <CAFPTHDYud1zr0VyizhyhEQXfHMgXVcHrPzE56WUKGCFNskQq2A@mail.gmail.com>
References: <CAFPTHDZXX9WQ_X1ZfEvS248T+pKuk6SmCnXcvgPM059N1xPUfA@mail.gmail.com>
	<CAJpy0uDLUEjHHME8om1vAf6qkXCeRR6cBvkpK8yWBAC=T0ZFLA@mail.gmail.com>
	<CAFPTHDZg1JrunGgOj332hr+gUuH_Jm7skqPpYSvd-QE3yEdRDQ@mail.gmail.com>
	<CAJpy0uBz7MCSUkvFJD9ij65vBahNmY+bfCgdGKRqXovYs+K_TA@mail.gmail.com>
	<CAJpy0uDsuNqjWd-TmGBxqSS1rnVCJ3B8=SYrtxQ=Vs8kb71QFA@mail.gmail.com>
	<CAJpy0uAMWg3KcXtVBS7B0rnchLNrCCVYBByJCzAp=u5LERgtfA@mail.gmail.com>
	<CAFPTHDZwEhxhDAeqcPi0GuYN6xBs8gFXHOMUnbg3u2Xigcz4Zg@mail.gmail.com>
	<CAE9k0PmTyCU1A9YEf+MRgfeZ8yK1bAYJu=o0bH8DNUTzXejQyQ@mail.gmail.com>
	<CAA4eK1L6czEzG4mLNZSyjYC5nX0FMSjjk3csKuxPD3Ph5-7Yvw@mail.gmail.com>
	<CAJpy0uAhGQJ=msVsn2GsqWXr+YESJK6x9NBvrUtKvtvp1OVuKQ@mail.gmail.com>
	<CAJpy0uAOuu-M6wobH2wHOdTymm-cX9+MqwPyRNoOt=sPKBdCew@mail.gmail.com>
	<CAFPTHDZiWYXoKoo4VcBYNH9a=gxDZhfkcBeXt5w6cLw4_ysyKw@mail.gmail.com>
	<OS9PR01MB12149D9054CC7F2DC3F0D26A1F577A@OS9PR01MB12149.jpnprd01.prod.outlook.com>
	<CAA4eK1KYxQALt46k5uWOO6SUtNjvjOaXwfNjH0AU656YrcGZEw@mail.gmail.com>
	<CAFPTHDZYonM+SXG19VVjgWduXQJSuDhcOUWq0NCiiuQubCSt6g@mail.gmail.com>
	<CAFPTHDYud1zr0VyizhyhEQXfHMgXVcHrPzE56WUKGCFNskQq2A@mail.gmail.com>

On Thu, Feb 26, 2026 at 1:07 PM Ajin Cherian <[email protected]> wrote:
>

Few comments:
=============
1.
+ oldctx = MemoryContextSwitchTo(SequenceSyncContext);

- initStringInfo(&app_name);
- appendStringInfo(&app_name, "pg_%u_sequence_sync_" UINT64_FORMAT,
- MySubscription->oid, GetSystemIdentifier());
+ /* Process sequences */
+ sequence_copied = copy_sequences(conn, seqinfos);

- /*
- * Establish the connection to the publisher for sequence synchronization.
- */
- LogRepWorkerWalRcvConn =
- walrcv_connect(MySubscription->conninfo, true, true,
-    must_use_password,
-    app_name.data, &err);
- if (LogRepWorkerWalRcvConn == NULL)
- ereport(ERROR,
- errcode(ERRCODE_CONNECTION_FAILURE),
- errmsg("sequencesync worker for subscription \"%s\" could not
connect to the publisher: %s",
-    MySubscription->name, err));
-
- pfree(app_name.data);
-
- copy_sequences(LogRepWorkerWalRcvConn);
+ MemoryContextSwitchTo(oldctx);

It is better to switch to SequenceSyncContext at the caller of
LogicalRepSyncSequences similar to what we are doing for
ApplyMessageContext.

2.
@@ -4221,6 +4221,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
  ProcessConfigFile(PGC_SIGHUP);
  }

+
  if (rc & WL_TIMEOUT)

Spurious line addition.

3. Apart from above, the attached patch contains comments and cosmetic changes.

-- 
With Regards,
Amit Kapila.

diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c
index ffbbd1257d0..3898b315bb8 100644
--- a/src/backend/replication/logical/sequencesync.c
+++ b/src/backend/replication/logical/sequencesync.c
@@ -111,8 +111,8 @@ static MemoryContext SequenceSyncContext = NULL;
  * unsynchronized after it exits, a new worker can be started in the next
  * iteration.
  *
- * The pointer to the sequencesync worker is cached to avoid acquiring an
- * LWLock and scanning the workers array each time via logicalrep_worker_find().
+ * The pointer to the sequencesync worker is cached to avoid scanning the
+ * workers array each time via logicalrep_worker_find().
  */
 void
 MaybeLaunchSequenceSyncWorker(void)
@@ -137,7 +137,7 @@ MaybeLaunchSequenceSyncWorker(void)
 	LWLockAcquire(LogicalRepWorkerLock, LW_SHARED);
 
 	/*
-	 * Return if the sequence sync worker for the current subscription is
+	 * Quick exit if the sequence sync worker for the current subscription is
 	 * already alive.
 	 */
 	if (sequencesync_worker &&
@@ -586,9 +586,8 @@ copy_sequences(WalReceiverConn *conn, List *seqinfos)
 			{
 				case COPYSEQ_SUCCESS:
 					elog(DEBUG1,
-						 "logical replication synchronizatio for subscription \"%s\", sequence \"%s.%s\" has been updated",
-						 MySubscription->name, seqinfo->nspname,
-						 seqinfo->seqname);
+						 "logical replication synchronization has updated sequence \"%s.%s\" in subscription \"%s\"",
+						 seqinfo->nspname, seqinfo->seqname, MySubscription->name);
 					batch_succeeded_count++;
 					sequence_copied = true;
 					break;
@@ -765,14 +764,12 @@ LogicalRepSyncSequences(WalReceiverConn *conn)
 		 * allocate them under SequenceSyncContext.
 		 */
 		oldctx = MemoryContextSwitchTo(SequenceSyncContext);
-
 		seq = palloc0_object(LogicalRepSequenceInfo);
 		seq->localrelid = subrel->srrelid;
 		seq->nspname = get_namespace_name(RelationGetNamespace(sequence_rel));
 		seq->seqname = pstrdup(RelationGetRelationName(sequence_rel));
 		seq->relstate = relstate;
 		seqinfos = lappend(seqinfos, seq);
-
 		MemoryContextSwitchTo(oldctx);
 
 		table_close(sequence_rel, NoLock);
@@ -860,7 +857,6 @@ start_sequence_sync(void)
 
 			/* Process any invalidation messages that might have accumulated */
 			AcceptInvalidationMessages();
-
 			maybe_reread_subscription();
 
 			/*
@@ -875,7 +871,6 @@ start_sequence_sync(void)
 			}
 			else
 			{
-
 				/*
 				 * Double the sleep time, but not beyond
 				 * the maximum allowable value.


Attachments:

  [text/plain] v7_amit_1.patch.txt (2.5K, 2-v7_amit_1.patch.txt)
  download | inline diff:
diff --git a/src/backend/replication/logical/sequencesync.c b/src/backend/replication/logical/sequencesync.c
index ffbbd1257d0..3898b315bb8 100644
--- a/src/backend/replication/logical/sequencesync.c
+++ b/src/backend/replication/logical/sequencesync.c
@@ -111,8 +111,8 @@ static MemoryContext SequenceSyncContext = NULL;
  * unsynchronized after it exits, a new worker can be started in the next
  * iteration.
  *
- * The pointer to the sequencesync worker is cached to avoid acquiring an
- * LWLock and scanning the workers array each time via logicalrep_worker_find().
+ * The pointer to the sequencesync worker is cached to avoid scanning the
+ * workers array each time via logicalrep_worker_find().
  */
 void
 MaybeLaunchSequenceSyncWorker(void)
@@ -137,7 +137,7 @@ MaybeLaunchSequenceSyncWorker(void)
 	LWLockAcquire(LogicalRepWorkerLock, LW_SHARED);
 
 	/*
-	 * Return if the sequence sync worker for the current subscription is
+	 * Quick exit if the sequence sync worker for the current subscription is
 	 * already alive.
 	 */
 	if (sequencesync_worker &&
@@ -586,9 +586,8 @@ copy_sequences(WalReceiverConn *conn, List *seqinfos)
 			{
 				case COPYSEQ_SUCCESS:
 					elog(DEBUG1,
-						 "logical replication synchronizatio for subscription \"%s\", sequence \"%s.%s\" has been updated",
-						 MySubscription->name, seqinfo->nspname,
-						 seqinfo->seqname);
+						 "logical replication synchronization has updated sequence \"%s.%s\" in subscription \"%s\"",
+						 seqinfo->nspname, seqinfo->seqname, MySubscription->name);
 					batch_succeeded_count++;
 					sequence_copied = true;
 					break;
@@ -765,14 +764,12 @@ LogicalRepSyncSequences(WalReceiverConn *conn)
 		 * allocate them under SequenceSyncContext.
 		 */
 		oldctx = MemoryContextSwitchTo(SequenceSyncContext);
-
 		seq = palloc0_object(LogicalRepSequenceInfo);
 		seq->localrelid = subrel->srrelid;
 		seq->nspname = get_namespace_name(RelationGetNamespace(sequence_rel));
 		seq->seqname = pstrdup(RelationGetRelationName(sequence_rel));
 		seq->relstate = relstate;
 		seqinfos = lappend(seqinfos, seq);
-
 		MemoryContextSwitchTo(oldctx);
 
 		table_close(sequence_rel, NoLock);
@@ -860,7 +857,6 @@ start_sequence_sync(void)
 
 			/* Process any invalidation messages that might have accumulated */
 			AcceptInvalidationMessages();
-
 			maybe_reread_subscription();
 
 			/*
@@ -875,7 +871,6 @@ start_sequence_sync(void)
 			}
 			else
 			{
-
 				/*
 				 * Double the sleep time, but not beyond
 				 * the maximum allowable value.


view thread (58+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Support automatic sequence replication
  In-Reply-To: <CAA4eK1JTau3fV7br6xwAV+LXXwM65RuGCuM2J3PQpCONtL1KXA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox