Horton

Re: Logical Replication of sequences

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: Nisha Moond <nisha.moond412@gmail.com>
Cc: shveta malik <shveta.malik@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-29T14:39:13Z
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 Wed, 28 May 2025 at 20:52, Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Thu, May 22, 2025 at 10:42 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> >
> > The attached v20250522 patch has the changes for the same.
> >
>
> Thank you for the patches, please find comments for patch-0004.
>
> 1)
> +/*
> + * report_error_sequences
> + *
> + * Logs a warning listing all sequences that are missing on the publisher,
> + * as well as those with value mismatches relative to the subscriber.
> + */
> +static void
> +report_error_sequences(StringInfo missing_seqs, StringInfo mismatched_seqs)
>
> The function description should be updated to reflect the recent
> changes, as it now raises an error instead of issuing a warning.
>
> 2)
> + ereport(ERROR, errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("%s", combined_error_msg->data));
> +
> + destroyStringInfo(combined_error_msg);
> +}
>
> I think we can remove destroyStringInfo() as we will never reach here
> in case of error.
>
> 3)
> + * we want to avoid keeping this batch transaction open for extended
> + * periods so it iscurrently limited to 100 sequences per batch.
> + */
>
> typo :  iscurrently / is currently
>
> 4)
> + HeapTuple tup;
> + Form_pg_sequence seqform;
> + LogicalRepSequenceInfo *seqinfo;
> +
> [...]
> + Assert(seqinfo);
>
> Since there's an assertion for 'seqinfo', it would be safer to
> initialize it to NULL to avoid any unexpected behavior.
>
> 6)
> + if (missing_seqs->len || mismatched_seqs->len)
> + report_error_sequences(missing_seqs, mismatched_seqs);
>
> I think it would be helpful to add a comment for this check, perhaps
> something like:
> /*
>  * Report an error if any sequences are missing on the remote side
>  * or if local sequence parameters don't match with the remote ones.
>  */
>  Please rephrase if needed.

These comments are handled in the attached v2025029 version patch.

Regards,
Vignesh