Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Don't include wait_event.h in pgstat.h

  2. Don't include proc.h in shm_mq.h

  3. Fix unsafe RTE_GROUP removal in simplify_EXISTS_query

  4. Add sequence synchronization for logical replication.

  1. [PATCH] Support automatic sequence replication

    Ajin Cherian <itsajin@gmail.com> — 2026-02-03T03:48:25Z

    Hello hackers,
    
    I'd like to propose an improvement to the sequence replication feature
    that was committed in [1].
    
    The current implementation synchronizes sequences during initial
    subscription setup, but the sequence sync worker exits after this
    initial sync. This means that as sequences advance on the publisher,
    they drift from the subscriber values over time. Users must manually
    run ALTER SUBSCRIPTION ... REFRESH SEQUENCES to resynchronize, which
    requires monitoring and intervention.
    
    Proposed Enhancement:
    
    This patch changes the sequence sync worker to run continuously
    throughout the subscription lifetime, automatically detecting and
    correcting sequence drift. The key changes are:
    
    1. The sequence sync worker remains running instead of exiting after
    initial sync, periodically checking for and synchronizing drifted
    sequences.
    
    2. The worker uses an exponential backoff strategy - starting at 2
    seconds, doubling up to a maximum of 30 seconds when sequences are in
    sync, and resetting to the minimum interval when drift is detected.
    
    3. Since synchronization is now automatic, ALTER SUBSCRIPTION ...
    REFRESH SEQUENCES is no longer needed and has been removed.
    
    The patch modifies documentation to reflect the new behavior, removes
    the REFRESH SEQUENCES command from the grammar and subscription
    commands, and implements the continuous monitoring loop in
    sequencesync.c. Tap tests have been updated to verify automatic
    synchronization rather than manual refresh.
    
    The attached v2 patch is attached and ready for review.
    
    Thoughts and feedback are welcome!
    
    [1] - https://github.com/postgres/postgres/commit/5509055d6956745532e65ab218e15b99d87d66ce
    
    Best regards,
    Ajin Cherian
    Fujitsu Australia
    
  2. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-03T10:21:48Z

    On Tue, Feb 3, 2026 at 9:18 AM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > Hello hackers,
    >
    > I'd like to propose an improvement to the sequence replication feature
    > that was committed in [1].
    >
    > The current implementation synchronizes sequences during initial
    > subscription setup, but the sequence sync worker exits after this
    > initial sync. This means that as sequences advance on the publisher,
    > they drift from the subscriber values over time. Users must manually
    > run ALTER SUBSCRIPTION ... REFRESH SEQUENCES to resynchronize, which
    > requires monitoring and intervention.
    >
    > Proposed Enhancement:
    >
    > This patch changes the sequence sync worker to run continuously
    > throughout the subscription lifetime, automatically detecting and
    > correcting sequence drift. The key changes are:
    >
    > 1. The sequence sync worker remains running instead of exiting after
    > initial sync, periodically checking for and synchronizing drifted
    > sequences.
    >
    > 2. The worker uses an exponential backoff strategy - starting at 2
    > seconds, doubling up to a maximum of 30 seconds when sequences are in
    > sync, and resetting to the minimum interval when drift is detected.
    >
    > 3. Since synchronization is now automatic, ALTER SUBSCRIPTION ...
    > REFRESH SEQUENCES is no longer needed and has been removed.
    >
    > The patch modifies documentation to reflect the new behavior, removes
    > the REFRESH SEQUENCES command from the grammar and subscription
    > commands, and implements the continuous monitoring loop in
    > sequencesync.c. Tap tests have been updated to verify automatic
    > synchronization rather than manual refresh.
    >
    > The attached v2 patch is attached and ready for review.
    >
    > Thoughts and feedback are welcome!
    >
    > [1] - https://github.com/postgres/postgres/commit/5509055d6956745532e65ab218e15b99d87d66ce
    >
    
    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).
    
    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.
    
    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"
    
    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"
    
    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?
    
    thanks
    Shveta
    
    
    
    
  3. Re: [PATCH] Support automatic sequence replication

    Ajin Cherian <itsajin@gmail.com> — 2026-02-05T05:03:13Z

    On Tue, Feb 3, 2026 at 9:22 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > On Tue, Feb 3, 2026 at 9:18 AM 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 = 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.
    
    Attaching patch v3 addressing the above comments.
    
    regards,
    Ajin Cherian
    Fujitsu Australia
    
  4. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-05T07:09:16Z

    We revisited the design of this patch. Sharing my thoughts and
    analysis here. Any feedback is appreciated.
    
    Background:
    -----------------------
    Previously, sequence synchronization was triggered during CREATE
    SUBSCRIPTION, ALTER SUBSCRIPTION REFRESH PUBLICATION, and  REFRESH
    SEQUENCES. A sequence-sync worker was started whenever a sequence
    entered the INIT state, which could also occur if a previous sync
    failed.
    
    Therefore, a mechanism was required to continuously scan
    pg_subscription_rel and start a sequence-sync worker for a
    subscription whenever any sequence was found in INIT. Since the apply
    worker already performs this role for table-sync workers, the same
    infrastructure was reused for sequence-sync workers. Using the
    launcher for this purpose was rejected, as it would have required
    overloading the launcher with logic to repeatedly inspect
    pg_subscription_rel and decide whether to start a worker for each
    sequence (see discussion at [1]).
    
    Current scenario:
    -----------------------
    The requirement is different now: the sequence-sync worker is now
    expected to run continuously, independent of sequence state. This
    makes us revisit our design choices and re-analyze whether we can do
    it in the launcher.
    
    The primary benefit of starting the sequence-sync worker from the
    launcher would be avoiding an extra apply worker for sequence-only
    subscriptions.  However, this approach introduces challenges. The
    launcher currently accesses only global pg_subscription and does not
    establish a database connection (see [2]). To decide whether to start
    an apply worker, a sequence-sync worker, or both, the launcher would
    need to access pg_subscription_rel, which requires a database
    connection. It is unclear which database the launcher should connect
    to, since subscriptions can target different databases. Another option
    would be to explicitly feed this information to the launcher during
    CREATE SUBSCRIPTION and REFRESH PUBLICATION by having an additional
    column in pg_subscription indicating object_type: table_only,
    seq_only, both.  This would undoubtedly add complexity. Also, I am
    unsure if it is a good idea to add an additional field to global
    catalog pg_subscription for this purpose.
    
    That said, it is reasonable to expect that users who create a
    publication for ALL SEQUENCES will typically have only a single
    publication–subscription pair. In such cases, the overhead of an extra
    apply worker per subscription, along with a sequence-sync worker, is
    likely acceptable.
    ~~
    
    Considering the above, starting the sequence-sync worker from the
    launcher seems feasible ((though it would require a more detailed
    analysis), but it comes with its own complexities. OTOH, (potential)
    significant extra worker overhead, which could impact the system,
    would only occur if a large number of 'sequence-only' subscriptions
    were created. It is unclear whether there is ever a need for multiple
    ALL-SEQUENCE subscriptions, or whether business requirements would
    need subscribing to multiple machines for ALL-SEQUENCES, which would
    necessitate multiple such subscriptions.
    
    Given this, it seems reasonable to continue with the current design of
    starting the sequence-sync worker from the apply worker. We may think
    of other approaches if there is any objection or user-feedback for
    this approach.
    
    ~~
    
    [1]: https://www.postgresql.org/message-id/CAA4eK1%2Bp%3DM%2B5NAq5VSxD4_XyE1MBTKwU40RD1cL9PgpbELKBRQ%40m…
    [2]:
       /*
             * Establish connection to nailed catalogs (we only ever access
             * pg_subscription).
             */
            BackgroundWorkerInitializeConnection(NULL, NULL, 0);
    
    thanks
    Shveta
    
    
    
    
  5. Re: [PATCH] Support automatic sequence replication

    Peter Smith <smithpb2250@gmail.com> — 2026-02-06T07:38:55Z

    Hi Ajin.
    
    Some review comments for patch v2-0001.
    
    ======
    .../replication/logical/sequencesync.c
    
    copy_sequences:
    
    1.
    + return drift_detected;
    
    This seems a bit strange. And it is not doing quite what the function
    comment says it does. I felt you should have another variable like
    'sequences_copied', which is set to true only when that
    'batch_succeeded_count++' is incremented. This is what you ultimately
    want to return. IMO, the variable 'drift_detected' isn't needed at
    all.
    
    ======
    src/test/subscription/t/036_sequences.pl
    
    2.
    ##########
    ## ALTER SUBSCRIPTION ... REFRESH PUBLICATION should cause sync of new
    # sequences of the publisher.
    ##########
    
    # Create a new sequence 'regress_s2', and update existing sequence 'regress_s1'
    $node_publisher->safe_psql(5.
    'postgres', qq(
    CREATE SEQUENCE regress_s2;
    INSERT INTO regress_seq_test SELECT nextval('regress_s2') FROM
    generate_series(1,100);
    
    -- Existing sequence
    INSERT INTO regress_seq_test SELECT nextval('regress_s1') FROM
    generate_series(1,100);
    ));
    
    ~
    
    IIUC, you are no longer sync of testing "existing sequences" in this
    test part, so you might also want to remove that comment and INSERT
    for 'regress_s1'.
    
    ======
    Kind Regards,
    Peter Smith.
    Fujitsu Australia
    
    
    
    
  6. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-06T09:15:32Z

    On Thu, Feb 5, 2026 at 10:33 AM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > On Tue, Feb 3, 2026 at 9:22 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > On Tue, Feb 3, 2026 at 9:18 AM 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 = 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?
    
    thanks
    Shveta
    
    
    
    
  7. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-06T09:17:26Z

    On Fri, Feb 6, 2026 at 2:45 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > On Thu, Feb 5, 2026 at 10:33 AM Ajin Cherian <itsajin@gmail.com> wrote:
    > >
    > > On Tue, Feb 3, 2026 at 9:22 PM shveta malik <shveta.malik@gmail.com> wrote:
    > > >
    > > > On Tue, Feb 3, 2026 at 9:18 AM 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 = 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
    
    
    
    
  8. Re: [PATCH] Support automatic sequence replication

    vignesh C <vignesh21@gmail.com> — 2026-02-06T12:37:13Z

    On Tue, 3 Feb 2026 at 15:52, shveta malik <shveta.malik@gmail.com> wrote:
    >
    > 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?
    
    One way to address the upgrade issue is to record the publisher LSN at
    the completion of the most recent SEQUENCE SYNC and persist it in the
    subscription metadata.
    During an upgrade, the following check should be performed: If the
    last recorded sequence-sync LSN is not ahead of the apply worker's
    LSN, report a clear error instructing the user to disable the
    subscription and explicitly run: ALTER SUBSCRIPTION … REFRESH
    SEQUENCES.
    Additionally, the existing ALTER SUBSCRIPTION … REFRESH SEQUENCES
    command should be enhanced so that it can be executed on disabled
    subscriptions and perform sequence synchronization independently,
    without relying on the sequence sync worker. Supporting execution on a
    disabled subscription is necessary because, if REFRESH SEQUENCES is
    run while the subscription is enabled, the apply worker may start
    immediately, ingest new transactions, and advance the replication
    slot's LSN beyond the point at which sequences were last synchronized
    again.
    
    Note: This approach may conservatively report that sequences need to
    be synchronized even when no sequence values have actually changed.
    This limitation is inherent to the design, as during an upgrade we
    don't connect to the publisher and decode WAL between the two LSNs to
    determine whether any sequence changes actually occurred.
    
    Regards,
    Vignesh
    
    
    
    
  9. Re: [PATCH] Support automatic sequence replication

    Peter Smith <smithpb2250@gmail.com> — 2026-02-08T23:54:37Z

    Some review comments for v3-0001.
    
    ======
    .../replication/logical/sequencesync.c
    
    copy_sequences:
    
    1.
    - if (sync_status == COPYSEQ_SUCCESS)
    +
    + /*
    + * For sequences in READY state, only sync if there's drift
    + */
    + if (relstate == SUBREL_STATE_READY && sync_status == COPYSEQ_SUCCESS)
    + {
    + should_sync = check_sequence_drift(seqinfo);
    + if (should_sync)
    + drift_detected = true;
    + }
    +
    + if (sync_status == COPYSEQ_SUCCESS && should_sync)
      sync_status = copy_sequence(seqinfo,
    - sequence_rel->rd_rel->relowner);
    + sequence_rel->rd_rel->relowner,
    + relstate);
    + else if (sync_status == COPYSEQ_SUCCESS && !should_sync)
    + sync_status = COPYSEQ_NOWORK;
    
    I'm struggling somewhat to follow this logic.
    
    For example, every condition refers to COPYSEQ_SUCCESS -- is that
    really necessary; can't this all be boiled down to something like
    below?
    
    SUGGESTION
    
    /*
     * For sequences in INIT state, always sync.
     * Otherwise, for sequences in READY state, only sync if there's drift.
     */
    if (sync_status == COPYSEQ_SUCCESS)
    {
      if ((relstate == SUBREL_STATE_INIT) || check_sequence_drift(seqinfo))
        sync_status = copy_sequence(...);
      else
        sync_status = COPYSEQ_NOWORK;
    }
    
    ======
    Kind Regards,
    Peter Smith.
    Fujitsu Australia
    
    
    
    
  10. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-11T03:30:02Z

    On Fri, Feb 6, 2026 at 2:47 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > On Fri, Feb 6, 2026 at 2:45 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > On Thu, Feb 5, 2026 at 10:33 AM Ajin Cherian <itsajin@gmail.com> wrote:
    > > >
    > > > On Tue, Feb 3, 2026 at 9:22 PM shveta malik <shveta.malik@gmail.com> wrote:
    > > > >
    > > > > On Tue, Feb 3, 2026 at 9:18 AM 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 = 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.
    >
    
    Few more comments:
    
    1)
    + /* Run sync for sequences in READY state */
    + sequence_copied |= LogicalRepSyncSequences(LogRepWorkerWalRcvConn,
    SUBREL_STATE_READY);
    +
    + /* Call initial sync for sequences in INIT state */
    + sequence_copied |= LogicalRepSyncSequences(LogRepWorkerWalRcvConn,
    SUBREL_STATE_INIT);
    
    Above logic means we ping primary twice for one seq-sync cycle? Is it
    possible to do it in below way:
    
    Get all sequences from the publisher in one go.
    If local-seq's state is INIT, copy those sequences, move the state to READY.
    If local-seq's state is READY, check the drift and proceed accordingly.
    
    i.e, there should be a single call to LogicalRepSyncSequences() and
    relstate shouldn't even be an argument.
    
    2)
    GetSequence:
    + /* Open and lock the sequence relation */
    + seqrel = table_open(relid, AccessShareLock);
    
    GetSequence is called from check_sequence_drift and
    check_sequence_drift() from copy_sequences(). In copy_sequences(), we
    already open the seq's (localrelid) table in
    get_and_validate_seq_info() and give it as output (see sequence_rel).
    Same can be passed to this instead of trying opening the table again.
    
    3)
    + /* Verify it's actually a sequence */
    + if (seqrel->rd_rel->relkind != RELKIND_SEQUENCE)
    + ereport(ERROR,
    + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
    + errmsg("\"%s\" is not a sequence",
    + RelationGetRelationName(seqrel))));
    
    Do we need this check? IIUC, sequence_rel is opened using
    RowExclusiveLock in get_and_validate_seq_info() and thus should not
    hit this case so that it becomes non-sequence later on. If required,
    we can have Assert.
    
    I can review further once these and previous comments are addressed.
    
    thanks
    Shveta
    
    
    
    
  11. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-11T03:51:29Z

    On Fri, Feb 6, 2026 at 6:07 PM vignesh C <vignesh21@gmail.com> wrote:
    >
    > On Tue, 3 Feb 2026 at 15:52, shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > 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?
    >
    > One way to address the upgrade issue is to record the publisher LSN at
    > the completion of the most recent SEQUENCE SYNC and persist it in the
    > subscription metadata.
    > During an upgrade, the following check should be performed: If the
    > last recorded sequence-sync LSN is not ahead of the apply worker's
    > LSN, report a clear error instructing the user to disable the
    > subscription and explicitly run: ALTER SUBSCRIPTION … REFRESH
    > SEQUENCES.
    
    I guess here we are trying to target the problem where a  table is
    dependent upon sequence and table data is replicated while seq's is
    not. And post upgrade, say if somehow the same sequence is used on the
    subscriber to insert data in the table, values can go back.
    
    > Additionally, the existing ALTER SUBSCRIPTION … REFRESH SEQUENCES
    > command should be enhanced so that it can be executed on disabled
    > subscriptions and perform sequence synchronization independently,
    > without relying on the sequence sync worker. Supporting execution on a
    > disabled subscription is necessary because, if REFRESH SEQUENCES is
    > run while the subscription is enabled, the apply worker may start
    > immediately, ingest new transactions, and advance the replication
    > slot's LSN beyond the point at which sequences were last synchronized
    > again.
    >
    > Note: This approach may conservatively report that sequences need to
    > be synchronized even when no sequence values have actually changed.
    > This limitation is inherent to the design, as during an upgrade we
    > don't connect to the publisher and decode WAL between the two LSNs to
    > determine whether any sequence changes actually occurred.
    >
    
    I like the idea. In particular, I like the approach of providing a
    REFRESH SEQUENCE command to the user. Even if we don’t implement the
    check during the upgrade process, we can clearly document that users
    should verify sequence drift before upgrading. If any sequence drift
    is detected, they should run REFRESH-SEQ manually.
    
    thanks
    Shveta
    
    
    
    
  12. Re: [PATCH] Support automatic sequence replication

    Ajin Cherian <itsajin@gmail.com> — 2026-02-12T09:23:59Z

    On Fri, Feb 6, 2026 at 8:15 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > On Thu, Feb 5, 2026 at 10:33 AM Ajin Cherian <itsajin@gmail.com> wrote:
    
    > 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?
    >
    
    I've moved this to LogicalRepApplyLoop()
    
    
    
    On Mon, Feb 9, 2026 at 10:55 AM Peter Smith <smithpb2250@gmail.com> wrote:
    >
    > Some review comments for v3-0001.
    >
    > ======
    > .../replication/logical/sequencesync.c
    >
    > copy_sequences:
    >
    > 1.
    > - if (sync_status == COPYSEQ_SUCCESS)
    > +
    > + /*
    > + * For sequences in READY state, only sync if there's drift
    > + */
    > + if (relstate == SUBREL_STATE_READY && sync_status == COPYSEQ_SUCCESS)
    > + {
    > + should_sync = check_sequence_drift(seqinfo);
    > + if (should_sync)
    > + drift_detected = true;
    > + }
    > +
    > + if (sync_status == COPYSEQ_SUCCESS && should_sync)
    >   sync_status = copy_sequence(seqinfo,
    > - sequence_rel->rd_rel->relowner);
    > + sequence_rel->rd_rel->relowner,
    > + relstate);
    > + else if (sync_status == COPYSEQ_SUCCESS && !should_sync)
    > + sync_status = COPYSEQ_NOWORK;
    >
    > I'm struggling somewhat to follow this logic.
    >
    > For example, every condition refers to COPYSEQ_SUCCESS -- is that
    > really necessary; can't this all be boiled down to something like
    > below?
    >
    > SUGGESTION
    >
    > /*
    >  * For sequences in INIT state, always sync.
    >  * Otherwise, for sequences in READY state, only sync if there's drift.
    >  */
    > if (sync_status == COPYSEQ_SUCCESS)
    > {
    >   if ((relstate == SUBREL_STATE_INIT) || check_sequence_drift(seqinfo))
    >     sync_status = copy_sequence(...);
    >   else
    >     sync_status = COPYSEQ_NOWORK;
    > }
    
    Changed as suggested.
    
    
    
    On Wed, Feb 11, 2026 at 2:30 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > On Fri, Feb 6, 2026 at 2:47 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > Few more comments:
    >
    > 1)
    > + /* Run sync for sequences in READY state */
    > + sequence_copied |= LogicalRepSyncSequences(LogRepWorkerWalRcvConn,
    > SUBREL_STATE_READY);
    > +
    > + /* Call initial sync for sequences in INIT state */
    > + sequence_copied |= LogicalRepSyncSequences(LogRepWorkerWalRcvConn,
    > SUBREL_STATE_INIT);
    >
    > Above logic means we ping primary twice for one seq-sync cycle? Is it
    > possible to do it in below way:
    >
    > Get all sequences from the publisher in one go.
    > If local-seq's state is INIT, copy those sequences, move the state to READY.
    > If local-seq's state is READY, check the drift and proceed accordingly.
    >
    > i.e, there should be a single call to LogicalRepSyncSequences() and
    > relstate shouldn't even be an argument.
    >
    
    Changed as suggested.
    
    > 2)
    > GetSequence:
    > + /* Open and lock the sequence relation */
    > + seqrel = table_open(relid, AccessShareLock);
    >
    > GetSequence is called from check_sequence_drift and
    > check_sequence_drift() from copy_sequences(). In copy_sequences(), we
    > already open the seq's (localrelid) table in
    > get_and_validate_seq_info() and give it as output (see sequence_rel).
    > Same can be passed to this instead of trying opening the table again.
    >
    > 3)
    > + /* Verify it's actually a sequence */
    > + if (seqrel->rd_rel->relkind != RELKIND_SEQUENCE)
    > + ereport(ERROR,
    > + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
    > + errmsg("\"%s\" is not a sequence",
    > + RelationGetRelationName(seqrel))));
    >
    
    Changed these.
    
    Other than these, I've changed seqinfos from being a global static
    list to a list that gets passed around and destroyed in each
    iteration.
    I haven't addressed the upgrade issue raised by Vignesh and Shveta in
    this patch. I will address that in the next patch.
     Here's patch v4 addressing the above comments.
    
    regards,
    Ajin Cherian
    Fujitsu Australia
    
  13. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-12T10:02:56Z

    On Thu, Feb 12, 2026 at 2:54 PM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > On Fri, Feb 6, 2026 at 8:15 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > On Thu, Feb 5, 2026 at 10:33 AM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > > 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?
    > >
    >
    > I've moved this to LogicalRepApplyLoop()
    >
    >
    >
    > On Mon, Feb 9, 2026 at 10:55 AM Peter Smith <smithpb2250@gmail.com> wrote:
    > >
    > > Some review comments for v3-0001.
    > >
    > > ======
    > > .../replication/logical/sequencesync.c
    > >
    > > copy_sequences:
    > >
    > > 1.
    > > - if (sync_status == COPYSEQ_SUCCESS)
    > > +
    > > + /*
    > > + * For sequences in READY state, only sync if there's drift
    > > + */
    > > + if (relstate == SUBREL_STATE_READY && sync_status == COPYSEQ_SUCCESS)
    > > + {
    > > + should_sync = check_sequence_drift(seqinfo);
    > > + if (should_sync)
    > > + drift_detected = true;
    > > + }
    > > +
    > > + if (sync_status == COPYSEQ_SUCCESS && should_sync)
    > >   sync_status = copy_sequence(seqinfo,
    > > - sequence_rel->rd_rel->relowner);
    > > + sequence_rel->rd_rel->relowner,
    > > + relstate);
    > > + else if (sync_status == COPYSEQ_SUCCESS && !should_sync)
    > > + sync_status = COPYSEQ_NOWORK;
    > >
    > > I'm struggling somewhat to follow this logic.
    > >
    > > For example, every condition refers to COPYSEQ_SUCCESS -- is that
    > > really necessary; can't this all be boiled down to something like
    > > below?
    > >
    > > SUGGESTION
    > >
    > > /*
    > >  * For sequences in INIT state, always sync.
    > >  * Otherwise, for sequences in READY state, only sync if there's drift.
    > >  */
    > > if (sync_status == COPYSEQ_SUCCESS)
    > > {
    > >   if ((relstate == SUBREL_STATE_INIT) || check_sequence_drift(seqinfo))
    > >     sync_status = copy_sequence(...);
    > >   else
    > >     sync_status = COPYSEQ_NOWORK;
    > > }
    >
    > Changed as suggested.
    >
    >
    >
    > On Wed, Feb 11, 2026 at 2:30 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > On Fri, Feb 6, 2026 at 2:47 PM shveta malik <shveta.malik@gmail.com> wrote:
    > > >
    > > Few more comments:
    > >
    > > 1)
    > > + /* Run sync for sequences in READY state */
    > > + sequence_copied |= LogicalRepSyncSequences(LogRepWorkerWalRcvConn,
    > > SUBREL_STATE_READY);
    > > +
    > > + /* Call initial sync for sequences in INIT state */
    > > + sequence_copied |= LogicalRepSyncSequences(LogRepWorkerWalRcvConn,
    > > SUBREL_STATE_INIT);
    > >
    > > Above logic means we ping primary twice for one seq-sync cycle? Is it
    > > possible to do it in below way:
    > >
    > > Get all sequences from the publisher in one go.
    > > If local-seq's state is INIT, copy those sequences, move the state to READY.
    > > If local-seq's state is READY, check the drift and proceed accordingly.
    > >
    > > i.e, there should be a single call to LogicalRepSyncSequences() and
    > > relstate shouldn't even be an argument.
    > >
    >
    > Changed as suggested.
    >
    > > 2)
    > > GetSequence:
    > > + /* Open and lock the sequence relation */
    > > + seqrel = table_open(relid, AccessShareLock);
    > >
    > > GetSequence is called from check_sequence_drift and
    > > check_sequence_drift() from copy_sequences(). In copy_sequences(), we
    > > already open the seq's (localrelid) table in
    > > get_and_validate_seq_info() and give it as output (see sequence_rel).
    > > Same can be passed to this instead of trying opening the table again.
    > >
    > > 3)
    > > + /* Verify it's actually a sequence */
    > > + if (seqrel->rd_rel->relkind != RELKIND_SEQUENCE)
    > > + ereport(ERROR,
    > > + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
    > > + errmsg("\"%s\" is not a sequence",
    > > + RelationGetRelationName(seqrel))));
    > >
    >
    > Changed these.
    >
    > Other than these, I've changed seqinfos from being a global static
    > list to a list that gets passed around and destroyed in each
    > iteration.
    > I haven't addressed the upgrade issue raised by Vignesh and Shveta in
    > this patch. I will address that in the next patch.
    
    Thanks Ajin, I will review. I will suggest waiting for others'
    feedback before doing anything about the upgrade issue. Meanwhile, we
    can try to improve the current patch itself.
    
    thanks
    Shveta
    
    
    
    
  14. Re: [PATCH] Support automatic sequence replication

    Peter Smith <smithpb2250@gmail.com> — 2026-02-13T05:42:35Z

    Hi Ajin.
    
    Some review comments for patch v4-0001
    
    ======
    src/backend/commands/sequence.c
    
    GetSequence:
    
    1.
    +/*
    + * Read the current sequence values (last_value and is_called)
    + *
    + * This is a read-only operation that acquires AccessShareLock on the sequence.
    + * Used by logical replication sequence synchronization to detect drift.
    + */
    
    The comment seems stale. e.g. the function is not acquiring a lock
    anymore, contrary to what that comment says.
    
    
    ======
    .../replication/logical/sequencesync.c
    
    2.
    -static List *seqinfos = NIL;
    
    The removal of this global was not strictly part of this patch; it is
    more like a prerequisite to make everything tidier, so your new code
    does not go further down that track of side-affecting a global. From
    that POV, I thought this global removal should be implemented as a
    first/separate (0001) patch so that it might be quickly reviewed and
    committed independently of the new seq-sync logic.
    
    ~~~
    
    LogicalRepSyncSequences:
    
    3.
    + /* Error on unexpected states */
    + if (relstate != SUBREL_STATE_INIT && relstate != SUBREL_STATE_READY)
    + {
    + table_close(sequence_rel, NoLock);
    + ereport(ERROR,
    + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    + errmsg("unexpected relstate '%c' for sequence \"%s.%s\" in
    subscription \"%s\"",
    + relstate,
    + get_namespace_name(RelationGetNamespace(sequence_rel)),
    + RelationGetRelationName(sequence_rel),
    + MySubscription->name)));
    + }
    +
    
    How is this possible? Should it just be Assert?
    
    ~~~
    
    start_sequence_sync:
    
    4.
    + /*
    + * Synchronize all sequences (both READY and INIT states).
    + * The function will process INIT sequences first, then READY sequences.
    + */
    + sequence_copied = LogicalRepSyncSequences(LogRepWorkerWalRcvConn);
    
    Why is talking about the processing order relevant?
    
    
    
    ======
    src/backend/replication/logical/syncutils.c
    
    5.
    + /* Check if any new sequences need syncing */
    + ProcessSequencesForSync();
    +
    
    Maybe don't say "new" because IIUC it also handles older sequences
    where the values have drifted.
    
    ======
    src/test/subscription/t/036_sequences.pl
    
    6.
    -$result = $node_publisher->safe_psql(
    - 'postgres', qq(
    - SELECT last_value, is_called FROM regress_s1;
    -));
    -is($result, '200|t', 'Check sequence value in the publisher');
    -
    -# Check - existing sequence ('regress_s1') is not synced
    -$result = $node_subscriber->safe_psql(
    - 'postgres', qq(
    - SELECT last_value, is_called FROM regress_s1;
    -));
    -is($result, '100|t', 'REFRESH PUBLICATION will not sync existing sequence');
    -
    
    Since you are no longer testing "existing sequences" in this test
    part, should you also remove the earlier INSERT for 'regress_s1'?
    
    ======
    Kind Regards,
    Peter Smith.
    Fujitsu Australia.
    
    
    
    
  15. Re: [PATCH] Support automatic sequence replication

    Ashutosh Sharma <ashu.coek88@gmail.com> — 2026-02-13T06:08:55Z

    Hi,
    
    On Tue, Feb 3, 2026 at 9:18 AM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > Hello hackers,
    >
    > I'd like to propose an improvement to the sequence replication feature
    > that was committed in [1].
    >
    > The current implementation synchronizes sequences during initial
    > subscription setup, but the sequence sync worker exits after this
    > initial sync. This means that as sequences advance on the publisher,
    > they drift from the subscriber values over time. Users must manually
    > run ALTER SUBSCRIPTION ... REFRESH SEQUENCES to resynchronize, which
    > requires monitoring and intervention.
    >
    > Proposed Enhancement:
    >
    > This patch changes the sequence sync worker to run continuously
    > throughout the subscription lifetime, automatically detecting and
    > correcting sequence drift. The key changes are:
    >
    > 1. The sequence sync worker remains running instead of exiting after
    > initial sync, periodically checking for and synchronizing drifted
    > sequences.
    >
    > 2. The worker uses an exponential backoff strategy - starting at 2
    > seconds, doubling up to a maximum of 30 seconds when sequences are in
    > sync, and resetting to the minimum interval when drift is detected.
    >
    > 3. Since synchronization is now automatic, ALTER SUBSCRIPTION ...
    > REFRESH SEQUENCES is no longer needed and has been removed.
    >
    > The patch modifies documentation to reflect the new behavior, removes
    > the REFRESH SEQUENCES command from the grammar and subscription
    > commands, and implements the continuous monitoring loop in
    > sequencesync.c. Tap tests have been updated to verify automatic
    > synchronization rather than manual refresh.
    >
    > The attached v2 patch is attached and ready for review.
    >
    > Thoughts and feedback are welcome!
    >
    > [1] -
    https://github.com/postgres/postgres/commit/5509055d6956745532e65ab218e15b99d87d66ce
    >
    
    Is this expected behavior?
    
    1) *Publisher:*
    
    
    
    
    
    *create sequence t1_seq;create table t1 (id int default nextval('t1_seq')
    primary key, a int);create publication t1_pub for table t1;create
    publication t1_seq_pub for all sequences;*
    
    2) *Subscriber:*
    
    
    
    
    
    
    
    
    *create sequence t1_seq;create table t1 (id int default nextval('t1_seq')
    primary key, a int);create subscription t1_sub connection 'host=127.0.0.1
    port=37500 dbname=test user=$USER' publication t1_pub with (create_slot =
    false, slot_name = 't1_sub');create subscription t1_seq_sub connection
    'host=127.0.0.1 port=37500 dbname=test user=$USER' publication t1_seq_pub
    with (create_slot = false, slot_name = 't1_seq_sub');select * from
    pg_subscription_rel;select * from pg_sequences;*
    
    3) *Publisher:*
    
    
    *insert into t1(a) values(10);select * from pg_sequences;*
    
    4) *Subscriber:*
    
    *select * from pg_sequences;* -- in sync with publisher.
    *insert into t1(a) values(20);*
    *select * from pg_sequences;* -- the sequence gets deviated from publisher.
    
    After a few minutes, re-running the above shows that the sequence value is
    reset to match the publisher. However, any new insert on the subscriber
    fails:
    
    *insert into t1(a) values(30);*
    
    
    
    
    *ERROR:  23505: duplicate key value violates unique constraint
    "t1_pkey"DETAIL:  Key (id)=(2) already exists.SCHEMA NAME:  publicTABLE
    NAME:  t1CONSTRAINT NAME:  t1_pkey*
    
    --
    
    Automatic sequence replication resets the last_value on the subscriber to
    match the publisher, which leads to duplicate key conflicts and prevents
    further inserts on the subscriber.
    
    --
    With Regards,
    Ashutosh Sharma.
    
  16. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-13T11:18:31Z

    On Fri, Feb 13, 2026 at 11:39 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
    >
    > Is this expected behavior?
    >
    > 1) Publisher:
    >
    > create sequence t1_seq;
    > create table t1 (id int default nextval('t1_seq') primary key, a int);
    >
    > create publication t1_pub for table t1;
    > create publication t1_seq_pub for all sequences;
    >
    > 2) Subscriber:
    >
    > create sequence t1_seq;
    > create table t1 (id int default nextval('t1_seq') primary key, a int);
    >
    > create subscription t1_sub connection 'host=127.0.0.1 port=37500 dbname=test user=$USER' publication t1_pub with (create_slot = false, slot_name = 't1_sub');
    > create subscription t1_seq_sub connection 'host=127.0.0.1 port=37500 dbname=test user=$USER' publication t1_seq_pub with (create_slot = false, slot_name = 't1_seq_sub');
    >
    > select * from pg_subscription_rel;
    > select * from pg_sequences;
    >
    > 3) Publisher:
    >
    > insert into t1(a) values(10);
    > select * from pg_sequences;
    >
    > 4) Subscriber:
    >
    > select * from pg_sequences; -- in sync with publisher.
    > insert into t1(a) values(20);
    > select * from pg_sequences; -- the sequence gets deviated from publisher.
    >
    > After a few minutes, re-running the above shows that the sequence value is reset to match the publisher. However, any new insert on the subscriber fails:
    >
    > insert into t1(a) values(30);
    > ERROR:  23505: duplicate key value violates unique constraint "t1_pkey"
    > DETAIL:  Key (id)=(2) already exists.
    > SCHEMA NAME:  public
    > TABLE NAME:  t1
    > CONSTRAINT NAME:  t1_pkey
    >
    > --
    >
    > Automatic sequence replication resets the last_value on the subscriber to match the publisher, which leads to duplicate key conflicts and prevents further inserts on the subscriber.
    >
    
    This is possible even without automatic sequence replication, say when
    the user uses REFRESH SEQUENCES command just before values(30). This
    is because sequence replication is mainly provided for upgrade
    purposes where sequences can be made up-to-date before upgrade. We
    should update this information in docs, if not already present.
    
    Having said that, we can possibly detect such synchronization as the
    sequence_update type of conflict and don't allow it to update on
    subscribers but that will be a separate patch.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  17. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-13T11:26:14Z

    On Fri, Feb 13, 2026 at 4:48 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Fri, Feb 13, 2026 at 11:39 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
    > >
    > > Is this expected behavior?
    > >
    > > 1) Publisher:
    > >
    > > create sequence t1_seq;
    > > create table t1 (id int default nextval('t1_seq') primary key, a int);
    > >
    > > create publication t1_pub for table t1;
    > > create publication t1_seq_pub for all sequences;
    > >
    > > 2) Subscriber:
    > >
    > > create sequence t1_seq;
    > > create table t1 (id int default nextval('t1_seq') primary key, a int);
    > >
    > > create subscription t1_sub connection 'host=127.0.0.1 port=37500 dbname=test user=$USER' publication t1_pub with (create_slot = false, slot_name = 't1_sub');
    > > create subscription t1_seq_sub connection 'host=127.0.0.1 port=37500 dbname=test user=$USER' publication t1_seq_pub with (create_slot = false, slot_name = 't1_seq_sub');
    > >
    > > select * from pg_subscription_rel;
    > > select * from pg_sequences;
    > >
    > > 3) Publisher:
    > >
    > > insert into t1(a) values(10);
    > > select * from pg_sequences;
    > >
    > > 4) Subscriber:
    > >
    > > select * from pg_sequences; -- in sync with publisher.
    > > insert into t1(a) values(20);
    > > select * from pg_sequences; -- the sequence gets deviated from publisher.
    > >
    > > After a few minutes, re-running the above shows that the sequence value is reset to match the publisher. However, any new insert on the subscriber fails:
    > >
    > > insert into t1(a) values(30);
    > > ERROR:  23505: duplicate key value violates unique constraint "t1_pkey"
    > > DETAIL:  Key (id)=(2) already exists.
    > > SCHEMA NAME:  public
    > > TABLE NAME:  t1
    > > CONSTRAINT NAME:  t1_pkey
    > >
    > > --
    > >
    > > Automatic sequence replication resets the last_value on the subscriber to match the publisher, which leads to duplicate key conflicts and prevents further inserts on the subscriber.
    > >
    >
    > This is possible even without automatic sequence replication, say when
    > the user uses REFRESH SEQUENCES command just before values(30). This
    > is because sequence replication is mainly provided for upgrade
    > purposes where sequences can be made up-to-date before upgrade. We
    > should update this information in docs, if not already present.
    >
    Yes. It is not present currently.
    
    > Having said that, we can possibly detect such synchronization as the
    > sequence_update type of conflict and don't allow it to update on
    > subscribers but that will be a separate patch.
    >
    
    I agree with this. It will not be in scope of the current patch.
    
    thanks
    Shveta
    
    
    
    
  18. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-13T11:40:23Z

    On Wed, Feb 11, 2026 at 9:21 AM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > > Additionally, the existing ALTER SUBSCRIPTION … REFRESH SEQUENCES
    > > command should be enhanced so that it can be executed on disabled
    > > subscriptions and perform sequence synchronization independently,
    > > without relying on the sequence sync worker. Supporting execution on a
    > > disabled subscription is necessary because, if REFRESH SEQUENCES is
    > > run while the subscription is enabled, the apply worker may start
    > > immediately, ingest new transactions, and advance the replication
    > > slot's LSN beyond the point at which sequences were last synchronized
    > > again.
    > >
    > > Note: This approach may conservatively report that sequences need to
    > > be synchronized even when no sequence values have actually changed.
    > > This limitation is inherent to the design, as during an upgrade we
    > > don't connect to the publisher and decode WAL between the two LSNs to
    > > determine whether any sequence changes actually occurred.
    > >
    >
    > I like the idea. In particular, I like the approach of providing a
    > REFRESH SEQUENCE command to the user. Even if we don’t implement the
    > check during the upgrade process, we can clearly document that users
    > should verify sequence drift before upgrading. If any sequence drift
    > is detected, they should run REFRESH-SEQ manually.
    >
    
    Yeah, implementing such checks are not required in upgrade but it is
    better to document the use of this command in upgrade context.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  19. Re: [PATCH] Support automatic sequence replication

    Ashutosh Sharma <ashu.coek88@gmail.com> — 2026-02-17T11:12:49Z

    Hi,
    
    On Fri, Feb 13, 2026 at 4:56 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > On Fri, Feb 13, 2026 at 4:48 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > >
    > > On Fri, Feb 13, 2026 at 11:39 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
    > > >
    > > > Is this expected behavior?
    > > >
    > > > 1) Publisher:
    > > >
    > > > create sequence t1_seq;
    > > > create table t1 (id int default nextval('t1_seq') primary key, a int);
    > > >
    > > > create publication t1_pub for table t1;
    > > > create publication t1_seq_pub for all sequences;
    > > >
    > > > 2) Subscriber:
    > > >
    > > > create sequence t1_seq;
    > > > create table t1 (id int default nextval('t1_seq') primary key, a int);
    > > >
    > > > create subscription t1_sub connection 'host=127.0.0.1 port=37500 dbname=test user=$USER' publication t1_pub with (create_slot = false, slot_name = 't1_sub');
    > > > create subscription t1_seq_sub connection 'host=127.0.0.1 port=37500 dbname=test user=$USER' publication t1_seq_pub with (create_slot = false, slot_name = 't1_seq_sub');
    > > >
    > > > select * from pg_subscription_rel;
    > > > select * from pg_sequences;
    > > >
    > > > 3) Publisher:
    > > >
    > > > insert into t1(a) values(10);
    > > > select * from pg_sequences;
    > > >
    > > > 4) Subscriber:
    > > >
    > > > select * from pg_sequences; -- in sync with publisher.
    > > > insert into t1(a) values(20);
    > > > select * from pg_sequences; -- the sequence gets deviated from publisher.
    > > >
    > > > After a few minutes, re-running the above shows that the sequence value is reset to match the publisher. However, any new insert on the subscriber fails:
    > > >
    > > > insert into t1(a) values(30);
    > > > ERROR:  23505: duplicate key value violates unique constraint "t1_pkey"
    > > > DETAIL:  Key (id)=(2) already exists.
    > > > SCHEMA NAME:  public
    > > > TABLE NAME:  t1
    > > > CONSTRAINT NAME:  t1_pkey
    > > >
    > > > --
    > > >
    > > > Automatic sequence replication resets the last_value on the subscriber to match the publisher, which leads to duplicate key conflicts and prevents further inserts on the subscriber.
    > > >
    > >
    > > This is possible even without automatic sequence replication, say when
    > > the user uses REFRESH SEQUENCES command just before values(30). This
    > > is because sequence replication is mainly provided for upgrade
    > > purposes where sequences can be made up-to-date before upgrade. We
    > > should update this information in docs, if not already present.
    > >
    > Yes. It is not present currently.
    >
    > > Having said that, we can possibly detect such synchronization as the
    > > sequence_update type of conflict and don't allow it to update on
    > > subscribers but that will be a separate patch.
    > >
    >
    > I agree with this. It will not be in scope of the current patch.
    
    Okay, agreed - it can be handled as a separate item.
    
    --
    With Regards,
    Ashutosh Sharma.
    
    
    
    
  20. Re: [PATCH] Support automatic sequence replication

    Ashutosh Sharma <ashu.coek88@gmail.com> — 2026-02-17T11:21:32Z

    Hi,
    
    On Thu, Feb 12, 2026 at 2:54 PM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > On Fri, Feb 6, 2026 at 8:15 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > On Thu, Feb 5, 2026 at 10:33 AM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > > 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?
    > >
    >
    > I've moved this to LogicalRepApplyLoop()
    >
    >
    >
    > On Mon, Feb 9, 2026 at 10:55 AM Peter Smith <smithpb2250@gmail.com> wrote:
    > >
    > > Some review comments for v3-0001.
    > >
    > > ======
    > > .../replication/logical/sequencesync.c
    > >
    > > copy_sequences:
    > >
    > > 1.
    > > - if (sync_status == COPYSEQ_SUCCESS)
    > > +
    > > + /*
    > > + * For sequences in READY state, only sync if there's drift
    > > + */
    > > + if (relstate == SUBREL_STATE_READY && sync_status == COPYSEQ_SUCCESS)
    > > + {
    > > + should_sync = check_sequence_drift(seqinfo);
    > > + if (should_sync)
    > > + drift_detected = true;
    > > + }
    > > +
    > > + if (sync_status == COPYSEQ_SUCCESS && should_sync)
    > >   sync_status = copy_sequence(seqinfo,
    > > - sequence_rel->rd_rel->relowner);
    > > + sequence_rel->rd_rel->relowner,
    > > + relstate);
    > > + else if (sync_status == COPYSEQ_SUCCESS && !should_sync)
    > > + sync_status = COPYSEQ_NOWORK;
    > >
    > > I'm struggling somewhat to follow this logic.
    > >
    > > For example, every condition refers to COPYSEQ_SUCCESS -- is that
    > > really necessary; can't this all be boiled down to something like
    > > below?
    > >
    > > SUGGESTION
    > >
    > > /*
    > >  * For sequences in INIT state, always sync.
    > >  * Otherwise, for sequences in READY state, only sync if there's drift.
    > >  */
    > > if (sync_status == COPYSEQ_SUCCESS)
    > > {
    > >   if ((relstate == SUBREL_STATE_INIT) || check_sequence_drift(seqinfo))
    > >     sync_status = copy_sequence(...);
    > >   else
    > >     sync_status = COPYSEQ_NOWORK;
    > > }
    >
    > Changed as suggested.
    >
    >
    >
    > On Wed, Feb 11, 2026 at 2:30 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > On Fri, Feb 6, 2026 at 2:47 PM shveta malik <shveta.malik@gmail.com> wrote:
    > > >
    > > Few more comments:
    > >
    > > 1)
    > > + /* Run sync for sequences in READY state */
    > > + sequence_copied |= LogicalRepSyncSequences(LogRepWorkerWalRcvConn,
    > > SUBREL_STATE_READY);
    > > +
    > > + /* Call initial sync for sequences in INIT state */
    > > + sequence_copied |= LogicalRepSyncSequences(LogRepWorkerWalRcvConn,
    > > SUBREL_STATE_INIT);
    > >
    > > Above logic means we ping primary twice for one seq-sync cycle? Is it
    > > possible to do it in below way:
    > >
    > > Get all sequences from the publisher in one go.
    > > If local-seq's state is INIT, copy those sequences, move the state to READY.
    > > If local-seq's state is READY, check the drift and proceed accordingly.
    > >
    > > i.e, there should be a single call to LogicalRepSyncSequences() and
    > > relstate shouldn't even be an argument.
    > >
    >
    > Changed as suggested.
    >
    > > 2)
    > > GetSequence:
    > > + /* Open and lock the sequence relation */
    > > + seqrel = table_open(relid, AccessShareLock);
    > >
    > > GetSequence is called from check_sequence_drift and
    > > check_sequence_drift() from copy_sequences(). In copy_sequences(), we
    > > already open the seq's (localrelid) table in
    > > get_and_validate_seq_info() and give it as output (see sequence_rel).
    > > Same can be passed to this instead of trying opening the table again.
    > >
    > > 3)
    > > + /* Verify it's actually a sequence */
    > > + if (seqrel->rd_rel->relkind != RELKIND_SEQUENCE)
    > > + ereport(ERROR,
    > > + (errcode(ERRCODE_WRONG_OBJECT_TYPE),
    > > + errmsg("\"%s\" is not a sequence",
    > > + RelationGetRelationName(seqrel))));
    > >
    >
    > Changed these.
    >
    > Other than these, I've changed seqinfos from being a global static
    > list to a list that gets passed around and destroyed in each
    > iteration.
    > I haven't addressed the upgrade issue raised by Vignesh and Shveta in
    > this patch. I will address that in the next patch.
    >  Here's patch v4 addressing the above comments.
    >
    
    How about retaining ALTER SUBSCRIPTION ... REFRESH SEQUENCES command?
    
    It can be useful in scenarios where automatic sequence replication
    cannot be enabled, for example, due to the max_worker_processes limit
    on the server preventing a new worker from being registered. Since
    increasing max_worker_processes requires a server restart, which the
    user may not want to perform immediately, this command would provide a
    way to manually synchronize the sequences.
    
    Thoughts?
    
    --
    
    One minor comment:
    
     * Sequence state transitions follow this pattern:
    - *   INIT -> READY
    + *  INIT --> READY ->-+
    + *             ^      | (check/synchronzize)
    + *             |      |
    + *             +--<---+
    
    
    "synchronzize" → "synchronize"
    
    --
    With Regards,
    Ashutosh Sharma.
    
    
    
    
  21. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-18T04:48:49Z

    On Tue, Feb 17, 2026 at 4:51 PM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
    >
    >
    > How about retaining ALTER SUBSCRIPTION ... REFRESH SEQUENCES command?
    >
    
    Yes, Shveta and myself also advocated the same for the upgrade case.
    
    > It can be useful in scenarios where automatic sequence replication
    > cannot be enabled, for example, due to the max_worker_processes limit
    > on the server preventing a new worker from being registered. Since
    > increasing max_worker_processes requires a server restart, which the
    > user may not want to perform immediately, this command would provide a
    > way to manually synchronize the sequences.
    >
    
    Good point. I think we won't be able to launch sync workers even when
    we reach max_logical_replication_workers limit. So, this is one more
    reason to retain the REFRESH SEQUENCES command.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  22. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-18T07:06:31Z

    I tested a few scenarios on the latest patch. Sequence sync worker did
    not stop in below scenarios:
    
    1) When the subscription was disabled.
    2) When the only publication for sequences was dropped from
    subscription ( ALTER SUBSCRIPTION sub1 DROP PUBLICATION pub_seq;)
    3) When all sequences were dropped on sub.
    
    Application worker stops in scenario 1, seq-sync worker should also
    stop. See maybe_reread_subscription().
    
    We need to decide the behavior of the seq-sync worker for 2 and 3.
    
    IMO, for scenario 2, we should stop the sequence sync worker. It is of
    no use to keep holding one worker slot when there are no publisher
    publishing sequences for that subscription.
    
    For scenario 3, it might be acceptable(or may be even expected?) to
    keep the seq-sync worker running. But the challenge is how we would
    distinguish scenario 2 from scenario 3. I believe scenario 2 will rely
    on the absence of sequence entries in pg_subscription_rel to sotp
    seq-sync worker. But in both scenario 2 and scenario 3, there would be
    no sequence entries in pg_subscription_rel. Given that, to keep the
    logic simpler, we can stop the seq-sync worker in scenario 3 as well.
    This seems like a corner case, and it should not cause much harm to
    stop the worker and restart it later when needed.
    
    Thoughts?
    
    thanks
    Shveta
    
    
    
    
  23. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-18T07:41:48Z

    On Wed, Feb 18, 2026 at 12:36 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > I tested a few scenarios on the latest patch. Sequence sync worker did
    > not stop in below scenarios:
    >
    > 1) When the subscription was disabled.
    > 2) When the only publication for sequences was dropped from
    > subscription ( ALTER SUBSCRIPTION sub1 DROP PUBLICATION pub_seq;)
    > 3) When all sequences were dropped on sub.
    >
    > Application worker stops in scenario 1, seq-sync worker should also
    > stop. See maybe_reread_subscription().
    >
    > We need to decide the behavior of the seq-sync worker for 2 and 3.
    >
    
    Shouldn't we try to map (2) and (3) to what we do for table publication?
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  24. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-18T10:04:46Z

    Few comments:
    
    1)
    + /* Check if any new sequences need syncing */
    + ProcessSequencesForSync();
    +
    
    Shall we name it 'maybe_start_seqsync_worker()' (or something better?)
    and move it immediately after 'maybe_advance_nonremovable_xid()'.
    Thoughts? This is because we do not process sequences here, we just
    check if the subscription includes sequences and start worker.
    
    2)
    We can also change the comment atop ProcessSequencesForSync to mention
    that. Currently it says:
    /*
     * Apply worker determines if sequence synchronization is needed.
     *
     * Start a sequencesync worker if one is not already running.
    
    Now, we shall change it to:
    Apply worker determines whether a sequence sync worker is needed.
    
    Check if the subscription includes sequences and start a sequencesync
    worker if one is not already running.
    
    3)
    I noticed that copy_sequences is invoked twice per sync cycle—once for
    sequences in the INIT state, and once for sequences in the READY state
    to detect drift. This means we ping the primary twice during each sync
    cycle. We should consider combining this logic into a single
    copy_sequences call. Since we already have the state information
    (INIT, READY, etc.) for each local sequence, copy_sequences can
    internally check the current state and decide whether to transition a
    sequence to READY based on its previous state. This approach would
    allow us to fetch all required information from the primary in a
    single call.
    
    I also think that we do not even need to mention the states here and
    we can give a single message instead of 2:
    DEBUG:  logical replication sequence synchronization for subscription
    "sub1" - for sequences in INIT state batch #1 = 5 attempted, 5
    succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    publisher, 0 skipped, 0 no drift
    DEBUG:  logical replication sequence synchronization for subscription
    "sub1" - for sequences in READY state batch #1 = 5 attempted, 0
    succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    publisher, 0 skipped, 5 no drift
    
    4)
    FetchRelationStates():
    + List    *seq_states;
    
    It would be better to initialize it with NIL.
    
    thanks
    Shveta
    
    
    
    
  25. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-18T11:28:20Z

    On Wed, Feb 18, 2026 at 1:12 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Wed, Feb 18, 2026 at 12:36 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > I tested a few scenarios on the latest patch. Sequence sync worker did
    > > not stop in below scenarios:
    > >
    > > 1) When the subscription was disabled.
    > > 2) When the only publication for sequences was dropped from
    > > subscription ( ALTER SUBSCRIPTION sub1 DROP PUBLICATION pub_seq;)
    > > 3) When all sequences were dropped on sub.
    > >
    > > Application worker stops in scenario 1, seq-sync worker should also
    > > stop. See maybe_reread_subscription().
    > >
    > > We need to decide the behavior of the seq-sync worker for 2 and 3.
    > >
    >
    > Shouldn't we try to map (2) and (3) to what we do for table publication?
    >
    
    I thought about it, this is what I have in mind:
    
    1) When there is no sequences and tables to be synced, we will be
    blocking 2 workers slot, one for apply worker and one for seq-sync
    worker. While only apply-worker is enough, as it may restart seq-sync
    worker as soon as it notices a sequence.
    
    2) In case of apply-worker, when no tables are being published, it
    hardly does any work. IIUC, it just sends responses to keep-alive
    messages. OTOH, seq-sync worker will begin a transaction and query
    pg_subscription_rel every few seconds. I feel, we should avoid this
    unnecessary activity if possible.
    
    Overall, I feel the sequence sync worker is logically different from
    the table apply worker. It behaves more like an auxiliary worker
    managed by the apply worker and derives all of its state from the
    system catalogs. In contrast, the apply worker actively consumes the
    logical replication stream and is responsible for advancing the slot
    and origin. If the apply worker is stopped for an extended period, the
    slot cannot advance, which may lead to WAL accumulation on the
    publisher. There is no such constraint associated with the seq-sync
    worker. And thus stopping the seq-sync worker aggressively (in
    scenarios 2 and 3) is both safer and simpler as compared to apply
    worker and will also avoid some unnecessary transactions and activity
    as I stated above. Thoughts?
    
    thanks
    Shveta
    
    
    
    
  26. Re: [PATCH] Support automatic sequence replication

    Peter Smith <smithpb2250@gmail.com> — 2026-02-18T21:56:29Z

    On Wed, Feb 18, 2026 at 9:05 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    ...
    > 3)
    > I noticed that copy_sequences is invoked twice per sync cycle—once for
    > sequences in the INIT state, and once for sequences in the READY state
    > to detect drift. This means we ping the primary twice during each sync
    > cycle. We should consider combining this logic into a single
    > copy_sequences call. Since we already have the state information
    > (INIT, READY, etc.) for each local sequence, copy_sequences can
    > internally check the current state and decide whether to transition a
    > sequence to READY based on its previous state. This approach would
    > allow us to fetch all required information from the primary in a
    > single call.
    >
    > I also think that we do not even need to mention the states here and
    > we can give a single message instead of 2:
    > DEBUG:  logical replication sequence synchronization for subscription
    > "sub1" - for sequences in INIT state batch #1 = 5 attempted, 5
    > succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    > publisher, 0 skipped, 0 no drift
    > DEBUG:  logical replication sequence synchronization for subscription
    > "sub1" - for sequences in READY state batch #1 = 5 attempted, 0
    > succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    > publisher, 0 skipped, 5 no drift
    >
    
    Those are DEBUG1 messages, not LOG messages, so I think the primary
    goal is to ensure that they are full of useful information to help
    debugging.  Knowing the prior state information means we know that the
    "drift" logic was needed when deciding to sync or not.
    
    If message volume can be reduced without any loss of debugging
    usefulness, then great, but we need to take care not to throw the baby
    out with the bath water.
    
    If message volume is a problem, then maybe that should be addressed by
    using more log levels DEBUG1,DEBUG2,DEBUG3,... etc.
    
    ======
    Kind Regards,
    Peter Smith.
    Fujitsu Australia
    
    
    
    
  27. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-19T02:15:36Z

    On Wed, Feb 18, 2026 at 4:58 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > On Wed, Feb 18, 2026 at 1:12 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > >
    > > On Wed, Feb 18, 2026 at 12:36 PM shveta malik <shveta.malik@gmail.com> wrote:
    > > >
    > > > I tested a few scenarios on the latest patch. Sequence sync worker did
    > > > not stop in below scenarios:
    > > >
    > > > 1) When the subscription was disabled.
    > > > 2) When the only publication for sequences was dropped from
    > > > subscription ( ALTER SUBSCRIPTION sub1 DROP PUBLICATION pub_seq;)
    > > > 3) When all sequences were dropped on sub.
    > > >
    > > > Application worker stops in scenario 1, seq-sync worker should also
    > > > stop. See maybe_reread_subscription().
    > > >
    > > > We need to decide the behavior of the seq-sync worker for 2 and 3.
    > > >
    > >
    > > Shouldn't we try to map (2) and (3) to what we do for table publication?
    > >
    >
    > I thought about it, this is what I have in mind:
    >
    > 1) When there is no sequences and tables to be synced, we will be
    > blocking 2 workers slot, one for apply worker and one for seq-sync
    > worker. While only apply-worker is enough, as it may restart seq-sync
    > worker as soon as it notices a sequence.
    >
    > 2) In case of apply-worker, when no tables are being published, it
    > hardly does any work. IIUC, it just sends responses to keep-alive
    > messages. OTOH, seq-sync worker will begin a transaction and query
    > pg_subscription_rel every few seconds. I feel, we should avoid this
    > unnecessary activity if possible.
    >
    > Overall, I feel the sequence sync worker is logically different from
    > the table apply worker. It behaves more like an auxiliary worker
    > managed by the apply worker and derives all of its state from the
    > system catalogs.
    >
    
    I understand that sequence-sync worker won't be doing anything useful
    in such corner cases but the chances of such situations are rare and
    the consequences are not that bad. Similarly, one can say that we can
    exit the launcher process when no subscription is present and the
    system should figure out and restart when required. In this case also,
    the apply-worker needs to check from time-to-time or needs to be
    informed to launch the new sequence-sync worker. I think we can
    evaluate these cases separately and can decide to write a top-up patch
    if found useful to make sequence-sync worker detect and exit.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  28. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-19T02:22:26Z

    On Thu, Feb 19, 2026 at 3:26 AM Peter Smith <smithpb2250@gmail.com> wrote:
    >
    > On Wed, Feb 18, 2026 at 9:05 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > ...
    > > 3)
    > > I noticed that copy_sequences is invoked twice per sync cycle—once for
    > > sequences in the INIT state, and once for sequences in the READY state
    > > to detect drift. This means we ping the primary twice during each sync
    > > cycle. We should consider combining this logic into a single
    > > copy_sequences call. Since we already have the state information
    > > (INIT, READY, etc.) for each local sequence, copy_sequences can
    > > internally check the current state and decide whether to transition a
    > > sequence to READY based on its previous state. This approach would
    > > allow us to fetch all required information from the primary in a
    > > single call.
    > >
    > > I also think that we do not even need to mention the states here and
    > > we can give a single message instead of 2:
    > > DEBUG:  logical replication sequence synchronization for subscription
    > > "sub1" - for sequences in INIT state batch #1 = 5 attempted, 5
    > > succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    > > publisher, 0 skipped, 0 no drift
    > > DEBUG:  logical replication sequence synchronization for subscription
    > > "sub1" - for sequences in READY state batch #1 = 5 attempted, 0
    > > succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    > > publisher, 0 skipped, 5 no drift
    > >
    >
    > Those are DEBUG1 messages, not LOG messages, so I think the primary
    > goal is to ensure that they are full of useful information to help
    > debugging.  Knowing the prior state information means we know that the
    > "drift" logic was needed when deciding to sync or not.
    >
    > If message volume can be reduced without any loss of debugging
    > usefulness, then great, but we need to take care not to throw the baby
    > out with the bath water.
    >
    
    I think it is useful to discuss how much DEBUG information is
    required. However, I would like to know if this is related to the
    patch being discussed or a case in HEAD? If later, then it is better
    to discuss it separately and address it as a separate patch if
    required.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  29. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-19T04:05:03Z

    On Thu, Feb 19, 2026 at 3:26 AM Peter Smith <smithpb2250@gmail.com> wrote:
    >
    > On Wed, Feb 18, 2026 at 9:05 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > ...
    > > 3)
    > > I noticed that copy_sequences is invoked twice per sync cycle—once for
    > > sequences in the INIT state, and once for sequences in the READY state
    > > to detect drift. This means we ping the primary twice during each sync
    > > cycle. We should consider combining this logic into a single
    > > copy_sequences call. Since we already have the state information
    > > (INIT, READY, etc.) for each local sequence, copy_sequences can
    > > internally check the current state and decide whether to transition a
    > > sequence to READY based on its previous state. This approach would
    > > allow us to fetch all required information from the primary in a
    > > single call.
    > >
    > > I also think that we do not even need to mention the states here and
    > > we can give a single message instead of 2:
    > > DEBUG:  logical replication sequence synchronization for subscription
    > > "sub1" - for sequences in INIT state batch #1 = 5 attempted, 5
    > > succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    > > publisher, 0 skipped, 0 no drift
    > > DEBUG:  logical replication sequence synchronization for subscription
    > > "sub1" - for sequences in READY state batch #1 = 5 attempted, 0
    > > succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    > > publisher, 0 skipped, 5 no drift
    > >
    >
    > Those are DEBUG1 messages, not LOG messages, so I think the primary
    > goal is to ensure that they are full of useful information to help
    > debugging.  Knowing the prior state information means we know that the
    > "drift" logic was needed when deciding to sync or not.
    >
    > If message volume can be reduced without any loss of debugging
    > usefulness, then great, but we need to take care not to throw the baby
    > out with the bath water.
    >
    > If message volume is a problem, then maybe that should be addressed by
    > using more log levels DEBUG1,DEBUG2,DEBUG3,... etc.
    >
    
    I am not totally against it, but I don’t find it particularly useful
    to dump the state (INIT, READY), since we already log exactly which
    sequences were updated individually (see below). The output below
    covers both cases—sequences with drift and those in the INIT state. We
    also include the “no drift” count. So all of these effectively
    summarizes the overall status.
    
    logical replication sync for subscription "sub2", sequence
    "public.myseq1" has been updated
    logical replication sync for subscription "sub2", sequence
    "public.myseq2" has been updated
    
     Even if we plan to retain multiple messages for INIT vs READY, I
    guess, with copy_sequences() now suggested to be called only once
    irrespective of 'state' will make it further tricky to separate out
    these log messages for INIT and READY. But let's see.
    
    > I think it is useful to discuss how much DEBUG information is
    > required. However, I would like to know if this is related to the
    > patch being discussed or a case in HEAD? If later, then it is better
    > to discuss it separately and address it as a separate patch if
    > required.
    
    Amit, this is related to the new patch only.
    
    thanks
    Shveta
    
    
    
    
  30. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-19T04:19:34Z

    On Thu, Feb 19, 2026 at 7:45 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Wed, Feb 18, 2026 at 4:58 PM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > On Wed, Feb 18, 2026 at 1:12 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > > >
    > > > On Wed, Feb 18, 2026 at 12:36 PM shveta malik <shveta.malik@gmail.com> wrote:
    > > > >
    > > > > I tested a few scenarios on the latest patch. Sequence sync worker did
    > > > > not stop in below scenarios:
    > > > >
    > > > > 1) When the subscription was disabled.
    > > > > 2) When the only publication for sequences was dropped from
    > > > > subscription ( ALTER SUBSCRIPTION sub1 DROP PUBLICATION pub_seq;)
    > > > > 3) When all sequences were dropped on sub.
    > > > >
    > > > > Application worker stops in scenario 1, seq-sync worker should also
    > > > > stop. See maybe_reread_subscription().
    > > > >
    > > > > We need to decide the behavior of the seq-sync worker for 2 and 3.
    > > > >
    > > >
    > > > Shouldn't we try to map (2) and (3) to what we do for table publication?
    > > >
    > >
    > > I thought about it, this is what I have in mind:
    > >
    > > 1) When there is no sequences and tables to be synced, we will be
    > > blocking 2 workers slot, one for apply worker and one for seq-sync
    > > worker. While only apply-worker is enough, as it may restart seq-sync
    > > worker as soon as it notices a sequence.
    > >
    > > 2) In case of apply-worker, when no tables are being published, it
    > > hardly does any work. IIUC, it just sends responses to keep-alive
    > > messages. OTOH, seq-sync worker will begin a transaction and query
    > > pg_subscription_rel every few seconds. I feel, we should avoid this
    > > unnecessary activity if possible.
    > >
    > > Overall, I feel the sequence sync worker is logically different from
    > > the table apply worker. It behaves more like an auxiliary worker
    > > managed by the apply worker and derives all of its state from the
    > > system catalogs.
    > >
    >
    > I understand that sequence-sync worker won't be doing anything useful
    > in such corner cases but the chances of such situations are rare and
    > the consequences are not that bad. Similarly, one can say that we can
    > exit the launcher process when no subscription is present and the
    > system should figure out and restart when required.
    
    No, I don’t think this is comparable to our case. For that scenario to
    work, we would need to expose subscription-related awareness and state
    to the postmaster, which is not recommended. In our case, the apply
    worker is already responsible for monitoring the sequence-sync worker.
    It is already checking two things:
    --whether the subscription includes sequences, and
    --whether the sequence-sync worker is running and if not, it starts it.
    
    So there is no additional logic required. None. We don’t need any
    extra awareness in the apply worker. Given that, I don’t think the two
    cases are comparable at all.
    
    But I also agree that it is a corner case, and running additional
    seq-sync worker per subscription may not harm that much.
    
    > In this case also,
    > the apply-worker needs to check from time-to-time or needs to be
    > informed to launch the new sequence-sync worker.
    
    It is already doing that. There is always a case where seq-sync worker
    errors out or exits unexpectedly. In such a case, the apply worker is
    the one to start it again. And for that, apply worker has to keep
    checking it.
    
    > I think we can
    > evaluate these cases separately and can decide to write a top-up patch
    > if found useful to make sequence-sync worker detect and exit.
    >
    
    Sure, we can do that.
    
    thanks
    Shveta
    
    
    
    
  31. Re: [PATCH] Support automatic sequence replication

    Dilip Kumar <dilipbalaut@gmail.com> — 2026-02-20T04:40:40Z

    On Tue, Feb 3, 2026 at 9:18 AM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > Hello hackers,
    >
    > I'd like to propose an improvement to the sequence replication feature
    > that was committed in [1].
    >
    > The current implementation synchronizes sequences during initial
    > subscription setup, but the sequence sync worker exits after this
    > initial sync. This means that as sequences advance on the publisher,
    > they drift from the subscriber values over time. Users must manually
    > run ALTER SUBSCRIPTION ... REFRESH SEQUENCES to resynchronize, which
    > requires monitoring and intervention.
    
    Thanks, Ajin, for the proposal. I am trying to think: what is the use
    case for automatically updating the sequence values? IIUC, the only
    use case for the sequence sync worker was when using logical
    replication for an upgrade; after the upgrade, you should have
    up-to-date values for the sequences. By adding an automatic update,
    are you targeting any new use case?
    
    If we are still targeting the same use case, I’d like to understand
    how updating the value at specific intervals will improve the user
    experience.
    
    -- 
    Regards,
    Dilip Kumar
    Google
    
    
    
    
  32. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-20T05:18:28Z

    On Fri, Feb 20, 2026 at 10:11 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
    >
    > On Tue, Feb 3, 2026 at 9:18 AM Ajin Cherian <itsajin@gmail.com> wrote:
    > >
    > > Hello hackers,
    > >
    > > I'd like to propose an improvement to the sequence replication feature
    > > that was committed in [1].
    > >
    > > The current implementation synchronizes sequences during initial
    > > subscription setup, but the sequence sync worker exits after this
    > > initial sync. This means that as sequences advance on the publisher,
    > > they drift from the subscriber values over time. Users must manually
    > > run ALTER SUBSCRIPTION ... REFRESH SEQUENCES to resynchronize, which
    > > requires monitoring and intervention.
    >
    > Thanks, Ajin, for the proposal. I am trying to think: what is the use
    > case for automatically updating the sequence values? IIUC, the only
    > use case for the sequence sync worker was when using logical
    > replication for an upgrade; after the upgrade, you should have
    > up-to-date values for the sequences. By adding an automatic update,
    > are you targeting any new use case?
    >
    > If we are still targeting the same use case, I’d like to understand
    > how updating the value at specific intervals will improve the user
    > experience.
    >
    
    Even for an upgrade, to avoid large downtime, it would be better if
    the subscriber has the latest values of sequences, so that during the
    upgrade of the publisher, the subscriber can receive all client
    requests. It is possible with the REFRESH SEQUENCES command as well
    but in the worst case (when there are large number of sequences), it
    can take much longer time.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  33. Re: [PATCH] Support automatic sequence replication

    Dilip Kumar <dilipbalaut@gmail.com> — 2026-02-20T06:24:22Z

    On Fri, Feb 20, 2026 at 10:48 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Fri, Feb 20, 2026 at 10:11 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
    > >
    > > On Tue, Feb 3, 2026 at 9:18 AM Ajin Cherian <itsajin@gmail.com> wrote:
    > > >
    > > > Hello hackers,
    > > >
    > > > I'd like to propose an improvement to the sequence replication feature
    > > > that was committed in [1].
    > > >
    > > > The current implementation synchronizes sequences during initial
    > > > subscription setup, but the sequence sync worker exits after this
    > > > initial sync. This means that as sequences advance on the publisher,
    > > > they drift from the subscriber values over time. Users must manually
    > > > run ALTER SUBSCRIPTION ... REFRESH SEQUENCES to resynchronize, which
    > > > requires monitoring and intervention.
    > >
    > > Thanks, Ajin, for the proposal. I am trying to think: what is the use
    > > case for automatically updating the sequence values? IIUC, the only
    > > use case for the sequence sync worker was when using logical
    > > replication for an upgrade; after the upgrade, you should have
    > > up-to-date values for the sequences. By adding an automatic update,
    > > are you targeting any new use case?
    > >
    > > If we are still targeting the same use case, I’d like to understand
    > > how updating the value at specific intervals will improve the user
    > > experience.
    > >
    >
    > Even for an upgrade, to avoid large downtime, it would be better if
    > the subscriber has the latest values of sequences, so that during the
    > upgrade of the publisher, the subscriber can receive all client
    > requests. It is possible with the REFRESH SEQUENCES command as well
    > but in the worst case (when there are large number of sequences), it
    > can take much longer time.
    
    To clarify, here is the specific sequence synchronization use case I
    have in mind (major version upgrade with minimal downtime )
    
    Setup: A PUB-SUB environment, both on PG17.
    Goal: Upgrade to PG18 with near-zero downtime.
    Upgrade: Run pg_upgrade on the Subscriber to move it to PG18.
    Replication: Logical replication continues from the PG17 Publisher to
    the PG18 Subscriber, including sequences.
    Sync: Wait for replication lag to hit near-zero and synchronize sequences.
    Cutover: Stop all traffic on the Publisher.
    Final Catch-up: Allow the PG18 Subscriber to catch up and perform a
    final sequence refresh.
    Switchover: Redirect all traffic to the Subscriber.
    
    In this major version upgrade scenario, the downtime is minimal (only
    while waiting for Final Catch-up). However, before the final
    switchover, the user must ensure all data and sequences are fully
    synced. They usually can't afford to wait for an automatic background
    sync, they need to force it using REFRESH SEQUENCES.
    
    My question is, does the proposed automatic sync allow us to avoid
    manual refreshes entirely in this process? Or is there another use
    case where the advantages of automation are more apparent?
    
    --
    Regards,
    Dilip Kumar
    Google
    
    
    
    
  34. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-20T06:48:02Z

    On Fri, Feb 20, 2026 at 11:54 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
    >
    > To clarify, here is the specific sequence synchronization use case I
    > have in mind (major version upgrade with minimal downtime )
    >
    > Setup: A PUB-SUB environment, both on PG17.
    > Goal: Upgrade to PG18 with near-zero downtime.
    > Upgrade: Run pg_upgrade on the Subscriber to move it to PG18.
    > Replication: Logical replication continues from the PG17 Publisher to
    > the PG18 Subscriber, including sequences.
    > Sync: Wait for replication lag to hit near-zero and synchronize sequences.
    > Cutover: Stop all traffic on the Publisher.
    > Final Catch-up: Allow the PG18 Subscriber to catch up and perform a
    > final sequence refresh.
    >
    
    In the "Final Catch-up" phase, without automatic sequence sync, there
    is a possibility that the REFRESH SEQUENCES command may take a lot of
    time to finish and in turn can delay switchover. With automatic
    sequence sync, in best case, all sequences were already synced, and
    even in average or worst case, the sync time would be much less.
    
    > Switchover: Redirect all traffic to the Subscriber.
    >
    > In this major version upgrade scenario, the downtime is minimal (only
    > while waiting for Final Catch-up). However, before the final
    > switchover, the user must ensure all data and sequences are fully
    > synced. They usually can't afford to wait for an automatic background
    > sync, they need to force it using REFRESH SEQUENCES.
    >
    > My question is, does the proposed automatic sync allow us to avoid
    > manual refreshes entirely in this process?
    >
    
    It can avoid manual refreshes entirely in most cases which the user
    can verify by checking required LSN which we discussed to update in
    docs. Even when manual REFRESH command is required, the time consumed
    by the command will be much less, especially when there are large
    number (say a million sequences) of sequences that need to be synced.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  35. Re: [PATCH] Support automatic sequence replication

    Ajin Cherian <itsajin@gmail.com> — 2026-02-20T10:57:16Z

    On Fri, Feb 13, 2026 at 4:43 PM Peter Smith <smithpb2250@gmail.com> wrote:
    >
    > Hi Ajin.
    >
    > Some review comments for patch v4-0001
    >
    > ======
    > src/backend/commands/sequence.c
    >
    > GetSequence:
    >
    > 1.
    > +/*
    > + * Read the current sequence values (last_value and is_called)
    > + *
    > + * This is a read-only operation that acquires AccessShareLock on the sequence.
    > + * Used by logical replication sequence synchronization to detect drift.
    > + */
    >
    > The comment seems stale. e.g. the function is not acquiring a lock
    > anymore, contrary to what that comment says.
    >
    
    Fixed.
    
    >
    > ======
    > .../replication/logical/sequencesync.c
    >
    > 2.
    > -static List *seqinfos = NIL;
    >
    > The removal of this global was not strictly part of this patch; it is
    > more like a prerequisite to make everything tidier, so your new code
    > does not go further down that track of side-affecting a global. From
    > that POV, I thought this global removal should be implemented as a
    > first/separate (0001) patch so that it might be quickly reviewed and
    > committed independently of the new seq-sync logic.
    >
    
    Keeping it as is for the time being. Let's see what everybody else thinks.
    
    > ~~~
    >
    > LogicalRepSyncSequences:
    >
    > 3.
    > + /* Error on unexpected states */
    > + if (relstate != SUBREL_STATE_INIT && relstate != SUBREL_STATE_READY)
    > + {
    > + table_close(sequence_rel, NoLock);
    > + ereport(ERROR,
    > + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    > + errmsg("unexpected relstate '%c' for sequence \"%s.%s\" in
    > subscription \"%s\"",
    > + relstate,
    > + get_namespace_name(RelationGetNamespace(sequence_rel)),
    > + RelationGetRelationName(sequence_rel),
    > + MySubscription->name)));
    > + }
    > +
    >
    > How is this possible? Should it just be Assert?
    >
    
    Fixed.
    
    > ~~~
    >
    > start_sequence_sync:
    >
    > 4.
    > + /*
    > + * Synchronize all sequences (both READY and INIT states).
    > + * The function will process INIT sequences first, then READY sequences.
    > + */
    > + sequence_copied = LogicalRepSyncSequences(LogRepWorkerWalRcvConn);
    >
    > Why is talking about the processing order relevant?
    >
    >
    
    Removed.
    
    >
    > ======
    > src/backend/replication/logical/syncutils.c
    >
    > 5.
    > + /* Check if any new sequences need syncing */
    > + ProcessSequencesForSync();
    > +
    >
    > Maybe don't say "new" because IIUC it also handles older sequences
    > where the values have drifted.
    >
    > ======
    > src/test/subscription/t/036_sequences.pl
    >
    > 6.
    > -$result = $node_publisher->safe_psql(
    > - 'postgres', qq(
    > - SELECT last_value, is_called FROM regress_s1;
    > -));
    > -is($result, '200|t', 'Check sequence value in the publisher');
    > -
    > -# Check - existing sequence ('regress_s1') is not synced
    > -$result = $node_subscriber->safe_psql(
    > - 'postgres', qq(
    > - SELECT last_value, is_called FROM regress_s1;
    > -));
    > -is($result, '100|t', 'REFRESH PUBLICATION will not sync existing sequence');
    > -
    >
    > Since you are no longer testing "existing sequences" in this test
    > part, should you also remove the earlier INSERT for 'regress_s1'?
    >
    
    Fixed both the above comments.
    
    On Tue, Feb 17, 2026 at 10:21 PM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
    >
    >
    > How about retaining ALTER SUBSCRIPTION ... REFRESH SEQUENCES command?
    >
    > It can be useful in scenarios where automatic sequence replication
    > cannot be enabled, for example, due to the max_worker_processes limit
    > on the server preventing a new worker from being registered. Since
    > increasing max_worker_processes requires a server restart, which the
    > user may not want to perform immediately, this command would provide a
    > way to manually synchronize the sequences.
    >
    
    Retained ALTER SUBSCRIPTION ... REFRESH SEQUENCES similar to the
    behaviour on HEAD, it only changes the state of the sequence to INIT.
    The sequence worker will update these sequences unconditionally.
    
    >
    > One minor comment:
    >
    >  * Sequence state transitions follow this pattern:
    > - *   INIT -> READY
    > + *  INIT --> READY ->-+
    > + *             ^      | (check/synchronzize)
    > + *             |      |
    > + *             +--<---+
    >
    >
    > "synchronzize" → "synchronize"
    >
    
    Fixed.
    
    
    
    On Wed, Feb 18, 2026 at 9:04 PM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > Few comments:
    >
    > 1)
    > + /* Check if any new sequences need syncing */
    > + ProcessSequencesForSync();
    > +
    >
    > Shall we name it 'maybe_start_seqsync_worker()' (or something better?)
    > and move it immediately after 'maybe_advance_nonremovable_xid()'.
    > Thoughts? This is because we do not process sequences here, we just
    > check if the subscription includes sequences and start worker.
    >
    
    Since snak_case are used for static functions, I've renamed it to
    MaybeLaunchSequenceSyncWorker and called it immediately after
    maybe_advance_nonremovable_xid()
    
    > 2)
    > We can also change the comment atop ProcessSequencesForSync to mention
    > that. Currently it says:
    > /*
    >  * Apply worker determines if sequence synchronization is needed.
    >  *
    >  * Start a sequencesync worker if one is not already running.
    >
    > Now, we shall change it to:
    > Apply worker determines whether a sequence sync worker is needed.
    >
    > Check if the subscription includes sequences and start a sequencesync
    > worker if one is not already running.
    >
    
    Fixed.
    
    > 3)
    > I noticed that copy_sequences is invoked twice per sync cycle—once for
    > sequences in the INIT state, and once for sequences in the READY state
    > to detect drift. This means we ping the primary twice during each sync
    > cycle. We should consider combining this logic into a single
    > copy_sequences call. Since we already have the state information
    > (INIT, READY, etc.) for each local sequence, copy_sequences can
    > internally check the current state and decide whether to transition a
    > sequence to READY based on its previous state. This approach would
    > allow us to fetch all required information from the primary in a
    > single call.
    >
    
    Changed it.
    
    > I also think that we do not even need to mention the states here and
    > we can give a single message instead of 2:
    > DEBUG:  logical replication sequence synchronization for subscription
    > "sub1" - for sequences in INIT state batch #1 = 5 attempted, 5
    > succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    > publisher, 0 skipped, 0 no drift
    > DEBUG:  logical replication sequence synchronization for subscription
    > "sub1" - for sequences in READY state batch #1 = 5 attempted, 0
    > succeeded, 0 mismatched, 0 insufficient permission, 0 missing from
    > publisher, 0 skipped, 5 no drift
    >
    
    Yes, since states are now specific to each sequence and not to
    batches, I've removed it.
    
    I've also found that the sequence worker was not being stopped when
    SUBSCRIPTION was disabled. I've added checks to do this as well.
    
    All the above changes are part of the attached v5 patch .
    
    regards,
    Ajin Cherian
    Fujitsu Australia
    
  36. RE: [PATCH] Support automatic sequence replication

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-02-23T01:26:10Z

    Dear Ajin,
    
    Thanks for updating the patch. Here are my comments.
    
    01. start_sequence_sync()
    ```
       /* Need to start transaction for cache lookup */
       StartTransactionCommand();
    ```
    
    Here, we must check additional parameter changes, such as conninfo and passwordrequired.
    Also, it's inefficient because transactions are started each time.
    
    Can we re-use maybe_reread_subscription() here? Some parameters do not take
    effect for the sequence sync worker, but it is OK to exit even if they
    are changed. If we use the function, no need to include "storage/ipc.h".
    
    02. match_previous_words
    
    No need to remove "REFRESH SEQUENCES" anymore.
    
    03. CopySeqResult
    ```
     COPYSEQ_NOWORK,
    ```
    
    It describes why the copying is skipped. How about "COPYSEQ_NO_DRIFT"?
    
    04. LogicalRepSyncSequences()
    
    ```
      oldctx = MemoryContextSwitchTo(ApplyContext);
    
      seq = palloc0_object(LogicalRepSequenceInfo);
      seq->localrelid = subrel->srrelid;
      seq->nspname = get_namespace_name(RelationGetNamespace(sequence_rel));
      seq->seqname = pstrdup(RelationGetRelationName(sequence_rel));
      seq->relstate = relstate;
      seqinfos = lappend(seqinfos, seq);
    
      MemoryContextSwitchTo(oldctx);
    ```
    
    ISTM they are palloc'd but not pfree'd.
    
    Since the sequencesync worker now has a long lifetime, we must take care of the
    memory allocation/freeing more carefully. How about introducing per-interaction
    memory context like ApplyMessageContext?
    
    05. LogicalRepApplyLoop()
    
    MaybeLaunchSequenceSyncWorker() should be called more; otherwise, the sequencesync
    worker won't be launched if the worker always receives messages and WL_TIMEOUT does
    not happen. Can you add most of the places under maybe_advance_nonremovable_xid()?
    Personally considered, no need to add within `else if (c == PqReplMsg_PrimaryStatusUpdate)`
    because it just consumes status updates from the primary.
    
    06.
    Not sure if the issue should be discussed here, but I found that sequences more
    likely to go backward if users use sequences on the subscriber side.
    Previously, the sync could happen based on the request, and users could understand
    the risk. But now everything would be done automatically, thus they may be
    surprised more.
    
    Should we consider some ratchet mechanisms, or retain it now because it's not
    expected usage?
    
    E.g., `nextval()` is called three times, and synchronization occurs between them.
    
    ```
    subscriber=# SELECT nextval('seq');
     nextval
    ---------
           2
    (1 row)
    
    subscriber=# SELECT nextval('seq');
     nextval
    ---------
           3
    (1 row)
    
    subscriber=# -- synchronization happened
    subscriber=# SELECT nextval('seq');
     nextval
    ---------
           1
    (1 row)
    ```
    
    07.
    Question: Can we introduce an intermediate state, such as SYNC, to clarify
    whether synchronization is proceeding?
    
    Best regards,
    Hayato Kuroda
    FUJITSU LIMITED
    
    
  37. Re: [PATCH] Support automatic sequence replication

    Dilip Kumar <dilipbalaut@gmail.com> — 2026-02-23T05:14:37Z

    On Fri, Feb 20, 2026 at 12:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Fri, Feb 20, 2026 at 11:54 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
    > >
    > > To clarify, here is the specific sequence synchronization use case I
    > > have in mind (major version upgrade with minimal downtime )
    > >
    > > Setup: A PUB-SUB environment, both on PG17.
    > > Goal: Upgrade to PG18 with near-zero downtime.
    > > Upgrade: Run pg_upgrade on the Subscriber to move it to PG18.
    > > Replication: Logical replication continues from the PG17 Publisher to
    > > the PG18 Subscriber, including sequences.
    > > Sync: Wait for replication lag to hit near-zero and synchronize sequences.
    > > Cutover: Stop all traffic on the Publisher.
    > > Final Catch-up: Allow the PG18 Subscriber to catch up and perform a
    > > final sequence refresh.
    > >
    >
    > In the "Final Catch-up" phase, without automatic sequence sync, there
    > is a possibility that the REFRESH SEQUENCES command may take a lot of
    > time to finish and in turn can delay switchover. With automatic
    > sequence sync, in best case, all sequences were already synced, and
    > even in average or worst case, the sync time would be much less.
    >
    > > Switchover: Redirect all traffic to the Subscriber.
    > >
    > > In this major version upgrade scenario, the downtime is minimal (only
    > > while waiting for Final Catch-up). However, before the final
    > > switchover, the user must ensure all data and sequences are fully
    > > synced. They usually can't afford to wait for an automatic background
    > > sync, they need to force it using REFRESH SEQUENCES.
    > >
    > > My question is, does the proposed automatic sync allow us to avoid
    > > manual refreshes entirely in this process?
    > >
    >
    > It can avoid manual refreshes entirely in most cases which the user
    > can verify by checking required LSN which we discussed to update in
    > docs. Even when manual REFRESH command is required, the time consumed
    > by the command will be much less, especially when there are large
    > number (say a million sequences) of sequences that need to be synced.
    
    Thanks for clarifying the use case, and I agree this is a valid use
    case, although I am not completely sure how exactly we are achieving
    that? I mean sequence sync workers need to fetch the list of all
    published sequences to identify whether they are in sync with the
    local sequence state or not?  Or somehow we can avoid that?  If not
    what we would save, updating the local states of the sequences if they
    are already in sync?
    
    -- 
    Regards,
    Dilip Kumar
    Google
    
    
    
    
  38. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-23T05:36:33Z

    On Mon, Feb 23, 2026 at 10:44 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
    >
    > On Fri, Feb 20, 2026 at 12:18 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > >
    > > On Fri, Feb 20, 2026 at 11:54 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
    > > >
    > > > To clarify, here is the specific sequence synchronization use case I
    > > > have in mind (major version upgrade with minimal downtime )
    > > >
    > > > Setup: A PUB-SUB environment, both on PG17.
    > > > Goal: Upgrade to PG18 with near-zero downtime.
    > > > Upgrade: Run pg_upgrade on the Subscriber to move it to PG18.
    > > > Replication: Logical replication continues from the PG17 Publisher to
    > > > the PG18 Subscriber, including sequences.
    > > > Sync: Wait for replication lag to hit near-zero and synchronize sequences.
    > > > Cutover: Stop all traffic on the Publisher.
    > > > Final Catch-up: Allow the PG18 Subscriber to catch up and perform a
    > > > final sequence refresh.
    > > >
    > >
    > > In the "Final Catch-up" phase, without automatic sequence sync, there
    > > is a possibility that the REFRESH SEQUENCES command may take a lot of
    > > time to finish and in turn can delay switchover. With automatic
    > > sequence sync, in best case, all sequences were already synced, and
    > > even in average or worst case, the sync time would be much less.
    > >
    > > > Switchover: Redirect all traffic to the Subscriber.
    > > >
    > > > In this major version upgrade scenario, the downtime is minimal (only
    > > > while waiting for Final Catch-up). However, before the final
    > > > switchover, the user must ensure all data and sequences are fully
    > > > synced. They usually can't afford to wait for an automatic background
    > > > sync, they need to force it using REFRESH SEQUENCES.
    > > >
    > > > My question is, does the proposed automatic sync allow us to avoid
    > > > manual refreshes entirely in this process?
    > > >
    > >
    > > It can avoid manual refreshes entirely in most cases which the user
    > > can verify by checking required LSN which we discussed to update in
    > > docs. Even when manual REFRESH command is required, the time consumed
    > > by the command will be much less, especially when there are large
    > > number (say a million sequences) of sequences that need to be synced.
    >
    > Thanks for clarifying the use case, and I agree this is a valid use
    > case, although I am not completely sure how exactly we are achieving
    > that? I mean sequence sync workers need to fetch the list of all
    > published sequences to identify whether they are in sync with the
    > local sequence state or not?  Or somehow we can avoid that?  If not
    > what we would save, updating the local states of the sequences if they
    > are already in sync?
    >
    
    We would save updating the local state per-sequence which involves
    writing to sequence relation page, a WAL recording, and updating the
    catalog-state. I feel that for a large number of sequences this cost
    could be high. Shveta or Ajin can add if I am missing or misunderstood
    anything.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  39. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-23T05:44:19Z

    On Mon, Feb 23, 2026 at 6:56 AM Hayato Kuroda (Fujitsu)
    <kuroda.hayato@fujitsu.com> wrote:
    >
    > 06.
    > Not sure if the issue should be discussed here, but I found that sequences more
    > likely to go backward if users use sequences on the subscriber side.
    > Previously, the sync could happen based on the request, and users could understand
    > the risk. But now everything would be done automatically, thus they may be
    > surprised more.
    >
    > Should we consider some ratchet mechanisms, or retain it now because it's not
    > expected usage?
    >
    
    We discussed this case upthread. We ideally can handle it via
    conflict/resolution strategy or simply avoid updating the sequences
    that are synced from the publisher. If we do later it would be tricky
    because we need to maintain a persistent state and then after
    failover, that state should be cleared. We discussed to have it
    documented that users should use such sequences only during upgrade or
    for failover cases, allowing to update it actively on multiple nodes
    can lead to inconsistency.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  40. RE: [PATCH] Support automatic sequence replication

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-02-23T05:44:46Z

    Dear Dilip,
    
    > Thanks for clarifying the use case, and I agree this is a valid use
    > case, although I am not completely sure how exactly we are achieving
    > that? I mean sequence sync workers need to fetch the list of all
    > published sequences to identify whether they are in sync with the
    > local sequence state or not?  Or somehow we can avoid that?  If not
    > what we would save, updating the local states of the sequences if they
    > are already in sync?
    
    IIUC, the sequencesync worker first lists all sequences on the subscriber, then
    gets their states on the publisher side. Then the worker updates the state only
    when the sequence between instances is different.
    
    Does it make sense for you?
    
    Best regards,
    Hayato Kuroda
    FUJITSU LIMITED
    
    
    
    
    
  41. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-24T05:36:15Z

    On Mon, Feb 23, 2026 at 6:56 AM Hayato Kuroda (Fujitsu)
    <kuroda.hayato@fujitsu.com> wrote:
    >
    > Thanks for updating the patch. Here are my comments.
    >
    > 01. start_sequence_sync()
    > ```
    >    /* Need to start transaction for cache lookup */
    >    StartTransactionCommand();
    > ```
    >
    > Here, we must check additional parameter changes, such as conninfo and passwordrequired.
    > Also, it's inefficient because transactions are started each time.
    >
    > Can we re-use maybe_reread_subscription() here? Some parameters do not take
    > effect for the sequence sync worker, but it is OK to exit even if they
    > are changed. If we use the function, no need to include "storage/ipc.h".
    >
    
    I also think it is better to restart the sequencesync worker on
    subscription parameter change and probably it is okay to use
    maybe_reread_subscription() but need to be careful to avoid anything
    apply_worker specific in that function. BTW, what happens now without
    this change when apply_worker stops due to parameter change, does
    sequencesync worker continue? If so, we should make it restart as we
    are doing for apply worker.
    
    Also, shouldn't we need to invoke AcceptInvalidationMessages() as we
    are doing in apply worker when not in a remote transaction? I think it
    will be required to get local_sequence definition changes , if any.
    
    > 02. match_previous_words
    >
    > No need to remove "REFRESH SEQUENCES" anymore.
    >
    > 03. CopySeqResult
    > ```
    >  COPYSEQ_NOWORK,
    > ```
    >
    > It describes why the copying is skipped. How about "COPYSEQ_NO_DRIFT"?
    >
    
    +1.
    
    > 04. LogicalRepSyncSequences()
    >
    > ```
    >   oldctx = MemoryContextSwitchTo(ApplyContext);
    >
    >   seq = palloc0_object(LogicalRepSequenceInfo);
    >   seq->localrelid = subrel->srrelid;
    >   seq->nspname = get_namespace_name(RelationGetNamespace(sequence_rel));
    >   seq->seqname = pstrdup(RelationGetRelationName(sequence_rel));
    >   seq->relstate = relstate;
    >   seqinfos = lappend(seqinfos, seq);
    >
    >   MemoryContextSwitchTo(oldctx);
    > ```
    >
    > ISTM they are palloc'd but not pfree'd.
    >
    
    They are freed, see following code. But it seems nspname and seqname
    should be freed separately for each sequence element and each sequence
    element also needs to be freed independently.
    
    + /* Clean up */
    + list_free(seqinfos);
    
    > Since the sequencesync worker now has a long lifetime, we must take care of the
    > memory allocation/freeing more carefully. How about introducing per-interaction
    > memory context like ApplyMessageContext?
    >
    
    Yeah, I feel that would be a better approach than retail pfree
    especially because it is also required at other places in sequencesync
    worker (see places where currently ApplyContext is used in
    sequencesync worker). I think it would be better if we name this new
    context as SequenceSyncContext.
    
    Few other miscellaneous comments
    =================================
    1.
      <sect2 id="sequences-out-of-sync">
    -   <title>Refreshing Out-of-Sync Sequences</title>
    -   <para>
    -    Subscriber sequence values will become out of sync as the publisher
    -    advances them.
    -   </para>
    -   <para>
    -    To detect this, compare the
    -    <link linkend="catalog-pg-subscription-rel">pg_subscription_rel</link>.<structfield>srsublsn</structfield>
    -    on the subscriber with the <structfield>page_lsn</structfield> obtained
    -    from the <link
    linkend="func-pg-get-sequence-data"><function>pg_get_sequence_data</function></link>
    -    function for the sequence on the publisher. Then run
    -    <link linkend="sql-altersubscription-params-refresh-sequences">
    -    <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link> to
    -    re-synchronize if necessary.
    -   </para>
    +   <title>Out-of-Sync Sequences</title>
        <warning>
         <para>
          Each sequence caches a block of values (typically 32) in memory before
    @@ -1961,16 +1941,6 @@ Publications:
     ------------+-----------+------------
             610 | t         | 0/017CEDF8
     (1 row)
    -</programlisting></para>
    -
    -   <para>
    -    The difference between the sequence page LSNs on the publisher and the
    -    sequence page LSNs on the subscriber indicates that the sequences are out
    -    of sync. Re-synchronize all sequences known to the subscriber using
    -    <link linkend="sql-altersubscription-params-refresh-sequences">
    -    <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link>.
    -<programlisting>
    -/* sub # */ ALTER SUBSCRIPTION sub1 REFRESH SEQUENCES;
    
    ...
    ...
    -   <listitem>
    -    <para>
    -     Incremental sequence changes are not replicated.  Although the data in
    -     serial or identity columns backed by sequences will be replicated as part
    -     of the table, the sequences themselves do not replicate ongoing changes.
    -     On the subscriber, a sequence will retain the last value it synchronized
    -     from the publisher. If the subscriber is used as a read-only database,
    -     then this should typically not be a problem.  If, however, some kind of
    -     switchover or failover to the subscriber database is intended, then the
    -     sequences would need to be updated to the latest values, either by
    -     executing <link linkend="sql-altersubscription-params-refresh-sequences">
    -     <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link>
    -     or by copying the current data from the publisher (perhaps using
    -     <command>pg_dump</command>) or by determining a sufficiently high value
    -     from the tables themselves.
    -    </para>
    -   </listitem>
    -
    
    I think this can still happen after this patch but chances are much
    lower, so we need some of this before upgrade. We should reword it
    accordingly. Similarly check other parts of the doc you removed.
    
    2.
    - "logical replication synchronization for subscription \"%s\",
    sequence \"%s.%s\" has finished",
    + "logical replication sync for subscription \"%s\", sequence
    \"%s.%s\" has been updated",
    
    Is there a need to shorten synchronization to sync in above message?
    
    3.
      elog(DEBUG1,
    - "logical replication sequence synchronization for subscription
    \"%s\" - batch #%d = %d attempted, %d succeeded, %d mismatched, %d
    insufficient permission, %d missing from publisher, %d skipped",
    - MySubscription->name,
    - (cur_batch_base_index / MAX_SEQUENCES_SYNC_PER_BATCH) + 1,
    - batch_size, batch_succeeded_count, batch_mismatched_count,
    - batch_insuffperm_count, batch_missing_count, batch_skipped_count);
    + "logical replication sequence synchronization for subscription
    \"%s\" - batch #%d = %d attempted, %d succeeded, %d mismatched, %d
    insufficient permission, %d missing from publisher, %d skipped, %d no
    drift",
    + MySubscription->name,
    + (cur_batch_base_index / MAX_SEQUENCES_SYNC_PER_BATCH) + 1,
    + batch_size, batch_succeeded_count, batch_mismatched_count,
    + batch_insuffperm_count, batch_missing_count, batch_skipped_count,
    batch_no_drift);
    
    Here, the formatting of message is incorrect.
    
    4.
      table_states_not_ready = NIL;
    -
      if (!IsTransactionState())
    
    Spurious line removal.
    
    5.
    MemoryContextSwitchTo(oldctx);
    -
    + list_free_deep(seq_states);
      /*
      * Does the subscription have tables?
      *
    @@ -260,7 +253,6 @@ FetchRelationStates(bool *has_pending_subtables,
      */
      has_subtables = (table_states_not_ready != NIL) ||
      HasSubscriptionTables(MySubscription->oid);
    -
      /*
      * If the subscription relation cache has been invalidated since we
      * entered this routine, we still use and return the relations we just
    @@ -271,10 +263,8 @@ FetchRelationStates(bool *has_pending_subtables,
      if (relation_states_validity == SYNC_RELATIONS_STATE_REBUILD_STARTED)
      relation_states_validity = SYNC_RELATIONS_STATE_VALID;
      }
    -
      if (has_pending_subtables)
      *has_pending_subtables = has_subtables;
    -
      if (has_pending_subsequences)
    ...
    ...
    
      }
    
    +
      if (rc & WL_TIMEOUT)
    
    All above places have spurious line removals and additions which made
    code harder to understand.
    
    6.
      else if (Matches("ALTER", "SUBSCRIPTION", MatchAny))
      COMPLETE_WITH("CONNECTION", "ENABLE", "DISABLE", "OWNER TO",
    -   "RENAME TO", "REFRESH PUBLICATION", "REFRESH SEQUENCES",
    -   "SET", "SKIP (", "ADD PUBLICATION", "DROP PUBLICATION");
    +   "RENAME TO", "REFRESH PUBLICATION", "SET", "SKIP (",
    +   "ADD PUBLICATION", "DROP PUBLICATION");
    
    unrelated change.
    
    7.
    + else
    + {
    +
    + /*
    + * Double the sleep time, but not beyond
    + * the maximum allowable value.
    + */
    + sleep_ms = Min(sleep_ms * 2, SEQSYNC_MAX_SLEEP_MS);
    
    Comments are not properly aligned.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  42. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-24T06:17:24Z

    On Mon, Feb 23, 2026 at 6:56 AM Hayato Kuroda (Fujitsu)
    <kuroda.hayato@fujitsu.com> wrote:
    >
    > 05. LogicalRepApplyLoop()
    >
    > MaybeLaunchSequenceSyncWorker() should be called more; otherwise, the sequencesync
    > worker won't be launched if the worker always receives messages and WL_TIMEOUT does
    > not happen. Can you add most of the places under maybe_advance_nonremovable_xid()?
    > Personally considered, no need to add within `else if (c == PqReplMsg_PrimaryStatusUpdate)`
    > because it just consumes status updates from the primary.
    >
    
    I don't think we need to be as aggressive as
    maybe_advance_nonremovable_xid because not doing that can lead to
    bload if slot is not advanced. The only minor downside with checking
    too frequently is that we need to traverse the all logical replication
    workers to find if sequencesync worker is available. I feel doing in
    ProcessSyncingRelations() where earlier we were doing
    ProcessSequencesForSync() should be sufficient. Can we find some cheap
    way to detect if sequencesync worker is present or not? Can you think
    some other way to not incur the cost of traversing the worker array
    and also detect sequence worker exit without much delay?
    
    ...
    >
    > 07.
    > Question: Can we introduce an intermediate state, such as SYNC, to clarify
    > whether synchronization is proceeding?
    >
    
    What is the advantage of this? For external purposes, the presence of
    sequencesync worker, which can be checked via pg_stat_subscription
    should be sufficient.
    
    BTW, what is the behavior of REFRESH SEQUENCES command if the sequence
    worker is active? Does it still try to refresh sequences, if so, is
    that required/good idea?
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  43. Re: [PATCH] Support automatic sequence replication

    Dilip Kumar <dilipbalaut@gmail.com> — 2026-02-24T10:37:00Z

    On Mon, Feb 23, 2026 at 11:14 AM Hayato Kuroda (Fujitsu)
    <kuroda.hayato@fujitsu.com> wrote:
    >
    > Dear Dilip,
    >
    > > Thanks for clarifying the use case, and I agree this is a valid use
    > > case, although I am not completely sure how exactly we are achieving
    > > that? I mean sequence sync workers need to fetch the list of all
    > > published sequences to identify whether they are in sync with the
    > > local sequence state or not?  Or somehow we can avoid that?  If not
    > > what we would save, updating the local states of the sequences if they
    > > are already in sync?
    >
    > IIUC, the sequencesync worker first lists all sequences on the subscriber, then
    > gets their states on the publisher side. Then the worker updates the state only
    > when the sequence between instances is different.
    >
    > Does it make sense for you?
    
    Thanks, yeah that absolutely makes sense, one of the key requirement
    while using the logical replication for the major version upgrade is
    how long is the downtime before switchover and thats directly
    proportional to the time we take in syncing.  So with automatic
    sequence sync if we are ensuring that only the sequences which were
    not synced automatically are synced with REFRESH that then this would
    be very good feature addition.  However, I will look into the patch to
    see how we are taking care of syncing the sequences which are not
    already synced.  Have we done any time measurement with very large
    number of sequences, that how much time REFRESH SEQUENCES takes with
    or without patch?
    BEST case(sequences are rarely getting changed), AVERAGE case(only
    some sequences are getting frequently modified and others are not) and
    WORST case(all sequences are frequently getting modified)
    
    -- 
    Regards,
    Dilip Kumar
    Google
    
    
    
    
  44. Re: [PATCH] Support automatic sequence replication

    Ajin Cherian <itsajin@gmail.com> — 2026-02-24T11:04:26Z

    On Tue, Feb 24, 2026 at 4:36 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Mon, Feb 23, 2026 at 6:56 AM Hayato Kuroda (Fujitsu)
    > <kuroda.hayato@fujitsu.com> wrote:
    > >
    > > Thanks for updating the patch. Here are my comments.
    > >
    > > 01. start_sequence_sync()
    > > ```
    > >    /* Need to start transaction for cache lookup */
    > >    StartTransactionCommand();
    > > ```
    > >
    > > Here, we must check additional parameter changes, such as conninfo and passwordrequired.
    > > Also, it's inefficient because transactions are started each time.
    > >
    > > Can we re-use maybe_reread_subscription() here? Some parameters do not take
    > > effect for the sequence sync worker, but it is OK to exit even if they
    > > are changed. If we use the function, no need to include "storage/ipc.h".
    > >
    >
    > I also think it is better to restart the sequencesync worker on
    > subscription parameter change and probably it is okay to use
    > maybe_reread_subscription() but need to be careful to avoid anything
    > apply_worker specific in that function. BTW, what happens now without
    > this change when apply_worker stops due to parameter change, does
    > sequencesync worker continue? If so, we should make it restart as we
    > are doing for apply worker.
    >
    
    I have changed it to use maybe_reread_subscription() and modified
    maybe_reread_subscription() to have have some special handling
    sequence sync workers as well.
    Yes, the apply worker will again restart the sequence worker when it
    finds that it isn't running and there are sequences in the
    subscription_rel.
    
    
    > Also, shouldn't we need to invoke AcceptInvalidationMessages() as we
    > are doing in apply worker when not in a remote transaction? I think it
    > will be required to get local_sequence definition changes , if any.
    
    I will need to investigate this further.
    
    >
    > > 02. match_previous_words
    > >
    > > No need to remove "REFRESH SEQUENCES" anymore.
    > >
    > > 03. CopySeqResult
    > > ```
    > >  COPYSEQ_NOWORK,
    > > ```
    > >
    > > It describes why the copying is skipped. How about "COPYSEQ_NO_DRIFT"?
    > >
    >
    > +1.
    
    Fixed.
    
    >
    > > 04. LogicalRepSyncSequences()
    > >
    > > ```
    > >   oldctx = MemoryContextSwitchTo(ApplyContext);
    > >
    > >   seq = palloc0_object(LogicalRepSequenceInfo);
    > >   seq->localrelid = subrel->srrelid;
    > >   seq->nspname = get_namespace_name(RelationGetNamespace(sequence_rel));
    > >   seq->seqname = pstrdup(RelationGetRelationName(sequence_rel));
    > >   seq->relstate = relstate;
    > >   seqinfos = lappend(seqinfos, seq);
    > >
    > >   MemoryContextSwitchTo(oldctx);
    > > ```
    > >
    > > ISTM they are palloc'd but not pfree'd.
    > >
    >
    > They are freed, see following code. But it seems nspname and seqname
    > should be freed separately for each sequence element and each sequence
    > element also needs to be freed independently.
    >
    > + /* Clean up */
    > + list_free(seqinfos);
    >
    > > Since the sequencesync worker now has a long lifetime, we must take care of the
    > > memory allocation/freeing more carefully. How about introducing per-interaction
    > > memory context like ApplyMessageContext?
    > >
    >
    > Yeah, I feel that would be a better approach than retail pfree
    > especially because it is also required at other places in sequencesync
    > worker (see places where currently ApplyContext is used in
    > sequencesync worker). I think it would be better if we name this new
    > context as SequenceSyncContext.
    >
    
    Done. Changed as requested.
    
    > Few other miscellaneous comments
    > =================================
    > 1.
    >   <sect2 id="sequences-out-of-sync">
    > -   <title>Refreshing Out-of-Sync Sequences</title>
    > -   <para>
    > -    Subscriber sequence values will become out of sync as the publisher
    > -    advances them.
    > -   </para>
    > -   <para>
    > -    To detect this, compare the
    > -    <link linkend="catalog-pg-subscription-rel">pg_subscription_rel</link>.<structfield>srsublsn</structfield>
    > -    on the subscriber with the <structfield>page_lsn</structfield> obtained
    > -    from the <link
    > linkend="func-pg-get-sequence-data"><function>pg_get_sequence_data</function></link>
    > -    function for the sequence on the publisher. Then run
    > -    <link linkend="sql-altersubscription-params-refresh-sequences">
    > -    <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link> to
    > -    re-synchronize if necessary.
    > -   </para>
    > +   <title>Out-of-Sync Sequences</title>
    >     <warning>
    >      <para>
    >       Each sequence caches a block of values (typically 32) in memory before
    > @@ -1961,16 +1941,6 @@ Publications:
    >  ------------+-----------+------------
    >          610 | t         | 0/017CEDF8
    >  (1 row)
    > -</programlisting></para>
    > -
    > -   <para>
    > -    The difference between the sequence page LSNs on the publisher and the
    > -    sequence page LSNs on the subscriber indicates that the sequences are out
    > -    of sync. Re-synchronize all sequences known to the subscriber using
    > -    <link linkend="sql-altersubscription-params-refresh-sequences">
    > -    <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link>.
    > -<programlisting>
    > -/* sub # */ ALTER SUBSCRIPTION sub1 REFRESH SEQUENCES;
    >
    > ...
    > ...
    > -   <listitem>
    > -    <para>
    > -     Incremental sequence changes are not replicated.  Although the data in
    > -     serial or identity columns backed by sequences will be replicated as part
    > -     of the table, the sequences themselves do not replicate ongoing changes.
    > -     On the subscriber, a sequence will retain the last value it synchronized
    > -     from the publisher. If the subscriber is used as a read-only database,
    > -     then this should typically not be a problem.  If, however, some kind of
    > -     switchover or failover to the subscriber database is intended, then the
    > -     sequences would need to be updated to the latest values, either by
    > -     executing <link linkend="sql-altersubscription-params-refresh-sequences">
    > -     <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link>
    > -     or by copying the current data from the publisher (perhaps using
    > -     <command>pg_dump</command>) or by determining a sufficiently high value
    > -     from the tables themselves.
    > -    </para>
    > -   </listitem>
    > -
    >
    > I think this can still happen after this patch but chances are much
    > lower, so we need some of this before upgrade. We should reword it
    > accordingly. Similarly check other parts of the doc you removed.
    >
    
    I have re-added them and modified them.
    
    > 2.
    > - "logical replication synchronization for subscription \"%s\",
    > sequence \"%s.%s\" has finished",
    > + "logical replication sync for subscription \"%s\", sequence
    > \"%s.%s\" has been updated",
    >
    > Is there a need to shorten synchronization to sync in above message?
    >
    
    Removed.
    
    > 3.
    >   elog(DEBUG1,
    > - "logical replication sequence synchronization for subscription
    > \"%s\" - batch #%d = %d attempted, %d succeeded, %d mismatched, %d
    > insufficient permission, %d missing from publisher, %d skipped",
    > - MySubscription->name,
    > - (cur_batch_base_index / MAX_SEQUENCES_SYNC_PER_BATCH) + 1,
    > - batch_size, batch_succeeded_count, batch_mismatched_count,
    > - batch_insuffperm_count, batch_missing_count, batch_skipped_count);
    > + "logical replication sequence synchronization for subscription
    > \"%s\" - batch #%d = %d attempted, %d succeeded, %d mismatched, %d
    > insufficient permission, %d missing from publisher, %d skipped, %d no
    > drift",
    > + MySubscription->name,
    > + (cur_batch_base_index / MAX_SEQUENCES_SYNC_PER_BATCH) + 1,
    > + batch_size, batch_succeeded_count, batch_mismatched_count,
    > + batch_insuffperm_count, batch_missing_count, batch_skipped_count,
    > batch_no_drift);
    >
    > Here, the formatting of message is incorrect.
    >
    
    Fixed.
    
    > 4.
    >   table_states_not_ready = NIL;
    > -
    >   if (!IsTransactionState())
    >
    > Spurious line removal.
    >
    > 5.
    > MemoryContextSwitchTo(oldctx);
    > -
    > + list_free_deep(seq_states);
    >   /*
    >   * Does the subscription have tables?
    >   *
    > @@ -260,7 +253,6 @@ FetchRelationStates(bool *has_pending_subtables,
    >   */
    >   has_subtables = (table_states_not_ready != NIL) ||
    >   HasSubscriptionTables(MySubscription->oid);
    > -
    >   /*
    >   * If the subscription relation cache has been invalidated since we
    >   * entered this routine, we still use and return the relations we just
    > @@ -271,10 +263,8 @@ FetchRelationStates(bool *has_pending_subtables,
    >   if (relation_states_validity == SYNC_RELATIONS_STATE_REBUILD_STARTED)
    >   relation_states_validity = SYNC_RELATIONS_STATE_VALID;
    >   }
    > -
    >   if (has_pending_subtables)
    >   *has_pending_subtables = has_subtables;
    > -
    >   if (has_pending_subsequences)
    > ...
    > ...
    >
    >   }
    >
    > +
    >   if (rc & WL_TIMEOUT)
    >
    > All above places have spurious line removals and additions which made
    > code harder to understand.
    >
    
    Fixed.
    
    > 6.
    >   else if (Matches("ALTER", "SUBSCRIPTION", MatchAny))
    >   COMPLETE_WITH("CONNECTION", "ENABLE", "DISABLE", "OWNER TO",
    > -   "RENAME TO", "REFRESH PUBLICATION", "REFRESH SEQUENCES",
    > -   "SET", "SKIP (", "ADD PUBLICATION", "DROP PUBLICATION");
    > +   "RENAME TO", "REFRESH PUBLICATION", "SET", "SKIP (",
    > +   "ADD PUBLICATION", "DROP PUBLICATION");
    >
    > unrelated change.
    >
    
    Removed.
    
    > 7.
    > + else
    > + {
    > +
    > + /*
    > + * Double the sleep time, but not beyond
    > + * the maximum allowable value.
    > + */
    > + sleep_ms = Min(sleep_ms * 2, SEQSYNC_MAX_SLEEP_MS);
    >
    > Comments are not properly aligned.
    >
    
    Fixed.
    
    On Tue, Feb 24, 2026 at 5:17 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Mon, Feb 23, 2026 at 6:56 AM Hayato Kuroda (Fujitsu)
    > <kuroda.hayato@fujitsu.com> wrote:
    > >
    > > 05. LogicalRepApplyLoop()
    > >
    > > MaybeLaunchSequenceSyncWorker() should be called more; otherwise, the sequencesync
    > > worker won't be launched if the worker always receives messages and WL_TIMEOUT does
    > > not happen. Can you add most of the places under maybe_advance_nonremovable_xid()?
    > > Personally considered, no need to add within `else if (c == PqReplMsg_PrimaryStatusUpdate)`
    > > because it just consumes status updates from the primary.
    > >
    >
    > I don't think we need to be as aggressive as
    > maybe_advance_nonremovable_xid because not doing that can lead to
    > bload if slot is not advanced. The only minor downside with checking
    > too frequently is that we need to traverse the all logical replication
    > workers to find if sequencesync worker is available. I feel doing in
    > ProcessSyncingRelations() where earlier we were doing
    > ProcessSequencesForSync() should be sufficient.
    
    Added it back as previous.
    
     Can we find some cheap
    > way to detect if sequencesync worker is present or not? Can you think
    > some other way to not incur the cost of traversing the worker array
    > and also detect sequence worker exit without much delay?
    >
    
    I will need to investigate this further.
    
    
    > ...
    > >
    > > 07.
    > > Question: Can we introduce an intermediate state, such as SYNC, to clarify
    > > whether synchronization is proceeding?
    > >
    >
    > What is the advantage of this? For external purposes, the presence of
    > sequencesync worker, which can be checked via pg_stat_subscription
    > should be sufficient.
    >
    > BTW, what is the behavior of REFRESH SEQUENCES command if the sequence
    > worker is active? Does it still try to refresh sequences, if so, is
    > that required/good idea?
    >
    
    Currently REFRESH SEQUENCES can only be called if the subscription is
    enabled. All it does is change the states of all the sequences in
    subscription_rel to INIT, this will prompt the sequence worker to wake
    up and unconditionally sync all the sequences. For sequences in the
    INIT state, it doesn't check if there is drift or not, it updates all
    sequences unconditionally. From your discussions with Dilip, I
    understand we want to reduce the time it takes to REFRESH SEQUENCES at
    the time of an upgrade. If so, then this might not be a good approach.
    As not only does the sequence worker have to update all sequences even
    if they have not drifted, it also has to update the relstate of all
    these sequences to READY. I propose, we change the logic such that
    REFRESH SEQUENCES only wakes up the sequence worker, that will reduce
    the time as most sequences will already be in READY state (unless
    newly added) and only sequences that have not drifted need to be
    updated and no catalog update is required for sequence states already
    in READY state. One downside to this is that, earlier users could
    issue a REFRESH SEQUENCES and wait to see if all the sequences have
    returned to the READY state to confirm that the sync has completed,
    with this approach that advantage is not there. Users might have to
    use a query to find the sequence values of all the sequences and
    compare that with the values in the publisher.
    
    I have addressed the above comments in the attached patch v6.
    
    regards,
    Ajin Cherian
    Fujitsu Australia
    
  45. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-24T11:25:20Z

    On Tue, Feb 24, 2026 at 4:34 PM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > Currently REFRESH SEQUENCES can only be called if the subscription is
    > enabled. All it does is change the states of all the sequences in
    > subscription_rel to INIT, this will prompt the sequence worker to wake
    > up and unconditionally sync all the sequences. For sequences in the
    > INIT state, it doesn't check if there is drift or not, it updates all
    > sequences unconditionally. From your discussions with Dilip, I
    > understand we want to reduce the time it takes to REFRESH SEQUENCES at
    > the time of an upgrade. If so, then this might not be a good approach.
    >
    
    The point I was trying to make is that with automatic sequence sync,
    we won't need to execute REFRESH SEQUENCES before upgrade or failover
    as the sequences will be in sync.
    
    > As not only does the sequence worker have to update all sequences even
    > if they have not drifted, it also has to update the relstate of all
    > these sequences to READY. I propose, we change the logic such that
    > REFRESH SEQUENCES only wakes up the sequence worker, that will reduce
    > the time as most sequences will already be in READY state (unless
    > newly added) and only sequences that have not drifted need to be
    > updated and no catalog update is required for sequence states already
    > in READY state. One downside to this is that, earlier users could
    > issue a REFRESH SEQUENCES and wait to see if all the sequences have
    > returned to the READY state to confirm that the sync has completed,
    > with this approach that advantage is not there. Users might have to
    > use a query to find the sequence values of all the sequences and
    > compare that with the values in the publisher.
    >
    
    Yeah, this point needs more thoughts as the existing method for
    REFRESH SEQUENCES also allows users to easily verify if the sequences
    are in sync.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  46. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-24T11:28:43Z

    On Tue, Feb 24, 2026 at 4:07 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
    >
    > On Mon, Feb 23, 2026 at 11:14 AM Hayato Kuroda (Fujitsu)
    > <kuroda.hayato@fujitsu.com> wrote:
    > >
    > > Dear Dilip,
    > >
    > > > Thanks for clarifying the use case, and I agree this is a valid use
    > > > case, although I am not completely sure how exactly we are achieving
    > > > that? I mean sequence sync workers need to fetch the list of all
    > > > published sequences to identify whether they are in sync with the
    > > > local sequence state or not?  Or somehow we can avoid that?  If not
    > > > what we would save, updating the local states of the sequences if they
    > > > are already in sync?
    > >
    > > IIUC, the sequencesync worker first lists all sequences on the subscriber, then
    > > gets their states on the publisher side. Then the worker updates the state only
    > > when the sequence between instances is different.
    > >
    > > Does it make sense for you?
    >
    > Thanks, yeah that absolutely makes sense, one of the key requirement
    > while using the logical replication for the major version upgrade is
    > how long is the downtime before switchover and thats directly
    > proportional to the time we take in syncing.  So with automatic
    > sequence sync if we are ensuring that only the sequences which were
    > not synced automatically are synced with REFRESH that then this would
    > be very good feature addition.  However, I will look into the patch to
    > see how we are taking care of syncing the sequences which are not
    > already synced.
    >
    
    This is done by comparing local and remote sequence values.
    
      Have we done any time measurement with very large
    > number of sequences, that how much time REFRESH SEQUENCES takes with
    > or without patch?
    
    Even if done, nothing is shared on -hackers. So, we should do some
    measurement in this regard, if not done already, and share the data.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  47. Re: [PATCH] Support automatic sequence replication

    Dilip Kumar <dilipbalaut@gmail.com> — 2026-02-24T13:18:43Z

    On Tue, Feb 24, 2026 at 4:55 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Tue, Feb 24, 2026 at 4:34 PM Ajin Cherian <itsajin@gmail.com> wrote:
    > >
    > > Currently REFRESH SEQUENCES can only be called if the subscription is
    > > enabled. All it does is change the states of all the sequences in
    > > subscription_rel to INIT, this will prompt the sequence worker to wake
    > > up and unconditionally sync all the sequences. For sequences in the
    > > INIT state, it doesn't check if there is drift or not, it updates all
    > > sequences unconditionally. From your discussions with Dilip, I
    > > understand we want to reduce the time it takes to REFRESH SEQUENCES at
    > > the time of an upgrade. If so, then this might not be a good approach.
    > >
    >
    > The point I was trying to make is that with automatic sequence sync,
    > we won't need to execute REFRESH SEQUENCES before upgrade or failover
    > as the sequences will be in sync.
    >
    
    That is a valid goal, however, the sequence sync worker is an
    asynchronous process triggered at specific intervals. For a switchover
    to guarantee consistency, we must disconnect all connections from the
    current publisher and wait for all pending data to sync. Relying on an
    automated, interval-based worker at this critical time is risky, as it
    directly extends our downtime. To minimize the cutover window, we
    should either treat sequence data the same as relational data
    (synchronous streaming) or manually trigger for the sequence sync
    during the switchover phase, i.e. REFRESH SEQUENCES.  Anyway this is
    my understanding, do you have anything else in mind that how you are
    trying to completely avoid REFRESH SEQUENCES.  Having said that I
    agree that in many cases where sequences are not frequently modified
    it is very much possible that when we are in switchover phase all
    sequences are already in sync and if we can identify that then we can
    avoid REFRESH SEQUENCES, but my point is we can not completely avoid
    that in all caes.  And the cases where we are frequently inserting
    into relation which has a sequence type field the sequences will be
    freqnectly modified and we have to call REFRESH SEQUENCES.
    
    -- 
    Regards,
    Dilip Kumar
    Google
    
    
    
    
  48. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-25T03:02:14Z

    On Tue, Feb 24, 2026 at 6:49 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
    >
    > On Tue, Feb 24, 2026 at 4:55 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > >
    > > On Tue, Feb 24, 2026 at 4:34 PM Ajin Cherian <itsajin@gmail.com> wrote:
    > > >
    > > > Currently REFRESH SEQUENCES can only be called if the subscription is
    > > > enabled. All it does is change the states of all the sequences in
    > > > subscription_rel to INIT, this will prompt the sequence worker to wake
    > > > up and unconditionally sync all the sequences. For sequences in the
    > > > INIT state, it doesn't check if there is drift or not, it updates all
    > > > sequences unconditionally. From your discussions with Dilip, I
    > > > understand we want to reduce the time it takes to REFRESH SEQUENCES at
    > > > the time of an upgrade. If so, then this might not be a good approach.
    > > >
    > >
    > > The point I was trying to make is that with automatic sequence sync,
    > > we won't need to execute REFRESH SEQUENCES before upgrade or failover
    > > as the sequences will be in sync.
    > >
    >
    > That is a valid goal, however, the sequence sync worker is an
    > asynchronous process triggered at specific intervals. For a switchover
    > to guarantee consistency, we must disconnect all connections from the
    > current publisher and wait for all pending data to sync. Relying on an
    > automated, interval-based worker at this critical time is risky, as it
    > directly extends our downtime. To minimize the cutover window, we
    > should either treat sequence data the same as relational data
    > (synchronous streaming) or manually trigger for the sequence sync
    > during the switchover phase, i.e. REFRESH SEQUENCES.  Anyway this is
    > my understanding, do you have anything else in mind that how you are
    > trying to completely avoid REFRESH SEQUENCES.  Having said that I
    > agree that in many cases where sequences are not frequently modified
    > it is very much possible that when we are in switchover phase all
    > sequences are already in sync and if we can identify that then we can
    > avoid REFRESH SEQUENCES, but my point is we can not completely avoid
    > that in all caes.
    >
    
    Right, I am also of the point that we can't completely avoid REFRESH
    SEQUENCES in all cases. Having sequence sync worker syncing
    periodically reduces the chances that one need to perform REFRESH
    SEQUENCES, so we need both. Do we agree with that?
    
    Additionally, I am thinking that after sync-worker we change REFRESH
    SEQUENCES functionality such that it will refresh all sequences during
    command. And, even we fail to copy one sequence, the command should
    fail. This should use almost the same functionality (like we sync only
    the required ones) as syncworker but online. This is because the case
    where it would be used will be less likely, say when due to some
    reason like max_logical_workers limit, we are not able to invoke
    sequencesync worker or just before upgrade (when sequences are not
    already synced), so doing sync during refresh command sounds
    reasonable. What do you think?
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  49. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-02-25T03:57:06Z

    On Wed, Feb 25, 2026 at 8:32 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > On Tue, Feb 24, 2026 at 6:49 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
    > >
    > > On Tue, Feb 24, 2026 at 4:55 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > > >
    > > > On Tue, Feb 24, 2026 at 4:34 PM Ajin Cherian <itsajin@gmail.com> wrote:
    > > > >
    > > > > Currently REFRESH SEQUENCES can only be called if the subscription is
    > > > > enabled. All it does is change the states of all the sequences in
    > > > > subscription_rel to INIT, this will prompt the sequence worker to wake
    > > > > up and unconditionally sync all the sequences. For sequences in the
    > > > > INIT state, it doesn't check if there is drift or not, it updates all
    > > > > sequences unconditionally. From your discussions with Dilip, I
    > > > > understand we want to reduce the time it takes to REFRESH SEQUENCES at
    > > > > the time of an upgrade. If so, then this might not be a good approach.
    > > > >
    > > >
    > > > The point I was trying to make is that with automatic sequence sync,
    > > > we won't need to execute REFRESH SEQUENCES before upgrade or failover
    > > > as the sequences will be in sync.
    > > >
    > >
    > > That is a valid goal, however, the sequence sync worker is an
    > > asynchronous process triggered at specific intervals. For a switchover
    > > to guarantee consistency, we must disconnect all connections from the
    > > current publisher and wait for all pending data to sync. Relying on an
    > > automated, interval-based worker at this critical time is risky, as it
    > > directly extends our downtime. To minimize the cutover window, we
    > > should either treat sequence data the same as relational data
    > > (synchronous streaming) or manually trigger for the sequence sync
    > > during the switchover phase, i.e. REFRESH SEQUENCES.  Anyway this is
    > > my understanding, do you have anything else in mind that how you are
    > > trying to completely avoid REFRESH SEQUENCES.  Having said that I
    > > agree that in many cases where sequences are not frequently modified
    > > it is very much possible that when we are in switchover phase all
    > > sequences are already in sync and if we can identify that then we can
    > > avoid REFRESH SEQUENCES, but my point is we can not completely avoid
    > > that in all caes.
    > >
    >
    > Right, I am also of the point that we can't completely avoid REFRESH
    > SEQUENCES in all cases. Having sequence sync worker syncing
    > periodically reduces the chances that one need to perform REFRESH
    > SEQUENCES, so we need both. Do we agree with that?
    >
    > Additionally, I am thinking that after sync-worker we change REFRESH
    > SEQUENCES functionality such that it will refresh all sequences during
    > command. And, even we fail to copy one sequence, the command should
    > fail. This should use almost the same functionality (like we sync only
    > the required ones) as syncworker but online. This is because the case
    > where it would be used will be less likely, say when due to some
    > reason like max_logical_workers limit, we are not able to invoke
    > sequencesync worker or just before upgrade (when sequences are not
    > already synced), so doing sync during refresh command sounds
    > reasonable. What do you think?
    >
    
    I agree with the overall direction of the discussion. But this is how
    I look at it:
    
    Given that sequences can be large, it’s not always practical and may
    place an unnecessary burden on users to manually verify whether all
    sequences are in sync before an upgrade. Requiring users to inspect
    sequence values and manually determine drift before deciding whether
    to run REFRESH introduces unnecessary operational complexity. So
    instead of trying to find ways to avoid REFRESH, we can position it as
    the final safety step before upgrade, optimized to refresh only
    drifted sequences and to fail if synchronization of any required
    sequence does not succeed. This approach gives us several advantages:
    
    --Users don’t need to manually check for drift.
    --REFRESH becomes a safe, final synchronization step before upgrade.
    --In most real-world scenarios, sequences are likely already in sync
    due to the periodic sync worker. When everything is already
    synchronized, REFRESH effectively becomes a lightweight confirmation
    check.
    --If some sequences are out of sync (due to high activity, worker
    limits such as max_logical_replication_workers, or other edge cases),
    REFRESH will correct only those.
    
    This creates a practical win-win situation: fast execution when
    sequences are already in sync, guaranteed correctness when they are
    not, and no additional manual validation burden on the user. Thoughts?
    
    thanks
    Shveta
    
    
    
    
  50. Re: [PATCH] Support automatic sequence replication

    Ajin Cherian <itsajin@gmail.com> — 2026-02-26T07:37:08Z

    On Tue, Feb 24, 2026 at 5:17 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >
    > Can we find some cheap
    > way to detect if sequencesync worker is present or not? Can you think
    > some other way to not incur the cost of traversing the worker array
    > and also detect sequence worker exit without much delay?
    >
    
    Added this.
    
    > Also, shouldn't we need to invoke AcceptInvalidationMessages() as we
    > are doing in apply worker when not in a remote transaction? I think it
    > will be required to get local_sequence definition changes , if any.
    
    Changed.
    
    Thanks Hou-san for helping me with these changes.
    I also did some performance testing on HEAD to see how long REFRESH
    SEQUENCES takes for a large number of sequences.
    I ran these on a 2× Intel Xeon E5-2699 v4 (22 cores each, 44 cores
    total / 88 threads) 512 GB RAM. I didn't see much value in
    differentiating between cases where half the sequences were different
    or all the sequences were different as REFRESH SEQUENCES updates all
    sequences after changing the state of all of them to INIT, it doesn't
    matter if they drifted or not.
    
    On HEAD:
    time to sync 10000 sequences: 1.080s (1080ms)
    time to sync 100000 sequences: 12.069s (12069ms)
    time to sync 1000000 sequences: 139.414s (139414ms)
    
    testing script attached (pass in the number of sequences as a run time
    parameter).
    
    regards,
    Ajin Cherian
    Fujitsu Australia
    
  51. Re: [PATCH] Support automatic sequence replication

    Nisha Moond <nisha.moond412@gmail.com> — 2026-02-26T14:13:12Z

    On Thu, Feb 26, 2026 at 1:07 PM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    > On Tue, Feb 24, 2026 at 5:17 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > >
    > > Can we find some cheap
    > > way to detect if sequencesync worker is present or not? Can you think
    > > some other way to not incur the cost of traversing the worker array
    > > and also detect sequence worker exit without much delay?
    > >
    >
    > Added this.
    >
    > > Also, shouldn't we need to invoke AcceptInvalidationMessages() as we
    > > are doing in apply worker when not in a remote transaction? I think it
    > > will be required to get local_sequence definition changes , if any.
    >
    > Changed.
    >
    > Thanks Hou-san for helping me with these changes.
    > I also did some performance testing on HEAD to see how long REFRESH
    > SEQUENCES takes for a large number of sequences.
    > I ran these on a 2× Intel Xeon E5-2699 v4 (22 cores each, 44 cores
    > total / 88 threads) 512 GB RAM. I didn't see much value in
    > differentiating between cases where half the sequences were different
    > or all the sequences were different as REFRESH SEQUENCES updates all
    > sequences after changing the state of all of them to INIT, it doesn't
    > matter if they drifted or not.
    >
    > On HEAD:
    > time to sync 10000 sequences: 1.080s (1080ms)
    > time to sync 100000 sequences: 12.069s (12069ms)
    > time to sync 1000000 sequences: 139.414s (139414ms)
    >
    > testing script attached (pass in the number of sequences as a run time
    > parameter).
    
    Hi Ajin,
    Thanks for sharing the performance results. I ran the same tests using
    your scripts on a different machine with the configuration:
     - Chip: Apple M4 Pro, 14 CPU cores
     - RAM: 24 GB
     - Postgres installation on pg_Head - commit 77c7a17a6e5
    
    For these tests, I used shared_buffers = 4GB. The time taken for 1M
    sequences is increased significantly:
      time to sync 10000 sequences: .994s (994ms)
      time to sync 100000 sequences: 11.032s (11032ms)
      time to sync 1000000 sequences: 426.850s (426850ms)
    
    I also tested with shared_buffers = 8GB, and the time for 1M sequences
    was 441.794s (441794 ms)
    
    --
    Thanks,
    Nisha
    
    
    
    
  52. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-27T06:01:14Z

    On Thu, Feb 26, 2026 at 7:43 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
    >
    > On Thu, Feb 26, 2026 at 1:07 PM Ajin Cherian <itsajin@gmail.com> wrote:
    > >
    > > I also did some performance testing on HEAD to see how long REFRESH
    > > SEQUENCES takes for a large number of sequences.
    > > I ran these on a 2× Intel Xeon E5-2699 v4 (22 cores each, 44 cores
    > > total / 88 threads) 512 GB RAM. I didn't see much value in
    > > differentiating between cases where half the sequences were different
    > > or all the sequences were different as REFRESH SEQUENCES updates all
    > > sequences after changing the state of all of them to INIT, it doesn't
    > > matter if they drifted or not.
    > >
    > > On HEAD:
    > > time to sync 10000 sequences: 1.080s (1080ms)
    > > time to sync 100000 sequences: 12.069s (12069ms)
    > > time to sync 1000000 sequences: 139.414s (139414ms)
    > >
    > > testing script attached (pass in the number of sequences as a run time
    > > parameter).
    >
    > Hi Ajin,
    > Thanks for sharing the performance results. I ran the same tests using
    > your scripts on a different machine with the configuration:
    >  - Chip: Apple M4 Pro, 14 CPU cores
    >  - RAM: 24 GB
    >  - Postgres installation on pg_Head - commit 77c7a17a6e5
    >
    > For these tests, I used shared_buffers = 4GB. The time taken for 1M
    > sequences is increased significantly:
    >   time to sync 10000 sequences: .994s (994ms)
    >   time to sync 100000 sequences: 11.032s (11032ms)
    >   time to sync 1000000 sequences: 426.850s (426850ms)
    >
    > I also tested with shared_buffers = 8GB, and the time for 1M sequences
    > was 441.794s (441794 ms)
    >
    
    IIUC, the tests by Ajin and Nisha show the time to sync sequences
    varies from a few seconds to minutes depending on the number of
    sequences and machine configuration. This indicates that having an
    autosync worker can avoid downtime before upgrade, otherwise, users
    always need to use REFRESH SEQUENCES before upgrade.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  53. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-02-27T11:07:28Z

    On Thu, Feb 26, 2026 at 1:07 PM Ajin Cherian <itsajin@gmail.com> wrote:
    >
    
    Few comments:
    =============
    1.
    + oldctx = MemoryContextSwitchTo(SequenceSyncContext);
    
    - initStringInfo(&app_name);
    - appendStringInfo(&app_name, "pg_%u_sequence_sync_" UINT64_FORMAT,
    - MySubscription->oid, GetSystemIdentifier());
    + /* Process sequences */
    + sequence_copied = copy_sequences(conn, seqinfos);
    
    - /*
    - * Establish the connection to the publisher for sequence synchronization.
    - */
    - LogRepWorkerWalRcvConn =
    - walrcv_connect(MySubscription->conninfo, true, true,
    -    must_use_password,
    -    app_name.data, &err);
    - if (LogRepWorkerWalRcvConn == NULL)
    - ereport(ERROR,
    - errcode(ERRCODE_CONNECTION_FAILURE),
    - errmsg("sequencesync worker for subscription \"%s\" could not
    connect to the publisher: %s",
    -    MySubscription->name, err));
    -
    - pfree(app_name.data);
    -
    - copy_sequences(LogRepWorkerWalRcvConn);
    + MemoryContextSwitchTo(oldctx);
    
    It is better to switch to SequenceSyncContext at the caller of
    LogicalRepSyncSequences similar to what we are doing for
    ApplyMessageContext.
    
    2.
    @@ -4221,6 +4221,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
      ProcessConfigFile(PGC_SIGHUP);
      }
    
    +
      if (rc & WL_TIMEOUT)
    
    Spurious line addition.
    
    3. Apart from above, the attached patch contains comments and cosmetic changes.
    
    -- 
    With Regards,
    Amit Kapila.
    
  54. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-02-28T08:40:31Z

    On Friday, February 27, 2026 7:07 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > Few comments:
    > =============
    > 1.
    > + oldctx = MemoryContextSwitchTo(SequenceSyncContext);
    > 
    > - initStringInfo(&app_name);
    > - appendStringInfo(&app_name, "pg_%u_sequence_sync_" UINT64_FORMAT,
    > - MySubscription->oid, GetSystemIdentifier());
    > + /* Process sequences */
    > + sequence_copied = copy_sequences(conn, seqinfos);
    > 
    > - /*
    > - * Establish the connection to the publisher for sequence synchronization.
    > - */
    > - LogRepWorkerWalRcvConn =
    > - walrcv_connect(MySubscription->conninfo, true, true,
    > -    must_use_password,
    > -    app_name.data, &err);
    > - if (LogRepWorkerWalRcvConn == NULL)
    > - ereport(ERROR,
    > - errcode(ERRCODE_CONNECTION_FAILURE),
    > - errmsg("sequencesync worker for subscription \"%s\" could not connect to
    > the publisher: %s",
    > -    MySubscription->name, err));
    > -
    > - pfree(app_name.data);
    > -
    > - copy_sequences(LogRepWorkerWalRcvConn);
    > + MemoryContextSwitchTo(oldctx);
    > 
    > It is better to switch to SequenceSyncContext at the caller of
    > LogicalRepSyncSequences similar to what we are doing for
    > ApplyMessageContext.
    
    Agreed. I changed as suggested.
    
    > 
    > 2.
    > @@ -4221,6 +4221,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
    >   ProcessConfigFile(PGC_SIGHUP);
    >   }
    > 
    > +
    >   if (rc & WL_TIMEOUT)
    > 
    > Spurious line addition.
    > 
    > 3. Apart from above, the attached patch contains comments and cosmetic
    > changes.
    
    Thanks for the patch, I have merged them into 0001.
    
    Here is the V8 patch set addressing the previous review comments:
    
    - For 0001, I noticed that the GetSequence() function added in the patch
      fetches the local sequence value without any privilege check. This
      allows the worker to read sequence data even without proper SELECT
      privilege, which seems unsafe. I've added a SELECT privilege check
      before fetching the sequence value. Additionally, I've updated several
      comments, made cosmetic changes, commit message update, and run
      pgindent on all patches.
    
    - 0002 includes the changes to synchronize sequences directly in the
      REFRESH SEQUENCES command
    
    Best Regards,
    Hou zj
    
  55. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-02T07:58:54Z

    On Saturday, February 28, 2026 4:41 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote:
    > Here is the V8 patch set addressing the previous review comments:
    > 
    > - For 0001, I noticed that the GetSequence() function added in the patch
    >   fetches the local sequence value without any privilege check. This
    >   allows the worker to read sequence data even without proper SELECT
    >   privilege, which seems unsafe. I've added a SELECT privilege check
    >   before fetching the sequence value. Additionally, I've updated several
    >   comments, made cosmetic changes, commit message update, and run
    >   pgindent on all patches.
    > 
    > - 0002 includes the changes to synchronize sequences directly in the
    >   REFRESH SEQUENCES command
    
    Rebased the patch to silence compile warning due to a recent commit
    a2c89835.
    
    Best Regards,
    Hou zj
    
  56. RE: [PATCH] Support automatic sequence replication

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-03-04T04:20:47Z

    Dear Hou,
    
    Thanks for updating the patch. Here are my comments.
    
    01.
    ```
       <para>
        A <firstterm>sequence synchronization worker</firstterm> will be started
    -   after executing any of the above subscriber commands, and will exit once the
    -   sequences are synchronized.
    +   after executing any of the above subscriber commands. The worker will
    +   remain running for the life of the subscription, periodically
    +   synchronizing all published sequences.
       </para>
    ```
    
    I think it's not accurate, because REFRESH SEQUENCE command does not need the
    sequencesync worker anymore.
    
    02.
    ```
    void
    GetSequence(Relation seqrel, int64 *last_value, bool *is_called)
    ```
    
    I think GetSequence() itself should conitan the permission check like SetSequence().
    My idea is to set NULL for last_value and is_called in this case.
    
    03.
    ```
    		/*
    		 * Verify that the current user has SELECT privilege on the sequence.
    		 * This is required to read the sequence state below.
    		 */
    		aclresult = pg_class_aclcheck(seqoid, GetUserId(), ACL_SELECT);
    
    		if (aclresult != ACLCHECK_OK)
    			return COPYSEQ_INSUFFICIENT_PERM;
    
    		/* Get current local sequence state */
    		GetSequence(sequence_rel, &local_last_value, &local_is_called);
    ```
    
    If you accept above comment, this part can be simplified.
    
    04.
    ```
    /*
     * get_and_validate_seq_info
     *
     * Extracts remote sequence information from the tuple slot received from the
     * publisher, and validates it against the corresponding local sequence
     * definition.
     */
    static CopySeqResult
    get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel,
    						  LogicalRepSequenceInfo **seqinfo, int *seqidx,
    						  List *seqinfos)
    ```
    
    It can return COPYSEQ_SUCCESS, but it might be misleading; copying is not
    happened yet here. How about returning boolean and add another argument to
    indicate the reason if the validation is failed?
    
    05.
    
    LogicalRepSyncSequences() starts the transaction and read sequences every time.
    Can we cache the seqinfos to reuse in the next iteration? My idea is to introduce
    a syscache callback for the pg_subscription_relto invalidate the cached list.
    
    How about measuring performance once and considering it's a good improvement?
    
    Best regards,
    Hayato Kuroda
    FUJITSU LIMITED
    
    
  57. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-03-04T10:24:35Z

    On Mon, Mar 2, 2026 at 1:28 PM Zhijie Hou (Fujitsu)
    <houzj.fnst@fujitsu.com> wrote:
    >
    > Rebased the patch to silence compile warning due to a recent commit
    > a2c89835.
    >
    
    Thanks for the patch. Please find a few comments for 001:
    
    1)
      /*
      * Record the remote sequence's LSN in pg_subscription_rel and mark the
    - * sequence as READY.
    + * sequence as READY if updating a sequence that is in INIT state.
      */
    - UpdateSubscriptionRelState(MySubscription->oid, seqoid, SUBREL_STATE_READY,
    -    seqinfo->page_lsn, false);
    + if (seqinfo->relstate == SUBREL_STATE_INIT)
    + UpdateSubscriptionRelState(MySubscription->oid, seqoid, SUBREL_STATE_READY,
    +    seqinfo->page_lsn, false);
    
    What if page-lsn has changed and we are in READY state, don't we need
    to update that in pg_subscription_rel? Or is that done somewhere else?
    
    2)
    + *
    + * If relstate is SUBREL_STATE_READY, only synchronize sequences that
    + * have drifted from their publisher values. Otherwise, synchronize
    + * all sequences.
    + *
    + * Returns true/false if any sequences were actually copied.
      */
    +static bool
    +copy_sequences(WalReceiverConn *conn, List *seqinfos)
    
    There is no relstate, comments need correction.
    
    3)
    Currently we use same state 'COPYSEQ_SUCCESS' for 2 cases: allowed to
    copy (as returned by validate_seqsync_state and
    get_and_validate_seq_info) and copy-done (by copy_sequences,
    copy_sequence). It is slightly confusing. Shall we add one more state
    for 'allowed' case, could be COPYSEQ_ELIGIBLE or COPYSEQ_PROCEED or
    COPYSEQ_ALLOWED? COPYSEQ_SUCCESS was used for such a case in previous
    seq-sync commands too (on HEAD), but now its usage is more in
    'allowed' case as compared to HEAD, so perhaps we can change in this
    patch. But I would like to know what others think here.
    
    
    4)
    + * Preliminary check to determine if copying the sequence is allowed.
    
    How about this comment:
    Check whether the user has required privileges on the sequence and
    whether the sequence has drifted.
    
    5)
    validate_seqsync_state():
    + /*
    + * Skip synchronization if the sequence is already in READY state and
    + * has not drifted from the publisher's value.
    + */
    + if (local_last_value == seqinfo->last_value &&
    + local_is_called == seqinfo->is_called)
    + return COPYSEQ_NO_DRIFT;
    
    Since we already have a comment where we check READY state in outer
    if-block and since we are not checking READY state here, perhaps we
    can change the above comment to simply:
     "Skip synchronization if it has not drifted from the publisher's value."
    
    thanks
    Shveta
    
    
    
    
  58. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-05T02:45:45Z

    On Wednesday, March 4, 2026 12:21 PM Kuroda, Hayato/黒田 隼人 <kuroda.hayato@fujitsu.com> wrote:
    > 
    > Thanks for updating the patch. Here are my comments.
    
    Thanks for the comments.
    
    > 
    > 01.
    > ```
    >    <para>
    >     A <firstterm>sequence synchronization worker</firstterm> will be started
    > -   after executing any of the above subscriber commands, and will exit once
    > the
    > -   sequences are synchronized.
    > +   after executing any of the above subscriber commands. The worker will
    > +   remain running for the life of the subscription, periodically
    > +   synchronizing all published sequences.
    >    </para>
    > ```
    > 
    > I think it's not accurate, because REFRESH SEQUENCE command does not
    > need the sequencesync worker anymore.
    
    Since the command is changed in 0002, I updated the doc there.
    
    > 
    > 02.
    > ```
    > void
    > GetSequence(Relation seqrel, int64 *last_value, bool *is_called) ```
    > 
    > I think GetSequence() itself should conitan the permission check like
    > SetSequence().
    > My idea is to set NULL for last_value and is_called in this case.
    
    Changed.
    
    > 
    > 03.
    > ```
    > 		/*
    > 		 * Verify that the current user has SELECT privilege on the
    > sequence.
    > 		 * This is required to read the sequence state below.
    > 		 */
    > 		aclresult = pg_class_aclcheck(seqoid, GetUserId(),
    > ACL_SELECT);
    > 
    > 		if (aclresult != ACLCHECK_OK)
    > 			return COPYSEQ_INSUFFICIENT_PERM;
    > 
    > 		/* Get current local sequence state */
    > 		GetSequence(sequence_rel, &local_last_value,
    > &local_is_called); ```
    > 
    > If you accept above comment, this part can be simplified.
    
    Changed.
    
    > 
    > 04.
    > ```
    > /*
    >  * get_and_validate_seq_info
    >  *
    >  * Extracts remote sequence information from the tuple slot received from the
    >  * publisher, and validates it against the corresponding local sequence
    >  * definition.
    >  */
    > static CopySeqResult
    > get_and_validate_seq_info(TupleTableSlot *slot, Relation *sequence_rel,
    > 						  LogicalRepSequenceInfo
    > **seqinfo, int *seqidx,
    > 						  List *seqinfos)
    > ```
    > 
    > It can return COPYSEQ_SUCCESS, but it might be misleading; copying is not
    > happened yet here. How about returning boolean and add another argument
    > to indicate the reason if the validation is failed?
    
    I have added one more enum value COPYSEQ_ALLOWED for cases where
    the validation passes to avoid changing the function signature.
    
    > 
    > 05.
    > 
    > LogicalRepSyncSequences() starts the transaction and read sequences every
    > time.
    > Can we cache the seqinfos to reuse in the next iteration? My idea is to
    > introduce a syscache callback for the pg_subscription_relto invalidate the
    > cached list.
    > 
    > How about measuring performance once and considering it's a good
    > improvement?
    
    I think the time of scanning sequence might be negligible when most of sequence
    needs to be updated, since the most cost would be on sequence update. There
    might be some value to save the CPU cycle when the most of sequence
    has not drifted in which case the cost of scanning might be noticeable. But we
    will do some more tests and share later.
    
    Best Regards,
    Hou zj
    
  59. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-05T02:46:00Z

    On Wednesday, March 4, 2026 6:25 PM shveta malik <shveta.malik@gmail.com> wrote:
    > 
    > On Mon, Mar 2, 2026 at 1:28 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
    > wrote:
    > >
    > > Rebased the patch to silence compile warning due to a recent commit
    > > a2c89835.
    > >
    > 
    > Thanks for the patch. Please find a few comments for 001:
    
    Thanks for the comments.
    
    > 
    > 1)
    >   /*
    >   * Record the remote sequence's LSN in pg_subscription_rel and mark the
    > - * sequence as READY.
    > + * sequence as READY if updating a sequence that is in INIT state.
    >   */
    > - UpdateSubscriptionRelState(MySubscription->oid, seqoid,
    > SUBREL_STATE_READY,
    > -    seqinfo->page_lsn, false);
    > + if (seqinfo->relstate == SUBREL_STATE_INIT)
    > + UpdateSubscriptionRelState(MySubscription->oid, seqoid,
    > SUBREL_STATE_READY,
    > +    seqinfo->page_lsn, false);
    > 
    > What if page-lsn has changed and we are in READY state, don't we need to
    > update that in pg_subscription_rel? Or is that done somewhere else?
    
    The check was done in 0002, I moved it to 0001 now.
    
    > 
    > 2)
    > + *
    > + * If relstate is SUBREL_STATE_READY, only synchronize sequences that
    > + * have drifted from their publisher values. Otherwise, synchronize
    > + * all sequences.
    > + *
    > + * Returns true/false if any sequences were actually copied.
    >   */
    > +static bool
    > +copy_sequences(WalReceiverConn *conn, List *seqinfos)
    > 
    > There is no relstate, comments need correction.
    
    Changed.
    
    > 
    > 3)
    > Currently we use same state 'COPYSEQ_SUCCESS' for 2 cases: allowed to
    > copy (as returned by validate_seqsync_state and
    > get_and_validate_seq_info) and copy-done (by copy_sequences,
    > copy_sequence). It is slightly confusing. Shall we add one more state for
    > 'allowed' case, could be COPYSEQ_ELIGIBLE or COPYSEQ_PROCEED or
    > COPYSEQ_ALLOWED? COPYSEQ_SUCCESS was used for such a case in
    > previous seq-sync commands too (on HEAD), but now its usage is more in
    > 'allowed' case as compared to HEAD, so perhaps we can change in this patch.
    > But I would like to know what others think here.
    
    I agree that adding a new value can make the code easier to read.
    
    > 
    > 
    > 4)
    > + * Preliminary check to determine if copying the sequence is allowed.
    > 
    > How about this comment:
    > Check whether the user has required privileges on the sequence and whether
    > the sequence has drifted.
    
    Changed as suggested.
    
    > 
    > 5)
    > validate_seqsync_state():
    > + /*
    > + * Skip synchronization if the sequence is already in READY state and
    > + * has not drifted from the publisher's value.
    > + */
    > + if (local_last_value == seqinfo->last_value && local_is_called ==
    > + seqinfo->is_called) return COPYSEQ_NO_DRIFT;
    > 
    > Since we already have a comment where we check READY state in outer if-
    > block and since we are not checking READY state here, perhaps we can
    > change the above comment to simply:
    >  "Skip synchronization if it has not drifted from the publisher's value."
    
    Changed.
    
    Here is V10 patch set which addressed all comments.
    
    I also fixed a bug that Kuroda-San reported off-list, where we were not
    resetting the StringInfo used to build the query in the copy_sequences loop,
    which caused the built query to be incorrect.
    
    Best Regards,
    Hou zj
    
  60. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-03-05T04:05:21Z

    On Thu, Mar 5, 2026 at 8:16 AM Zhijie Hou (Fujitsu)
    <houzj.fnst@fujitsu.com> wrote:
    >
    >
    > Here is V10 patch set which addressed all comments.
    >
    
    Thank You. Please find a few comments on 001:
    
    
    1)
    + /*
    + * Skip synchronization if the current user does not have sufficient
    + * privileges to read the sequence data.
    + */
    + if (local_last_value == 0)
    + return COPYSEQ_INSUFFICIENT_PERM;
    
    I don't think it is the right way to handle this. The local_last_value
    can be genuinely 0 for some cases and we may end up giving the wrong
    ERROR.
    
    Try this:
    CREATE SEQUENCE my_seq START WITH 0 INCREMENT BY 1 MINVALUE 0;
    And then set-up pub-sub.
    
    We get:
    2026-03-05 08:57:39.591 IST [92281] WARNING:  insufficient privileges
    on sequence ("public.my_seq")
    2026-03-05 08:57:39.591 IST [92281] ERROR:  logical replication
    sequence synchronization failed for subscription "subi1"
    
    Either we shall move back the acl check to the caller of GetSequence
    or pass the info of acl-check failure in  a new argument or return
    value.
    
    2)
    + /*
    + * For sequences in INIT state, always sync. Otherwise, for
    + * sequences in READY state, only sync if there's drift.
    + */
      if (sync_status == COPYSEQ_SUCCESS)
    - sync_status = copy_sequence(seqinfo,
    - sequence_rel->rd_rel->relowner);
    + sync_status = copy_sequence(seqinfo, sequence_rel);
    
    We shall add such a comment atop copy_sequence as well. I am unsure if
    we need it here or not.
    
    3)
    + /* Sleep for the configured interval */
    + (void) WaitLatch(MyLatch,
    + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
    + sleep_ms,
    + WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE);
    
    I don't think this wait-event is appropriate. Unlike tablesync, we are
    not waiting for any state change here. Shall we add a new one for our
    case? How about WAIT_EVENT_LOGICAL_SEQSYNC_MAIN? Thoughts?
    
    4)
    + relstate = subrel->srsubstate;
    
    it will be good to move it just after below part:
    
    /* Skip if the relation is not a sequence */
    
    
    5)
    
      }
    + /* Check if there are any sequences. */
    + has_subsequences = (seq_states != NIL);
    
    One blank line before new change will improve readability.
    
    6)
    
     ##########
     ## ALTER SUBSCRIPTION ... REFRESH PUBLICATION should cause sync of new
    -# sequences of the publisher, but changes to existing sequences should
    -# not be synced.
    +# sequences of the publisher.
     ##########
    
     # Create a new sequence 'regress_s2', and update existing sequence 'regress_s1'
    
    Last comment needs to be changed. Remove this please:  'and update
    existing sequence 'regress_s1''
    
    thanks
    Shveta
    
    
    
    
  61. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-03-05T05:48:20Z

    On Thu, Mar 5, 2026 at 9:35 AM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > On Thu, Mar 5, 2026 at 8:16 AM Zhijie Hou (Fujitsu)
    > <houzj.fnst@fujitsu.com> wrote:
    > >
    > >
    > > Here is V10 patch set which addressed all comments.
    > >
    >
    > Thank You. Please find a few comments on 001:
    >
    
    A concern in 002:
    
    I realized that below might not be the correct logic to avoid
    overwriting sequences at sub which are already at latest values.
    
    + /*
    + * Skip synchronization if the local sequence value is already ahead of
    + * the publisher's value.
    ...
    + */
    + if (local_last_value > seqinfo->last_value)
    + {
    + ereport(WARNING,
    + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    + errmsg("skipped synchronizing the sequence \"%s.%s\"",
    +    seqinfo->nspname, seqinfo->seqname),
    + errdetail("The local last_value %lld is ahead of the one on publisher",
    +   (long long int) local_last_value));
    +
    + return COPYSEQ_NO_DRIFT;
    + }
    
    
    A sequence could be descending one too and thus we may wrongly end up
    avoiding synchronization. We should first check if it is descending or
    ascending (perhaps by checking if increment_by < 0 or >0), then decide
    to manage conflict.
    
    Example:
    postgres=# CREATE SEQUENCE desc_seq START WITH 1000 INCREMENT BY -1
    MINVALUE 1 MAXVALUE 1000;
    CREATE SEQUENCE
    postgres=# select nextval('desc_seq');
    nextval
    ---------
        1000
    
    postgres=# select nextval('desc_seq');
    nextval
    ---------
         999
    
    Doc also mentions descending sequences. See [1] (search for descending).
    
    [1]: https://www.postgresql.org/docs/18/sql-createsequence.html
    
    thanks
    Shveta
    
    
    
    
  62. Re: [PATCH] Support automatic sequence replication

    Amit Kapila <amit.kapila16@gmail.com> — 2026-03-05T10:46:50Z

    On Thu, Mar 5, 2026 at 9:35 AM shveta malik <shveta.malik@gmail.com> wrote:
    >
    >
    > 3)
    > + /* Sleep for the configured interval */
    > + (void) WaitLatch(MyLatch,
    > + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
    > + sleep_ms,
    > + WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE);
    >
    > I don't think this wait-event is appropriate. Unlike tablesync, we are
    > not waiting for any state change here. Shall we add a new one for our
    > case? How about WAIT_EVENT_LOGICAL_SEQSYNC_MAIN? Thoughts?
    >
    
    +1 for a new wait event. Few other minor comments:
    
    1.
    + * Check if the subscription includes sequences and start a sequencesync
    + * worker if one is not already running. The active sequencesync worker will
    + * handle all pending sequence synchronization. If any sequences remain
    + * unsynchronized after it exits, a new worker can be started in the next
    + * iteration.
      *
    - * Start a sequencesync worker if one is not already running. The active
    - * sequencesync worker will handle all pending sequence synchronization. If any
    - * sequences remain unsynchronized after it exits, a new worker can be started
    - * in the next iteration.
    
    Why did this comment change? The earlier one sounds okay to me.
    
    2.
      break;
    +
      case COPYSEQ_INSUFFICIENT_PERM:
    
    Why does this patch add additional new lines? We use both styles
    (existing and what this patch does) in the code but it seems
    unnecessary to change for this patch.
    
    3.
    - ProcessSequencesForSync();
    +
    + /* Check if sequence worker needs to be started */
    + MaybeLaunchSequenceSyncWorker();
    
    No need for an additional line and a comment here.
    
    -- 
    With Regards,
    Amit Kapila.
    
    
    
    
  63. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-05T11:34:51Z

    On Thursday, March 5, 2026 6:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    > On Thu, Mar 5, 2026 at 9:35 AM shveta malik <shveta.malik@gmail.com>
    > wrote:
    > >
    > >
    > > 3)
    > > + /* Sleep for the configured interval */
    > > + (void) WaitLatch(MyLatch,
    > > + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, sleep_ms,
    > > + WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE);
    > >
    > > I don't think this wait-event is appropriate. Unlike tablesync, we are
    > > not waiting for any state change here. Shall we add a new one for our
    > > case? How about WAIT_EVENT_LOGICAL_SEQSYNC_MAIN? Thoughts?
    > >
    > 
    > +1 for a new wait event. Few other minor comments:
    
    Added.
    
    > 
    > 1.
    > + * Check if the subscription includes sequences and start a
    > + sequencesync
    > + * worker if one is not already running. The active sequencesync worker
    > + will
    > + * handle all pending sequence synchronization. If any sequences remain
    > + * unsynchronized after it exits, a new worker can be started in the
    > + next
    > + * iteration.
    >   *
    > - * Start a sequencesync worker if one is not already running. The active
    > - * sequencesync worker will handle all pending sequence synchronization. If
    > any
    > - * sequences remain unsynchronized after it exits, a new worker can be
    > started
    > - * in the next iteration.
    > 
    > Why did this comment change? The earlier one sounds okay to me.
    
    I think either version is fine, so reverted this change now.
    
    > 
    > 2.
    >   break;
    > +
    >   case COPYSEQ_INSUFFICIENT_PERM:
    > 
    > Why does this patch add additional new lines? We use both styles (existing
    > and what this patch does) in the code but it seems unnecessary to change for
    > this patch.
    
    Removed.
    
    > 
    > 3.
    > - ProcessSequencesForSync();
    > +
    > + /* Check if sequence worker needs to be started */
    > + MaybeLaunchSequenceSyncWorker();
    > 
    > No need for an additional line and a comment here.
    
    Removed.
    
    Here is the V11 patch which addressed all above comments and [1][2].
    
    [1] https://www.postgresql.org/message-id/CAJpy0uAfu-VPqCknLLvJ%2BPUx_cyoR-b70xUNT6Pyv8N-odKizQ%40mail.gmail.com
    [2] https://www.postgresql.org/message-id/CAJpy0uBeAdz6-3P26Eryeq3TyjA-GiKY3z0hFMxzZD%3DAYGqQ3Q%40mail.gmail.com
    
    Best Regards,
    Hou zj
    
  64. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-05T11:34:53Z

    On Thursday, March 5, 2026 1:48 PM shveta malik <shveta.malik@gmail.com> wrote:
    > 
    > On Thu, Mar 5, 2026 at 9:35 AM shveta malik <shveta.malik@gmail.com>
    > wrote:
    > >
    > > On Thu, Mar 5, 2026 at 8:16 AM Zhijie Hou (Fujitsu)
    > > <houzj.fnst@fujitsu.com> wrote:
    > > >
    > > >
    > > > Here is V10 patch set which addressed all comments.
    > > >
    > >
    > > Thank You. Please find a few comments on 001:
    > >
    > 
    > A concern in 002:
    > 
    > I realized that below might not be the correct logic to avoid overwriting
    > sequences at sub which are already at latest values.
    > 
    > + /*
    > + * Skip synchronization if the local sequence value is already ahead of
    > + * the publisher's value.
    > ...
    > + */
    > + if (local_last_value > seqinfo->last_value) { ereport(WARNING,
    > + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    > + errmsg("skipped synchronizing the sequence \"%s.%s\"",
    > +    seqinfo->nspname, seqinfo->seqname), errdetail("The local
    > + last_value %lld is ahead of the one on publisher",
    > +   (long long int) local_last_value));
    > +
    > + return COPYSEQ_NO_DRIFT;
    > + }
    > 
    > 
    > A sequence could be descending one too and thus we may wrongly end up
    > avoiding synchronization. We should first check if it is descending or ascending
    > (perhaps by checking if increment_by < 0 or >0), then decide to manage
    > conflict.
    
    Thanks for catching this, I changed it to check the increment before reporting warning.
    
    Best Regards,
    Hou zj
    
  65. RE: [PATCH] Support automatic sequence replication

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-03-05T12:57:03Z

    Dear Hackers,
    
    > 05.
    > 
    > LogicalRepSyncSequences() starts the transaction and read sequences every
    > time.
    > Can we cache the seqinfos to reuse in the next iteration? My idea is to introduce
    > a syscache callback for the pg_subscription_relto invalidate the cached list.
    > 
    > How about measuring performance once and considering it's a good
    > improvement?
    
    I profiled the sequencesync worker when sequences were less actively updated on
    the publisher side. In the actively updated system, copying sequences used most
    of the CPU time; thus, we could not observe the effect.
    
    Abstract
    --------------
    Sequencesync worker spent 20-25% of the working time scanning pg_subscription_rel
    in the workload. It's not so large compared with the total CPU time; the worker
    can work once per 2 seconds or longer. We may able to consider the optimization
    if there are easy ways.
    
    Source
    -----------
    ea47447 + v8 patch set + attached fix patch.
    To simplify the analysis, I extracted the scan part into the function
    scan_subscription_relations. No configure options are set at build.
    
    Workload
    ---------------
    Two workloads were tested.
    
    A - profile with no sequence updates
    
    0. Defined 100 sequences on both nodes
    1. Built a pub-sub replication system.
    2. Attached the sequencesync worker as early as creating the subscription. 
    3. Waited 10 minutes.
    
    
    B - profile with 10% sequences updates
    
    0. Defined 100 sequences on both nodes
    1. Built a pub-sub replication system.
    2. Waited till the initial sync was done. On my env 100s was enough
    3. Attached the sequencesync worker
    4. Updated 10 sequences per second.
    5. Repeat step 4 for 10 minutes.
    
    Result
    ----------
    The attached profiles show the detailed results: noupdate.out corresponds to
    workload A, while 10percent_update.out is for workload B.
    
    In both cases, scan_subscription_relations spends more than 20% of their working
    time. Notable points are to open sequence relations with the AccessShareLock,
    committing the transaction, starting the catalog scan, etc.
    
    workload A:
    ```
                   |           --20.83%--scan_subscription_relations
                   |                     |          
                   |                     |--10.83%--try_table_open
                   |                     |          try_relation_open
    ```
    
    workload B:
    ```
                           |           --24.01%--scan_subscription_relations
                           |                     |          
                           |                     |--12.52%--try_table_open
                           |                     |          try_relation_open
    ```
    
    Consideration
    --------------
    Based on that, we may be able to cache seqinfos to avoid starting the
    transaction and opening the sequence. But we need to introduce a relcache
    callback to invalidate the specific entry of the list, not sure it's beneficial
    more than the complexity.
    
    Configuration
    ----------------------
    Each node had shared_buffer=1GB, and others had the default GUC values.
    
    Environment
    --------------------
    CPU: Intel(R) Xeon(R) Platinum 8358P CPU @ 2.60GHz, 4 cores, 1 thread per core
    Memory: 15GiB
    OS: AlmaLinux 9.7
    
    Best regards,
    Hayato Kuroda
    FUJITSU LIMITED
    
    
  66. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-03-06T05:25:02Z

    On Thu, Mar 5, 2026 at 5:04 PM Zhijie Hou (Fujitsu)
    <houzj.fnst@fujitsu.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.
    
    thanks
    Shveta
    
  67. Re: [PATCH] Support automatic sequence replication

    Chao Li <li.evan.chao@gmail.com> — 2026-03-06T08:52:28Z

    
    > On Mar 5, 2026, at 19:34, Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote:
    > 
    > On Thursday, March 5, 2026 1:48 PM shveta malik <shveta.malik@gmail.com> wrote:
    >> 
    >> On Thu, Mar 5, 2026 at 9:35 AM shveta malik <shveta.malik@gmail.com>
    >> wrote:
    >>> 
    >>> On Thu, Mar 5, 2026 at 8:16 AM Zhijie Hou (Fujitsu)
    >>> <houzj.fnst@fujitsu.com> wrote:
    >>>> 
    >>>> 
    >>>> Here is V10 patch set which addressed all comments.
    >>>> 
    >>> 
    >>> Thank You. Please find a few comments on 001:
    >>> 
    >> 
    >> A concern in 002:
    >> 
    >> I realized that below might not be the correct logic to avoid overwriting
    >> sequences at sub which are already at latest values.
    >> 
    >> + /*
    >> + * Skip synchronization if the local sequence value is already ahead of
    >> + * the publisher's value.
    >> ...
    >> + */
    >> + if (local_last_value > seqinfo->last_value) { ereport(WARNING,
    >> + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
    >> + errmsg("skipped synchronizing the sequence \"%s.%s\"",
    >> +    seqinfo->nspname, seqinfo->seqname), errdetail("The local
    >> + last_value %lld is ahead of the one on publisher",
    >> +   (long long int) local_last_value));
    >> +
    >> + return COPYSEQ_NO_DRIFT;
    >> + }
    >> 
    >> 
    >> A sequence could be descending one too and thus we may wrongly end up
    >> avoiding synchronization. We should first check if it is descending or ascending
    >> (perhaps by checking if increment_by < 0 or >0), then decide to manage
    >> conflict.
    > 
    > Thanks for catching this, I changed it to check the increment before reporting warning.
    > 
    > Best Regards,
    > Hou zj
    
    Hi,
    
    I just started reviewing this patch and wanted to first discuss the design.
    
    The current approach introduces a long-lived sync worker for any subscription that has at least one sequence. I noticed a previous email suggesting that this approach is “acceptable”, but it still seems like a big runtime cost.
    
    What I had in mind instead is whether we could extend the WAL decoding protocol to send RM_SEQ_ID over the logical replication stream, so that sequence synchronization becomes part of logical replication itself. That would make it essentially event-driven and close to zero cost at runtime, rather than relying on periodic polling.
    
    There is also one case I haven’t seen discussed yet. Suppose the standby side inserts a tuple into a table that is under logical replication. This might not immediately cause a tuple-level replication conflict, but it could advance the sequence locally. In that case, the standby sequence could diverge from the primary sequence and remain out of sync indefinitely. How should that situation be handled?
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  68. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-06T10:27:59Z

    On Friday, March 6, 2026 4:52 PM Chao Li <li.evan.chao@gmail.com> wrote:
    > 
    > I just started reviewing this patch and wanted to first discuss the design.
    > 
    
    Thanks for reviewing.
    
    > The current approach introduces a long-lived sync worker for any subscription
    > that has at least one sequence. I noticed a previous email suggesting that
    > this approach is “acceptable”, but it still seems like a big runtime cost.
    
    Currently in the patch, the sequence sync worker operates at a dynamic interval
    (between 2 and 30 seconds), adjusting based on how frequently the sequence is
    updated, and only synchronizes when the sequence has actually diverged from the
    publisher. So while it's a long-lived worker, I think its runtime impact is
    acceptable.
    
    > What I had in mind instead is whether we could extend the WAL decoding
    > protocol to send RM_SEQ_ID over the logical replication stream, so that
    > sequence synchronization becomes part of logical replication itself.
    
    That approach was explored in depth during earlier discussions, but it was
    ultimately set aside due to the complexity and correctness challenges it would
    introduce into logical decoding. Adding sequence information to the replication
    stream would require significant changes to the decoding machinery, and given
    that the primary use case is to support upgrades, the trade-off didn't seem
    justified. You can find a detailed breakdown of the design considerations and
    the reasoning behind the decision in [1].
    
    > That would make it essentially event-driven and close to zero cost at runtime,
    > rather than relying on periodic polling.
    
    I don't think so. Even with an event-driven model, there's still a cost,
    decoding sequence changes would add overhead to walsenders on the publisher that
    needs to replicate sequences. So it's just a different distribution of the
    overhead.
    
    > There is also one case I haven’t seen discussed yet. Suppose the standby side
    > inserts a tuple into a table that is under logical replication. This might not
    > immediately cause a tuple-level replication conflict, but it could advance the
    > sequence locally. In that case, the standby sequence could diverge from the
    > primary sequence and remain out of sync indefinitely. How should that
    > situation be handled?
    
    This scenario was also considered in earlier discussions, see [2]. The
    divergence issue you mentioned is not introduced by the sequence sync worker; It
    is considered as a logical conflict and resolving it would require user
    intervention or a future enhancement to handle such conflicts automatically.
    
    [1] https://www.postgresql.org/message-id/CAA4eK1LC%2BKJiAkSrpE_NwvNdidw9F2os7GERUeSxSKv71gXysQ%40mail.gmail.com
    [2] https://www.postgresql.org/message-id/CAA4eK1LLkxqeZ_GDjquzxY3bwN3yV8Nq7brvgyOBiWOXtWt4Jg%40mail.gmail.com
    
    Best Regards,
    Hou zj
    
  69. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-03-09T03:13:06Z

    On Fri, Mar 6, 2026 at 10:55 AM shveta malik <shveta.malik@gmail.com> wrote:
    >
    > On Thu, Mar 5, 2026 at 5:04 PM Zhijie Hou (Fujitsu)
    > <houzj.fnst@fujitsu.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
    
    
    
    
  70. RE: [PATCH] Support automatic sequence replication

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-03-09T03:16:47Z

    Dear Hou,
    
    Thanks for uprating the patch. Few comments.
    
    01.
    The patch needs to be rebased, because of the missing inclusion of wait_event.h.
    It might be affected by the commit 868825aae.
    
    02. start_sequence_sync
    Missing ProcessConfigFile() in the loop. Now it's done only in copy_sequences(),
    but we can reach there when some sequences need to be synchronized.
    
    03.
    Can we describe needed privileges? IIUC, initial sync needs the UPDATE privilege,
    additionally periodic synch needs SELECT privilege.
    
    04.
    ```
    +               long            sleep_ms = SEQSYNC_MIN_SLEEP_MS;
    ```
    
    This is used in the for loop, make it the loop-specific variable.
    
    Best regards,
    Hayato Kuroda
    FUJITSU LIMITED
    
    
  71. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-03-09T10:29:15Z

    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
    
    
    
    
  72. Re: [PATCH] Support automatic sequence replication

    Chao Li <li.evan.chao@gmail.com> — 2026-03-10T06:43:53Z

    
    > On Mar 5, 2026, at 19:34, Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote:
    > 
    > On Thursday, March 5, 2026 6:47 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
    >> On Thu, Mar 5, 2026 at 9:35 AM shveta malik <shveta.malik@gmail.com>
    >> wrote:
    >>> 
    >>> 
    >>> 3)
    >>> + /* Sleep for the configured interval */
    >>> + (void) WaitLatch(MyLatch,
    >>> + WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH, sleep_ms,
    >>> + WAIT_EVENT_LOGICAL_SYNC_STATE_CHANGE);
    >>> 
    >>> I don't think this wait-event is appropriate. Unlike tablesync, we are
    >>> not waiting for any state change here. Shall we add a new one for our
    >>> case? How about WAIT_EVENT_LOGICAL_SEQSYNC_MAIN? Thoughts?
    >>> 
    >> 
    >> +1 for a new wait event. Few other minor comments:
    > 
    > Added.
    > 
    >> 
    >> 1.
    >> + * Check if the subscription includes sequences and start a
    >> + sequencesync
    >> + * worker if one is not already running. The active sequencesync worker
    >> + will
    >> + * handle all pending sequence synchronization. If any sequences remain
    >> + * unsynchronized after it exits, a new worker can be started in the
    >> + next
    >> + * iteration.
    >>  *
    >> - * Start a sequencesync worker if one is not already running. The active
    >> - * sequencesync worker will handle all pending sequence synchronization. If
    >> any
    >> - * sequences remain unsynchronized after it exits, a new worker can be
    >> started
    >> - * in the next iteration.
    >> 
    >> Why did this comment change? The earlier one sounds okay to me.
    > 
    > I think either version is fine, so reverted this change now.
    > 
    >> 
    >> 2.
    >>  break;
    >> +
    >>  case COPYSEQ_INSUFFICIENT_PERM:
    >> 
    >> Why does this patch add additional new lines? We use both styles (existing
    >> and what this patch does) in the code but it seems unnecessary to change for
    >> this patch.
    > 
    > Removed.
    > 
    >> 
    >> 3.
    >> - ProcessSequencesForSync();
    >> +
    >> + /* Check if sequence worker needs to be started */
    >> + MaybeLaunchSequenceSyncWorker();
    >> 
    >> No need for an additional line and a comment here.
    > 
    > Removed.
    > 
    > Here is the V11 patch which addressed all above comments and [1][2].
    > 
    > [1] https://www.postgresql.org/message-id/CAJpy0uAfu-VPqCknLLvJ%2BPUx_cyoR-b70xUNT6Pyv8N-odKizQ%40mail.gmail.com
    > [2] https://www.postgresql.org/message-id/CAJpy0uBeAdz6-3P26Eryeq3TyjA-GiKY3z0hFMxzZD%3DAYGqQ3Q%40mail.gmail.com
    > 
    > Best Regards,
    > Hou zj
    > <v11-0002-Synchronize-sequences-directly-in-REFRESH-SEQUEN.patch><v11-0001-Support-automatic-sequence-replication.patch>
    
    Hi Zhijie,
    
    Thanks for your clarification in the other email. I have reviewed v11 and here comes my comments:
    
    1 - 0001
    ```
     static CopySeqResult
    -copy_sequence(LogicalRepSequenceInfo *seqinfo, Oid seqowner)
    +validate_seqsync_state(LogicalRepSequenceInfo *seqinfo, Relation sequence_rel)
     {
    -	UserContext ucxt;
     	AclResult	aclresult;
    +	Oid			seqoid = seqinfo->localrelid;
    +
    +	/* Perform drift check if it's not the initial sync */
    +	if (seqinfo->relstate == SUBREL_STATE_READY)
    +	{
    +		int64		local_last_value;
    +		bool		local_is_called;
    +
    +		/*
    +		 * Skip synchronization if the current user does not have sufficient
    +		 * privileges to read the sequence data.
    +		 */
    +		if (!GetSequence(sequence_rel, &local_last_value, &local_is_called))
    +			return COPYSEQ_INSUFFICIENT_PERM;
    +
    +		/*
    +		 * Skip synchronization if the sequence has not drifted from the
    +		 * publisher's value.
    +		 */
    +		if (local_last_value == seqinfo->last_value &&
    +			local_is_called == seqinfo->is_called)
    +			return COPYSEQ_NO_DRIFT;
    +	}
    +
    +	/* Verify that the current user can update the sequence */
    +	aclresult = pg_class_aclcheck(seqoid, GetUserId(), ACL_UPDATE);
    +
    +	if (aclresult != ACLCHECK_OK)
    +		return COPYSEQ_INSUFFICIENT_PERM;
    +
    +	return COPYSEQ_ALLOWED;
    +}
    ```
    
    I think we can move pg_class_aclcheck() to before the drift check, because it’s a local check, should be cheaper than a remote check. If the local check fails, we don’t need to touch remote.
    
    2 - 0001
    ```
    +static CopySeqResult
    +copy_sequence(LogicalRepSequenceInfo *seqinfo, Relation sequence_rel)
    +{
    +	UserContext ucxt;
     	bool		run_as_owner = MySubscription->runasowner;
     	Oid			seqoid = seqinfo->localrelid;
    +	CopySeqResult result;
    +	XLogRecPtr	local_page_lsn;
    +
    +	(void) GetSubscriptionRelState(MySubscription->oid,
    +								   RelationGetRelid(sequence_rel),
    +								   &local_page_lsn);
    ```
    
    I think GetSubscriptionRelState is called to get local_page_lsn. But local_page_lsn is only used very late in this function, and there is an early return branch, so can we move this call to right before where local_page_lsn is used?
    
    3 - 0001
    ```
     	/*
     	 * Record the remote sequence's LSN in pg_subscription_rel and mark the
    -	 * sequence as READY.
    +	 * sequence as READY if updating a sequence that is in INIT state.
     	 */
    -	UpdateSubscriptionRelState(MySubscription->oid, seqoid, SUBREL_STATE_READY,
    -							   seqinfo->page_lsn, false);
    +	if (seqinfo->relstate == SUBREL_STATE_INIT ||
    +		seqinfo->page_lsn != local_page_lsn)
    +		UpdateSubscriptionRelState(MySubscription->oid, seqoid, SUBREL_STATE_READY,
    +								   seqinfo->page_lsn, false);
    ```
    
    In the comment, I think you don’t have to add “if updating .. that is in INIT state”, but if you do, then you should also mention the lsn condition.
    
    4 - 0001
    ```
    +			else
    +			{
    +				/*
    +				 * Double the sleep time, but not beyond the maximum allowable
    +				 * value.
    +				 */
    +				sleep_ms = Min(sleep_ms * 2, SEQSYNC_MAX_SLEEP_MS);
    +			}
    ```
    
    Double the sleep time when no drift is an optimization. But looks like the doubling happens only when all sequences have no drift. Say, there are 1000 sequences, and only one is hot, then the it will still fetch the 1000 sequences from remote every 2 seconds, making the optimization much less efficient. I think the worker can wake up every 2 seconds, but next fetch time should be per sequence.
    
    5 - 0001
    ```
    + * If the state of sequence is SUBREL_STATE_READY, only synchronize sequences
    ```
    
    “The state of sequence is SUBREL_STATE_READY” is inaccurate, I think it should be “the state of sequence sync is SUBREL_STATE_READY”.
    
    6 - 0001
    
    start_sequence_sync runs an infinite loop to periodically sync sequences. I don’t it has an auto reconnect mechanism. When something wrong happens, the sync worker will exit, how can the worker 
    
    7 - 0001
    
    Say a major version upgrade use case that uses logical replication. Before shutdown the publication side (old version), if there are 1000 sequences, how can a user decide if all sequences have been synced? From this perspective, would it make sense to log a INFO message when all sequences have no drift? If next round still no drift, then don’t repeat the message. In other words, when the states between (all no drift) and (any drift) switch, log a INFO message.
    
    8 - 0001
    ```
    +bool
    +GetSequence(Relation seqrel, int64 *last_value, bool *is_called)
    ```
    
    This function name feels too general. How about ReadSequenceState or GetSequenceLastValueAndIsCalled?
    
    9 - 0001
    ```
    +					elog(ERROR, "unrecognized Sequence replication result: %d", (int) sync_status);
    ```
    Nit: The “S” seems not have to be capital.
    
    10 - 0002
    ```
    +					? 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));
    ```
    
    Per https://www.postgresql.org/docs/18/error-style-guide.html, detail message should ends with a period:
    
    Detail and hint messages: Use complete sentences, and end each with a period. Capitalize the first word of sentences. Put two spaces after the period if another sentence follows (for English text; might be inappropriate in other languages).
    
    11 - 0002 - same code block as 10
    
    If we give up the sync, does that mean it’s a safe situation for upgrade? If yes, does it make sense to add a hint to say something like “Now it’s safe for upgrade”, or whatever else guide to users.
    
    12 - 0002
    ```
    +void
    +AlterSubSyncSequences(WalReceiverConn *conn, Oid subid, char *subname,
    +					  bool runasowner)
    +{
    +	/*
    +	 * Init the SequenceSyncContext which we clean up after the sequence
    +	 * synchronization.
    +	 */
    +	SequenceSyncContext = AllocSetContextCreate(CurrentMemoryContext,
    +												"SequenceSyncContext",
    +												ALLOCSET_DEFAULT_SIZES);
    +
    +	PG_TRY();
    +	{
    +		MemoryContext	oldctx;
    +
    +		oldctx = MemoryContextSwitchTo(SequenceSyncContext);
    +
    +		LogicalRepSyncSequences(conn, subid, subname, runasowner);
    ```
    
    I think we can take the return value of LogicalRepSyncSequences and log an INFO to tell user the current status.
    
    13 - 0002
    ```
    +# Verify there is no logical replication apply worker running
    +$result = $node_subscriber->safe_psql(
    +	'postgres',
    +	"SELECT count(*) FROM pg_stat_activity WHERE backend_type = 'logical replication apply worker'");
    +
    +is($result, '0', 'no logical replication worker is running’);
    ```
    
    Looks like a mismatch. The test checks “apply worker” but the error message mentions “logical replication worker”.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  73. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-13T07:12:09Z

    On Tuesday, March 10, 2026 2:44 PM Chao Li <li.evan.chao@gmail.com> wrote:
    > Thanks for your clarification in the other email. I have reviewed v11 and here
    > comes my comments:
    
    Thanks for the comments.
    
    > 
    > 1 - 0001
    > ```
    >  static CopySeqResult
    > -copy_sequence(LogicalRepSequenceInfo *seqinfo, Oid seqowner)
    > +validate_seqsync_state(LogicalRepSequenceInfo *seqinfo, Relation
    > sequence_rel)
    >  {
    > -	UserContext ucxt;
    >  	AclResult	aclresult;
    > +	Oid			seqoid = seqinfo->localrelid;
    > +
    > +	/* Perform drift check if it's not the initial sync */
    > +	if (seqinfo->relstate == SUBREL_STATE_READY)
    > +	{
    > +		int64		local_last_value;
    > +		bool		local_is_called;
    > +
    > +		/*
    > +		 * Skip synchronization if the current user does not have
    > sufficient
    > +		 * privileges to read the sequence data.
    > +		 */
    > +		if (!GetSequence(sequence_rel, &local_last_value,
    > &local_is_called))
    > +			return COPYSEQ_INSUFFICIENT_PERM;
    > +
    > +		/*
    > +		 * Skip synchronization if the sequence has not drifted from
    > the
    > +		 * publisher's value.
    > +		 */
    > +		if (local_last_value == seqinfo->last_value &&
    > +			local_is_called == seqinfo->is_called)
    > +			return COPYSEQ_NO_DRIFT;
    > +	}
    > +
    > +	/* Verify that the current user can update the sequence */
    > +	aclresult = pg_class_aclcheck(seqoid, GetUserId(), ACL_UPDATE);
    > +
    > +	if (aclresult != ACLCHECK_OK)
    > +		return COPYSEQ_INSUFFICIENT_PERM;
    > +
    > +	return COPYSEQ_ALLOWED;
    > +}
    > ```
    > 
    > I think we can move pg_class_aclcheck() to before the drift check, because it’s
    > a local check, should be cheaper than a remote check. If the local check fails,
    > we don’t need to touch remote.
    
    It makes more sense to structure the permission check around the actual
    requirement. Since we only update the sequence when a drift is detected, the
    permission check should naturally follow that order. If the permission check
    fails, an error will be raised anyway, and in that case, performance is less of
    a concern.
    
    > 
    > 2 - 0001
    > ```
    > +static CopySeqResult
    > +copy_sequence(LogicalRepSequenceInfo *seqinfo, Relation sequence_rel)
    > +{
    > +	UserContext ucxt;
    >  	bool		run_as_owner = MySubscription->runasowner;
    >  	Oid			seqoid = seqinfo->localrelid;
    > +	CopySeqResult result;
    > +	XLogRecPtr	local_page_lsn;
    > +
    > +	(void) GetSubscriptionRelState(MySubscription->oid,
    > +
    > RelationGetRelid(sequence_rel),
    > +
    > &local_page_lsn);
    > ```
    > 
    > I think GetSubscriptionRelState is called to get local_page_lsn. But
    > local_page_lsn is only used very late in this function, and there is an early
    > return branch, so can we move this call to right before where local_page_lsn
    > is used?
    
    Yes, we can do that. Changed.
    
    > 
    > 3 - 0001
    > ```
    >  	/*
    >  	 * Record the remote sequence's LSN in pg_subscription_rel and mark
    > the
    > -	 * sequence as READY.
    > +	 * sequence as READY if updating a sequence that is in INIT state.
    >  	 */
    > -	UpdateSubscriptionRelState(MySubscription->oid, seqoid,
    > SUBREL_STATE_READY,
    > -							   seqinfo->page_lsn,
    > false);
    > +	if (seqinfo->relstate == SUBREL_STATE_INIT ||
    > +		seqinfo->page_lsn != local_page_lsn)
    > +		UpdateSubscriptionRelState(MySubscription->oid, seqoid,
    > SUBREL_STATE_READY,
    > +								   seqinfo-
    > >page_lsn, false);
    > ```
    > 
    > In the comment, I think you don’t have to add “if updating .. that is in INIT
    > state”, but if you do, then you should also mention the lsn condition.
    
    Changed.
    
    > 
    > 4 - 0001
    > ```
    > +			else
    > +			{
    > +				/*
    > +				 * Double the sleep time, but not beyond the
    > maximum allowable
    > +				 * value.
    > +				 */
    > +				sleep_ms = Min(sleep_ms * 2,
    > SEQSYNC_MAX_SLEEP_MS);
    > +			}
    > ```
    > 
    > Double the sleep time when no drift is an optimization. But looks like the
    > doubling happens only when all sequences have no drift. Say, there are 1000
    > sequences, and only one is hot, then the it will still fetch the 1000 sequences
    > from remote every 2 seconds, making the optimization much less efficient. I
    > think the worker can wake up every 2 seconds, but next fetch time should be
    > per sequence.
    
    I'm unsure whether the complexity of per-sequence interval adjustment is
    justified. We could consider this as a future enhancement based on user
    feedback.
    
    > 5 - 0001
    > ```
    > + * If the state of sequence is SUBREL_STATE_READY, only synchronize
    > sequences
    > ```
    > 
    > “The state of sequence is SUBREL_STATE_READY” is inaccurate, I think it
    > should be “the state of sequence sync is SUBREL_STATE_READY”.
    
    I slightly reworded the comments.
    
    > 6 - 0001
    > 
    > start_sequence_sync runs an infinite loop to periodically sync sequences. I
    > don’t it has an auto reconnect mechanism. When something wrong happens,
    > the sync worker will exit, how can the worker
    
    The comment seems incomplete.
    
    > 
    > 7 - 0001
    > 
    > Say a major version upgrade use case that uses logical replication. Before
    > shutdown the publication side (old version), if there are 1000 sequences, how
    > can a user decide if all sequences have been synced? From this perspective,
    > would it make sense to log a INFO message when all sequences have no drift?
    > If next round still no drift, then don’t repeat the message. In other words,
    > when the states between (all no drift) and (any drift) switch, log a INFO
    > message.
    
    To determine whether a sequence needs to be resynchronized, users should check
    srsublsn in pg_subscription_rel, as documented in [1]. To ensure all sequences
    are fully synced, they can simply execute REFRESH SEQUENCE as the final step.
    
    > 
    > 8 - 0001
    > ```
    > +bool
    > +GetSequence(Relation seqrel, int64 *last_value, bool *is_called)
    > ```
    > 
    > This function name feels too general. How about ReadSequenceState or
    > GetSequenceLastValueAndIsCalled?
    
    I am not sure if the suggested name is better, considering that we already
    have SetSequence().
    
    > 
    > 9 - 0001
    > ```
    > +					elog(ERROR, "unrecognized Sequence
    > replication result: %d", (int) sync_status);
    > ```
    > Nit: The “S” seems not have to be capital.
    
    Changed.
    
    > 
    > 10 - 0002
    
    0002 includes significant changes in this version, though I have made an effort to address
    the comments that are still applicable.
    
    [1] https://www.postgresql.org/docs/devel/logical-replication-sequences.html#SEQUENCES-OUT-OF-SYNC
    
    Best Regards,
    Hou zj
    
  74. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-13T07:12:49Z

    On Monday, March 9, 2026 11:17 AM Kuroda, Hayato/黒田 隼人 <kuroda.hayato@fujitsu.com> wrote:
    > Few comments.
    
    Thanks for the comments.
    
    > 
    > 03.
    > Can we describe needed privileges? IIUC, initial sync needs the UPDATE
    > privilege, additionally periodic synch needs SELECT privilege.
    
    I could not find existing doc for the privilege required for table sync or
    sequence sync, so I think we can consider that as a separate patch.
    
    > 
    > 04.
    > ```
    > +               long            sleep_ms = SEQSYNC_MIN_SLEEP_MS;
    > ```
    > 
    > This is used in the for loop, make it the loop-specific variable.
    
    This value should survive across sync loop as it's dynamically
    adjusted in the loop.
    
    Other comments have been addressed in latest version.
    
    Best Regards,
    Hou zj
    
  75. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-13T07:13:48Z

    On Monday, March 9, 2026 11:13 AM shveta malik <shveta.malik@gmail.com> wrote:
    > 
    > No major concerns on 001, just a few trivial things. Do these only if you feel
    > okay about these.
    > 
    
    Thanks for the reviews. I've updated the patch set addressing all comments.
    
    In 0001, aside from addressing comments in [1][2][3][4], I've changed the
    logic to delay updating the page LSN for each sequence in
    pg_subscription_rel. Frequent catalog updates would generate many
    invalidations, degrading the performance of the apply worker which
    relies on cached data from pg_subscription_rel.
    
    0002 adds caching of sequence information for the current subscription in the
    sequence sync worker. The cache is invalidated immediately when
    pg_subscription_rel is modified. Concurrent changes to sequence names or
    namespaces are detected before synchronization, as the worker verifies the
    sequence data at sync time.
    
    0003 (formerly 0002) modifies REFRESH SEQUENCES to synchronize sequence values
    directly without launching a worker.
    
    [1] https://www.postgresql.org/message-id/02EDB3D2-4E5A-4EDE-BADF-3DF62D707831%40gmail.com
    [2] https://www.postgresql.org/message-id/OS9PR01MB12149E4614DA95963670772EEF579A%40OS9PR01MB12149.jpnprd01.prod.outlook.com
    [3] https://www.postgresql.org/message-id/CAJpy0uAmEkjsBS6RxPv9iDcK2kfJ5%3Dbq4Mq1zMCQtaYFoDfbbQ%40mail.gmail.com
    [4] https://www.postgresql.org/message-id/CAJpy0uC0T_tp62zxJN_2d_A%3DYpvf14ebjGFepckeJugW5OHOyA%40mail.gmail.com
    
    Best Regards,
    Hou zj
    
  76. Re: [PATCH] Support automatic sequence replication

    Chao Li <li.evan.chao@gmail.com> — 2026-03-13T07:39:48Z

    
    > On Mar 13, 2026, at 15:12, Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote:
    > 
    >> 6 - 0001
    >> 
    >> start_sequence_sync runs an infinite loop to periodically sync sequences. I
    >> don’t it has an auto reconnect mechanism. When something wrong happens,
    >> the sync worker will exit, how can the worker
    > 
    > The comment seems incomplete.
    
    Ah… just discard it. I was initially worrying about how the worker reconnect to primary if the connection is broken. Then I realized that once the worker exits, it will be restarted, then reconnect. So I tended to delete the comment, but not sure why I didn’t delete it completely.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  77. Re: [PATCH] Support automatic sequence replication

    Chao Li <li.evan.chao@gmail.com> — 2026-03-13T08:37:15Z

    
    > On Mar 13, 2026, at 15:13, Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote:
    > 
    > On Monday, March 9, 2026 11:13 AM shveta malik <shveta.malik@gmail.com> wrote:
    >> 
    >> No major concerns on 001, just a few trivial things. Do these only if you feel
    >> okay about these.
    >> 
    > 
    > Thanks for the reviews. I've updated the patch set addressing all comments.
    > 
    > In 0001, aside from addressing comments in [1][2][3][4], I've changed the
    > logic to delay updating the page LSN for each sequence in
    > pg_subscription_rel. Frequent catalog updates would generate many
    > invalidations, degrading the performance of the apply worker which
    > relies on cached data from pg_subscription_rel.
    > 
    > 0002 adds caching of sequence information for the current subscription in the
    > sequence sync worker. The cache is invalidated immediately when
    > pg_subscription_rel is modified. Concurrent changes to sequence names or
    > namespaces are detected before synchronization, as the worker verifies the
    > sequence data at sync time.
    > 
    > 0003 (formerly 0002) modifies REFRESH SEQUENCES to synchronize sequence values
    > directly without launching a worker.
    > 
    > [1] https://www.postgresql.org/message-id/02EDB3D2-4E5A-4EDE-BADF-3DF62D707831%40gmail.com
    > [2] https://www.postgresql.org/message-id/OS9PR01MB12149E4614DA95963670772EEF579A%40OS9PR01MB12149.jpnprd01.prod.outlook.com
    > [3] https://www.postgresql.org/message-id/CAJpy0uAmEkjsBS6RxPv9iDcK2kfJ5%3Dbq4Mq1zMCQtaYFoDfbbQ%40mail.gmail.com
    > [4] https://www.postgresql.org/message-id/CAJpy0uC0T_tp62zxJN_2d_A%3DYpvf14ebjGFepckeJugW5OHOyA%40mail.gmail.com
    > 
    > Best Regards,
    > Hou zj
    > <v12-0003-Synchronize-sequences-directly-in-REFRESH-SEQUEN.patch><v12-0001-Support-automatic-sequence-replication.patch><v12-0002-Cache-sequence-information-in-the-sequence-sync-.patch>
    
    I reviewed v12 again. 0001 looks good. A few comments on 0002 and 0003.
    
    1 - 0002
    ```
    +	/*
    +	 * Setup callback for syscache so that we know when something changes in
    +	 * the subscription relation state.
    +	 */
    +	CacheRegisterSyscacheCallback(SUBSCRIPTIONRELMAP,
    +								  invalidate_syncing_sequence_infos,
    +								  (Datum) 0);
    ```
    
    I wonder if SUBSCRIPTIONRELMAP should be SUBSCRIPTIONREL?
    
    2 - 0003
    ```
    +		/*
    +		 * Use the current memory context for synchronization. Since this should
    +		 * be short-lived command context that will be cleaned up automatically,
    +		 * we can simply assign it as the synchronization context.
    +		 */
    +		SequenceSyncContext = CurrentMemoryContext;
    ```
    
    I think it’s still better to create a memory context from CurrentMemoryContext for SequenceSyncContext, and destroy it after copy_sequence.
    
    Today, this is only on the SQL command path, CurrentMemoryContext is supposed to be short-lived. But AlterSubSyncSequences() might be called somewhere else in future, then we could not predict what would be CurrentMemoryContext.
    
    3 - 0003
    ```
        "output the wanring for the missing sequence regress_s4”);
    ```
    
    Typo: wanring -> warning
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  78. Re: [PATCH] Support automatic sequence replication

    shveta malik <shveta.malik@gmail.com> — 2026-03-13T10:41:15Z

    On Fri, Mar 13, 2026 at 12:43 PM Zhijie Hou (Fujitsu)
    <houzj.fnst@fujitsu.com> wrote:
    >
    > On Monday, March 9, 2026 11:13 AM shveta malik <shveta.malik@gmail.com> wrote:
    > >
    > > No major concerns on 001, just a few trivial things. Do these only if you feel
    > > okay about these.
    > >
    >
    > Thanks for the reviews. I've updated the patch set addressing all comments.
    >
    
    Thanks Hou-San. Please find my concerns on 001:
    
    1)
    Consider a case where the page LSN has changed and the sequence has
    drifted, but the page LSN was not updated because the update interval
    had not yet elapsed. Later, if there is no further drift for a couple
    of minutes, we may continue invoking copy_sequence with update_lsn =
    true. However, since check_seq_privileges_and_drift() keeps returning
    no drift, the LSN might never get updated.
    
    2)
    Also, IIUC, we will end up advancing 'next_lsn_update' based on
    'update_lsn' even though no actual lsn-update has occurred. As a
    result, the next page LSN update may never happen if the update_lsn =
    true cases always coincide with the no-drift case.
    
    Shall copy_sequence() call UpdateSubscriptionRelState() even if there
    is no drift but need_lsn_update is true?
    
    thanks
    Shveta
    
    
    
    
  79. RE: [PATCH] Support automatic sequence replication

    Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com> — 2026-03-13T11:35:41Z

    Dear Hou,
    
    Thanks for updating the patch. Comments for v12-0002.
    
    01.
    ```
    +       /* Free the existing invalid cache entries */
    +       foreach_ptr(LogicalRepSequenceInfo, seqinfo, sequence_infos)
    +       {
    +               pfree(seqinfo->nspname);
    +               pfree(seqinfo->seqname);
    +               pfree(seqinfo);
    +       }
    ```
    
    According to the comment atop foreach_delete_current, we should not directly pfree()
    the iterator.
    
    02.
    ```
    +               /* Cache the information in a permanent memory context */
    +               oldctx = MemoryContextSwitchTo(CacheMemoryContext);
    ```
    
    Do you have a reason to use CacheMemoryContext instead of ApplyContext?
    According to the readme, the context can be used for the limited purpose, like catcache
    and relcache. Not sure we can easily use it.
    
    03.
    I think seqinfo->found_on_pub must be set to false before doing copy_sequences() again.
    Otherwise, sequences dropped on the publisher cannot be detected as the missing ones.
    Or we may have to have another array to indicate it.
    I found a below scenario.
    
    There are 10 sequences on both instances, and sequencesync worker synchronizes once.
    Now two of them are dropped on the publisher. In the next iteration by the worker,
    it can find that only 8 sequeces are found on the publisher. Then it scans the
    cache to check each found_on_pub in sequence_infos, but they were cached as true.
    Thus sequencesync worker cannot report anything for missing ones.
    
    Best regards,
    Hayato Kuroda
    FUJITSU LIMITED
    
    
  80. RE: [PATCH] Support automatic sequence replication

    Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> — 2026-03-16T06:58:52Z

    On Friday, March 13, 2026 7:36 PM Kuroda, Hayato/黒田 隼人 <kuroda.hayato@fujitsu.com> wrote:
    
    > Thanks for updating the patch. Comments for v12-0002.
    
    Thanks for the comments.
    
    After some off-list discussion with Amit, we agreed that further analysis is
    needed, which means rescheduling this feature for the next release.
    
    The main issue requiring analysis is how to reduce the impact of invalidations
    that can occur once the sequence synchronization worker begins modifying
    pg_subscription_rel regularly. The current patch updates pg_subscription_rel
    only every 30 seconds, which seems acceptable for an initial version. However,
    there are other potential approaches worth exploring, such as: 1) adding a
    subscription option to let users control the update frequency, or 2) updating
    the catalog only after modifying a specific number of sequences.
    
    It would also be worthwhile to examine other modules for similar issues and
    their solutions. For example, autoanalyze and autovacuum also modify the catalog
    regularly. We should investigate whether they face the same invalidation
    challenges and how those challenges are addressed.
    
    Overall, we will continue working on this to improve the patch set, but will
    schedule it for PG20.
    
    Best Regards,
    Hou zj
    
  81. Re: [PATCH] Support automatic sequence replication

    Ajin Cherian <itsajin@gmail.com> — 2026-03-26T04:25:03Z

    On Mon, Mar 16, 2026 at 5:59 PM Zhijie Hou (Fujitsu)
    <houzj.fnst@fujitsu.com> wrote:
    > Overall, we will continue working on this to improve the patch set, but will
    > schedule it for PG20.
    >
    
    Just rebasing the patch so that it doesn't break cfbot.
    
    regards,
    Ajin Cherian
    Fujitsu Australia
    
  82. Re: [PATCH] Support automatic sequence replication

    Ajin Cherian <itsajin@gmail.com> — 2026-04-15T10:41:37Z

    Rebasing patch to keep cfbot happy.
    
    regards,
    Ajin Cherian
    Fujitsu Australia