Re: Logical Replication of sequences

On Tue, 20 May 2025 at 08:35, Nisha Moond <nisha(dot)moond412(at)gmail(dot)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