public inbox for [email protected]  
help / color / mirror / Atom feed
From: shveta malik <[email protected]>
To: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Zhijie Hou (Fujitsu) <[email protected]>
Cc: Ajin Cherian <[email protected]>
Cc: Ashutosh Sharma <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: shveta malik <[email protected]>
Subject: Re: [PATCH] Support automatic sequence replication
Date: Mon, 9 Mar 2026 15:59:15 +0530
Message-ID: <CAJpy0uCaF7G722rmD5mrFb8-N_bYF9A+vYpeEZCCsQUExPcUhQ@mail.gmail.com> (raw)
In-Reply-To: <OS9PR01MB12149E4614DA95963670772EEF579A@OS9PR01MB12149.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>
	<CAJpy0uA1txsV5RhjZjLBDrUjvxVyBDtMXzHr6=DzLHf7ybBrqg@mail.gmail.com>
	<TY4PR01MB1690739DE978BCBD12358478E947DA@TY4PR01MB16907.jpnprd01.prod.outlook.com>
	<CAJpy0uAfu-VPqCknLLvJ+PUx_cyoR-b70xUNT6Pyv8N-odKizQ@mail.gmail.com>
	<CAA4eK1K3S=FaeDS62qoidt=uUaOzRiyzq=DVV1uhV-jcva8KxA@mail.gmail.com>
	<TY4PR01MB169077FE33BDE8363F8795EC5947DA@TY4PR01MB16907.jpnprd01.prod.outlook.com>
	<OS9PR01MB12149E4614DA95963670772EEF579A@OS9PR01MB12149.jpnprd01.prod.outlook.com>

Few comments for 0002:


1)
+ /*
+ * Allow synchronization if the remote sequence data has a more recent
+ * LSN than the local state, or if no local LSN exists yet.
+ */
+ if (!XLogRecPtrIsValid(local_page_lsn) ||
+ local_page_lsn < seqinfo->page_lsn)
+ return COPYSEQ_ALLOWED;
  /*
  * Skip synchronization if the current user does not have sufficient
  * privileges to read the sequence data.
@@ -384,6 +418,40 @@ validate_seqsync_state(LogicalRepSequenceInfo
*seqinfo, Relation sequence_rel)
  if (!GetSequence(sequence_rel, &local_last_value, &local_is_called))
  return COPYSEQ_INSUFFICIENT_PERM;

Shouldn't the 'COPYSEQ_INSUFFICIENT_PERM' check be the first one, else
we may end up allowing the copy due to the first check even if the
user has no privileges? Or let me know if I am missing something here.

2)
+ ereport(WARNING,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("skipped synchronizing the sequence \"%s.%s\"",
+    seqinfo->nspname, seqinfo->seqname),
+ seqinfo->increment
+ ? errdetail("The local last_value %lld is ahead of the one on publisher",
+ (long long int) local_last_value)
+ : errdetail("The local last_value %lld is behind of the one on publisher",
+ (long long int) local_last_value));

This error message and DETAIL might not be very clear to the user.
IMO, it may not be easy for the user to deduce that a sequence is
descending and thus it being "behind" is the reason for skipping
synchronization.

Shall we update ERROR msg to say ascending/descending
ERROR: skipped synchronizing ascending sequence <name>
ERROR: skipped synchronizing descending sequence <name>

Since we use descending/ascending in sequence doc many times, I feel
it is okay to use it in ERROR messages too to give more clarity.
Thoughts?

3)
    A <firstterm>sequence synchronization worker</firstterm> will be started
-   after executing any of the above subscriber commands. The worker will
-   remain running for the life of the subscription, periodically
-   synchronizing all published sequences.

    A <firstterm>sequence synchronization worker</firstterm> will be started
+   after executing <command>ALTER SUBSCRIPTION ... REFRESH
PUBLICATION</command>
+   command. The worker will remain running for the life of the subscription,
+   periodically synchronizing all published sequences.

Why have we changed this to mention REFRESH PUB alone responsible for
starting worker? From user's point of view, even CREATE SUB (which has
sequences) will also start the worker no?

thanks
Shveta





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: <CAJpy0uCaF7G722rmD5mrFb8-N_bYF9A+vYpeEZCCsQUExPcUhQ@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