Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: [PATCH] Support automatic sequence replication
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <TY4PR01MB169072D89B6A978E60388B87A9445A@TY4PR01MB16907.jpnprd01.prod.outlook.com>
Date: Fri, 13 Mar 2026 16:37:15 +0800
Cc: shveta malik <shveta.malik@gmail.com>,
 Amit Kapila <amit.kapila16@gmail.com>,
 Ajin Cherian <itsajin@gmail.com>,
 "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
 Ashutosh Sharma <ashu.coek88@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Content-Transfer-Encoding: quoted-printable
Message-Id: <DF86134B-C3F3-4BF3-91E2-D863CD553C0D@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>
 <CAA4eK1JTau3fV7br6xwAV+LXXwM65RuGCuM2J3PQpCONtL1KXA@mail.gmail.com>
 <OS9PR01MB1691377CDB1468CDC9820BBEB9470A@OS9PR01MB16913.jpnprd01.prod.outlook.com>
 <TY4PR01MB1690715895CDE6FEFA13C2A2C947EA@TY4PR01MB16907.jpnprd01.prod.outlook.com>
 <CAJpy0uA1txsV5RhjZjLBDrUjvxVyBDtMXzHr6=DzLHf7ybBrqg@mail.gmail.com>
 <TY4PR01MB1690739DE978BCBD12358478E947DA@TY4PR01MB16907.jpnprd01.prod.outlook.com>
 <CAJpy0uAfu-VPqCknLLvJ+PUx_cyoR-b70xUNT6Pyv8N-odKizQ@mail.gmail.com>
 <CAJpy0uBeAdz6-3P26Eryeq3TyjA-GiKY3z0hFMxzZD=AYGqQ3Q@mail.gmail.com>
 <TY4PR01MB169072DEE7CC20E9B06F0164C947DA@TY4PR01MB16907.jpnprd01.prod.outlook.com>
 <CAJpy0uC0T_tp62zxJN_2d_A=Ypvf14ebjGFepckeJugW5OHOyA@mail.gmail.com>
 <CAJpy0uAmEkjsBS6RxPv9iDcK2kfJ5=bq4Mq1zMCQtaYFoDfbbQ@mail.gmail.com>
 <TY4PR01MB169072D89B6A978E60388B87A9445A@TY4PR01MB16907.jpnprd01.prod.outlook.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
Archived-At: 
 <https://www.postgresql.org/message-id/DF86134B-C3F3-4BF3-91E2-D863CD553C0D%40gmail.com>
Precedence: bulk


> On Mar 13, 2026, at 15:13, Zhijie Hou (Fujitsu) =
<houzj.fnst@fujitsu.com> wrote:
>=20
> On Monday, March 9, 2026 11:13 AM shveta malik =
<shveta.malik@gmail.com> wrote:
>>=20
>> No major concerns on 001, just a few trivial things. Do these only if =
you feel
>> okay about these.
>>=20
>=20
> Thanks for the reviews. I've updated the patch set addressing all =
comments.
>=20
> In 0001, aside from addressing comments in [1][2][3][4], I've changed =
the
> logic to delay updating the page LSN for each sequence in
> pg_subscription_rel. Frequent catalog updates would generate many
> invalidations, degrading the performance of the apply worker which
> relies on cached data from pg_subscription_rel.
>=20
> 0002 adds caching of sequence information for the current subscription =
in the
> sequence sync worker. The cache is invalidated immediately when
> pg_subscription_rel is modified. Concurrent changes to sequence names =
or
> namespaces are detected before synchronization, as the worker verifies =
the
> sequence data at sync time.
>=20
> 0003 (formerly 0002) modifies REFRESH SEQUENCES to synchronize =
sequence values
> directly without launching a worker.
>=20
> [1] =
https://www.postgresql.org/message-id/02EDB3D2-4E5A-4EDE-BADF-3DF62D707831=
%40gmail.com
> [2] =
https://www.postgresql.org/message-id/OS9PR01MB12149E4614DA95963670772EEF5=
79A%40OS9PR01MB12149.jpnprd01.prod.outlook.com
> [3] =
https://www.postgresql.org/message-id/CAJpy0uAmEkjsBS6RxPv9iDcK2kfJ5%3Dbq4=
Mq1zMCQtaYFoDfbbQ%40mail.gmail.com
> [4] =
https://www.postgresql.org/message-id/CAJpy0uC0T_tp62zxJN_2d_A%3DYpvf14ebj=
GFepckeJugW5OHOyA%40mail.gmail.com
>=20
> Best Regards,
> Hou zj
> =
<v12-0003-Synchronize-sequences-directly-in-REFRESH-SEQUEN.patch><v12-0001=
-Support-automatic-sequence-replication.patch><v12-0002-Cache-sequence-inf=
ormation-in-the-sequence-sync-.patch>

I reviewed v12 again. 0001 looks good. A few comments on 0002 and 0003.

1 - 0002
```
+	/*
+	 * Setup callback for syscache so that we know when something =
changes in
+	 * the subscription relation state.
+	 */
+	CacheRegisterSyscacheCallback(SUBSCRIPTIONRELMAP,
+								  =
invalidate_syncing_sequence_infos,
+								  =
(Datum) 0);
```

I wonder if SUBSCRIPTIONRELMAP should be SUBSCRIPTIONREL?

2 - 0003
```
+		/*
+		 * Use the current memory context for synchronization. =
Since this should
+		 * be short-lived command context that will be cleaned =
up automatically,
+		 * we can simply assign it as the synchronization =
context.
+		 */
+		SequenceSyncContext =3D CurrentMemoryContext;
```

I think it=E2=80=99s still better to create a memory context from =
CurrentMemoryContext for SequenceSyncContext, and destroy it after =
copy_sequence.

Today, this is only on the SQL command path, CurrentMemoryContext is =
supposed to be short-lived. But AlterSubSyncSequences() might be called =
somewhere else in future, then we could not predict what would be =
CurrentMemoryContext.

3 - 0003
```
    "output the wanring for the missing sequence regress_s4=E2=80=9D);
```

Typo: wanring -> warning

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/