On Fri, Feb 6, 2026 at 2:45 PM shveta malik <[email protected]> wrote: > > On Thu, Feb 5, 2026 at 10:33 AM Ajin Cherian <[email protected]> wrote: > > > > On Tue, Feb 3, 2026 at 9:22 PM shveta malik <[email protected]> wrote: > > > > > > On Tue, Feb 3, 2026 at 9:18 AM Ajin Cherian <[email protected]> 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 = 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’t 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?
Correction to last line: There, we could check whether *the current susbcription* has any sequences and, if a worker is not already running,start one. thanks Shveta
