public inbox for [email protected]
help / color / mirror / Atom feedFrom: Hayato Kuroda (Fujitsu) <[email protected]>
To: Zhijie Hou (Fujitsu) <[email protected]>
To: Amit Kapila <[email protected]>
To: Ajin Cherian <[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: Wed, 4 Mar 2026 04:20:47 +0000
Message-ID: <OS9PR01MB12149FF8CA4CFDBB117E4447DF57CA@OS9PR01MB12149.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <TY4PR01MB1690715895CDE6FEFA13C2A2C947EA@TY4PR01MB16907.jpnprd01.prod.outlook.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>
Dear Hou,
Thanks for updating the patch. Here are my comments.
01.
```
<para>
A <firstterm>sequence synchronization worker</firstterm> will be started
- after executing any of the above subscriber commands, and will exit once the
- sequences are synchronized.
+ after executing any of the above subscriber commands. The worker will
+ remain running for the life of the subscription, periodically
+ synchronizing all published sequences.
</para>
```
I think it's not accurate, because REFRESH SEQUENCE command does not need the
sequencesync worker anymore.
02.
```
void
GetSequence(Relation seqrel, int64 *last_value, bool *is_called)
```
I think GetSequence() itself should conitan the permission check like SetSequence().
My idea is to set NULL for last_value and is_called in this case.
03.
```
/*
* Verify that the current user has SELECT privilege on the sequence.
* This is required to read the sequence state below.
*/
aclresult = pg_class_aclcheck(seqoid, GetUserId(), ACL_SELECT);
if (aclresult != ACLCHECK_OK)
return COPYSEQ_INSUFFICIENT_PERM;
/* Get current local sequence state */
GetSequence(sequence_rel, &local_last_value, &local_is_called);
```
If you accept above comment, this part can be simplified.
04.
```
/*
* get_and_validate_seq_info
*
* Extracts remote sequence information from the tuple slot received from the
* publisher, and validates it against the corresponding local sequence
* definition.
*/
static CopySeqResult
get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel,
LogicalRepSequenceInfo **seqinfo, int *seqidx,
List *seqinfos)
```
It can return COPYSEQ_SUCCESS, but it might be misleading; copying is not
happened yet here. How about returning boolean and add another argument to
indicate the reason if the validation is failed?
05.
LogicalRepSyncSequences() starts the transaction and read sequences every time.
Can we cache the seqinfos to reuse in the next iteration? My idea is to introduce
a syscache callback for the pg_subscription_relto invalidate the cached list.
How about measuring performance once and considering it's a good improvement?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
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], [email protected]
Subject: RE: [PATCH] Support automatic sequence replication
In-Reply-To: <OS9PR01MB12149FF8CA4CFDBB117E4447DF57CA@OS9PR01MB12149.jpnprd01.prod.outlook.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