MIME-Version: 1.0
References: <CAFPTHDZXX9WQ_X1ZfEvS248T+pKuk6SmCnXcvgPM059N1xPUfA@mail.gmail.com>
 <CAJpy0uDLUEjHHME8om1vAf6qkXCeRR6cBvkpK8yWBAC=T0ZFLA@mail.gmail.com> <CAFPTHDZg1JrunGgOj332hr+gUuH_Jm7skqPpYSvd-QE3yEdRDQ@mail.gmail.com>
In-Reply-To: <CAFPTHDZg1JrunGgOj332hr+gUuH_Jm7skqPpYSvd-QE3yEdRDQ@mail.gmail.com>
From: shveta malik <shveta.malik@gmail.com>
Date: Fri, 6 Feb 2026 14:45:32 +0530
Message-ID: <CAJpy0uBz7MCSUkvFJD9ij65vBahNmY+bfCgdGKRqXovYs+K_TA@mail.gmail.com>
Subject: Re: [PATCH] Support automatic sequence replication
To: Ajin Cherian <itsajin@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, 
	shveta malik <shveta.malik@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAJpy0uBz7MCSUkvFJD9ij65vBahNmY%2BbfCgdGKRqXovYs%2BK_TA%40mail.gmail.com>
Precedence: bulk

On Thu, Feb 5, 2026 at 10:33=E2=80=AFAM Ajin Cherian <itsajin@gmail.com> wr=
ote:
>
> On Tue, Feb 3, 2026 at 9:22=E2=80=AFPM shveta malik <shveta.malik@gmail.c=
om> wrote:
> >
> > On Tue, Feb 3, 2026 at 9:18=E2=80=AFAM Ajin Cherian <itsajin@gmail.com>=
 wrote:
> > Thanks for the patch.
> >
> > +1 for the overall idea of patch that once a subscription is created
> > which subscribes to sequences, a sequence sync worker is started which
> > continuously syncs the sequences. This makes usage of REFRESH
> > SEQUENCES redundant and thus it is removed. I am still reviewing the
> > design choice here, and will post my comments soon (if any).
> >
>
> Thanks!
>
> > By quick validation, few issues in current implementation:
> >
> > 1)
> > If the sequence sync worker exits due to some issue (or killed or
> > server restarts), sequence-sync worker is not started again by apply
> > worker unless there is a sequence in INIT state i.e. synchronization
> > of sequences in READY state stops. IIUC, the logic of
> > ProcessSequencesForSync() needs to change to start seq sync worker
> > irrespective of state of sequences.
> >
>
> Yes, I fixed this. I've changed FetchRelationStates to fetch sequences
> in ANY state and not just ones in NON READY state.
>
> > 2)
> > There is some issue in how LOGs (DEBUGs) are getting generated.
> >
> > a) Even if there is no drift, it still keeps on dumping:
> > "logical replication sequence synchronization for subscription "sub1"
> > - total unsynchronized: 3"
> >
>
> Removed this.
>
> > b)
> > When there is a drift in say single sequence, it puts rest (which are
> > in sync) to "missing" section:
> > "logical replication sequence synchronization for subscription "sub1"
> > - batch #1 =3D 3 attempted, 1 succeeded, 0 mismatched, 0 insufficient
> > permission, 2 missing from publisher, 0 skipped"
> >
>
> Fixed, and added a new section called "no drift". Also now this debug
> message is printed every time the worker attempts to synchronize
> sequences. also mentioning the state of the sequences being synced.
>
> > 3)
> > If a sequence sync worker is taking a nap, and subscription is
> > disabled or the server is stopped just before upgrade, how is the user
> > supposed to know that sequences are synced at the end?
>
> Well, one way is to wait for a debug message that says that all the
> attempted sequences are in the "no drift" state. Also remote
> sequence's LSN is updated in pg_subscription_rel for each sequence.
> Let me know if you have anything more in mind. One option is to leave
> the ALTER SUBSCRIPTION..REFRESH SEQUENCE in place, that will change
> the state of all the sequences to the INIT state, and the user can
> then wait for the sequences to change state to READY.

I think this point needs more analysis. I am analyzing and discussing
it further.

> Attaching patch v3 addressing the above comments.
>

Thank You for the patch. I haven=E2=80=99t reviewed the details yet, but it
would be good to evaluate whether we really need to start the sequence
sync worker so deep inside the apply worker. Currently, the caller of
ProcessSequencesForSync(), namely ProcessSyncingRelations() is invoked
for every apply message to process possible state-changes of relation
and start worker (tablesync/sequence-sync etc). Since monitoring
state-change behavior is no longer required for sequences, should we
consider moving this logic to the main loop of the apply worker,
possibly within LogicalRepApplyLoop()? There, we could check whether
the table has any sequences and, if a worker is not already running,
start one. Thoughts?

thanks
Shveta