Horton

Re: Logical Replication of sequences

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: shveta malik <shveta.malik@gmail.com>
Cc: Nisha Moond <nisha.moond412@gmail.com>, Peter Smith <smithpb2250@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Euler Taveira <euler@eulerto.com>, Michael Paquier <michael@paquier.xyz>, "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>, "Jonathan S. Katz" <jkatz@postgresql.org>
Date: 2025-05-21T17:36:54Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Doc: Add documentation for sequence synchronization.

  2. Remove unused assignment in CREATE PUBLICATION grammar.

  3. Add seq_sync_error_count to subscription statistics.

  4. Fix few issues in commit 5509055d69.

  5. Add sequence synchronization for logical replication.

  6. Add worker type argument to logical replication worker functions.

  7. Introduce "REFRESH SEQUENCES" for subscriptions.

  8. Refactor logical worker synchronization code into a separate file.

  9. Standardize use of REFRESH PUBLICATION in code and messages.

  10. Add "ALL SEQUENCES" support to publications.

  11. Expose sequence page LSN via pg_get_sequence_data.

  12. Resume conflict-relevant data retention automatically.

  13. Post-commit review fixes for 228c370868.

  14. Generate GUC tables from .dat file

On Tue, 20 May 2025 at 09:54, shveta malik <shveta.malik@gmail.com> wrote:
>
> On Tue, May 20, 2025 at 8:35 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > >
> > > Thanks for the comments, these are handled in the attached v20250516
> > > version patch.
> > >
> >
> > Thanks for the patches. Here are my review comments -
> >
> > Patch-0004: src/backend/replication/logical/sequencesync.c
> >
> > The sequence count logic using curr_seq in copy_sequences() seems buggy.
> > Currently, curr_seq is incremented based on the number of tuples
> > received from the publisher inside the inner while loop.
> > This means it's counting the number of sequences returned by the
> > publisher, not the number of sequences processed locally. This can
> > lead to two issues:
> >
> > 1) Repeated syncing of sequences:
> > If some sequences are missing on the publisher, curr_seq will reflect
> > fewer items than expected, and subsequent batches may reprocess
> > already-synced sequences. Because next batch will use curr_seq to get
> > values from the list as -
> >
> >   seqinfo = (LogicalRepSequenceInfo *)
> > lfirst(list_nth_cell(remotesequences, curr_seq + i));
> >
> > Example:
> > For 110 sequences(s1 to s110), if 5 (s1 to s5) are missing on the
> > publisher in the first batch, curr_seq = 95. In the next cycle, we
> > resync s95 to s99.
> > ~~~~
> >
> > 2) Risk of sequencesync worker getting stuck in infinite loop
> >
> > Consider a case where remotesequences has 10 sequences (s1–s10) need
> > syncing, and concurrently s9, s10 are deleted on the publisher.
> >
> > Cycle 1:
> > Publisher returns s1–s8. So curr_seq = 8.
> >
> > Cycle 2:
> > Publisher query returns zero rows (as s9, s10 no longer exist).
> > curr_seq stays at 8 and never advances.
> >
> > This causes the while (curr_seq < total_seq) loop to run forever.
> > ~~~~
> >
> > I think curr_seq  should be incremented by batch_seq_count just
> > outside the inner while loop.
> >
>
> I faced the similar issue while testing. I think it is due to the
> code-logic issue pointed out by Nisha above.

Yes, it is the same issue, this has been fixed in the v20250521
version posted at [1].
[1] - https://www.postgresql.org/message-id/CALDaNm2ZgyYbowqZJfpkpRV_tev5o-rqpkLDkp496ku15Tdsqw%40mail.gmail.com

Regards,
Vignesh