Re: [PATCH] Support automatic sequence replication

On Fri, Mar 6, 2026 at 10:55 AM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
> On Thu, Mar 5, 2026 at 5:04 PM Zhijie Hou (Fujitsu)
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> >
> > Thanks for catching this, I changed it to check the increment before reporting warning.
> >
>
> Thanks! I have changed comments atop sequencesync.c to get rid of old
> implementation details (worker dependency upon INIT state) and
> rephrased a little bit. Please take it if you find it okay. Attaching
> patch as txt file. It is a top-up for 0001.
>

No major concerns on 001, just a few trivial things. Do these only if
you feel okay about these.

1)
copy_sequence():

+ (void) GetSubscriptionRelState(MySubscription->oid,
+ RelationGetRelid(sequence_rel),
+ &local_page_lsn);

IIUC, we only need it to get local_page_lsn which is used at the end
of copy_sequence(). Shall we move fetching it towards the end. That
way if validate_seqsync_state() returns copy-not-allowed, then there
is no need to even do 'GetSubscriptionRelState'.

2)
validate_seqsync_state() and get_and_validate_seq_info() are a bit
confusing (at least to me). They have similar names but serve
different purposes. Would it make sense to rename the new function
validate_seqsync_state() to check_seq_privileges_and_drift()?

3)
I feel that the section below in the doc needs some change to briefly
mention the role of the SeqSync worker, at least at a high level, to
indicate that it is running to perform incremental synchronization
already. Thoughts?

---------
29.7.2. Refreshing Out-of-Sync Sequences
Subscriber sequence values can become out of sync as the publisher
advances them. To detect this, compare the
pg_subscription_rel.srsublsn on the subscriber with the page_lsn
obtained from the pg_get_sequence_data function for the sequence on
the publisher. Then run ALTER SUBSCRIPTION ... REFRESH SEQUENCES to
re-synchronize if necessary.
---------

thanks
Shveta