Horton

Re: Logical Replication of sequences

Nisha Moond <nisha.moond412@gmail.com>

View thread
From: Nisha Moond <nisha.moond412@gmail.com>
To: vignesh C <vignesh21@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-20T03:05:26Z
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

>
> 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.

--
Thanks,
Nisha