On Fri, Mar 6, 2026 at 10:55 AM shveta malik <[email protected]> wrote: > > On Thu, Mar 5, 2026 at 5:04 PM Zhijie Hou (Fujitsu) > <[email protected]> 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
