Horton

Re: Logical Replication of sequences

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Nisha Moond <nisha.moond412@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Peter Smith <smithpb2250@gmail.com>, shveta malik <shveta.malik@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Shlok Kyal <shlok.kyal.oss@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-08-06T08:57: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 Fri, 1 Aug 2025 at 16:18, Hayato Kuroda (Fujitsu)
<kuroda.hayato@fujitsu.com> wrote:
>
> Dear Vignesh,
>
> I played with your patch and found something.
>
> 01.
> In LogicalRepSyncSequences() and GetSubscriptionRelations(), there is a possibility
> that the sequence on the subscriber could be dropped before opens that.
> This can cause `could not open relation with OID %u` error, which is not user-friendly.
> Can we avoid that? Even if it is difficult we should add ereport().

Addressed this by taking RowExclusiveLock on the sequence while
preparing the list

> 02.
> ```
>                 /*
>                  * Check that our sequencesync worker has permission to insert into
>                  * the target sequence.
>                  */
>                 aclresult = pg_class_aclcheck(RelationGetRelid(sequence_rel), GetUserId(),
>                                                                           ACL_INSERT);
>                 if (aclresult != ACLCHECK_OK)
>                         aclcheck_error(aclresult,
>                                                    get_relkind_objtype(sequence_rel->rd_rel->relkind),
>                                                    seqname);
> ```
>
> Hmm, but upcoming SetSequence() needs UPDATE privilege. I feel it should be checked.

Modified

> 03.
> Similar with 1, sequences can be dropped just before entering copy_sequences().
> This can cause `cache lookup failed for sequence` error, which cannot be translated.
> Can we avoid that or change the error-function to erport()?

Changed it to log this sequence concurrently dropped

> 04.
> ```
>                                 if (message_level_is_interesting(DEBUG1))
>                                         ereport(DEBUG1,
>                                                         errmsg_internal("logical replication synchronization for subscription \"%s\", sequence \"%s.%s\" has finished",
>                                                                                         MySubscription->name,
>                                                                                         seqinfo->nspname,
>                                                                                         seqinfo->seqname));
> ```
>
> I feel no need to add if-statement because we do not touch additional data here.

Modified

> 05.
> ```
>         list_free_deep(sequences_to_copy);
> ```
>
> IIUC, this function free's each elements and list itself, but they do no-op for
> attributes of elements. Can we pfree() for seqname and nspname?

Modified

The attached v20250806 version patch has the changes for the same.

Regards,
Vignesh