Re: Logical Replication of sequences

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, shveta malik <shveta.malik@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:34:57Z
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

Attachments

On Tue, 20 May 2025 at 08:35, 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.

These are handled in the attached v20250521 version patch.
Also the issue reported at [1] is handled in the attached patch.

[1] - https://www.postgresql.org/message-id/CAHut%2BPstucunJLQn8C%3DbewmYdoSQStBcEJgG2bkZJUZnTowhFQ%40mail.gmail.com

Regards,
Vignesh