Horton

RE: Logical Replication of sequences

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

View thread
From: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, vignesh C <vignesh21@gmail.com>, Peter Smith <smithpb2250@gmail.com>, shveta malik <shveta.malik@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Shlok Kyal <shlok.kyal.oss@gmail.com>, Chao Li <li.evan.chao@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Nisha Moond <nisha.moond412@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Euler Taveira <euler@eulerto.com>, Michael Paquier <michael@paquier.xyz>, "Jonathan S. Katz" <jkatz@postgresql.org>
Date: 2025-10-16T11:24:30Z
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

On Tuesday, October 14, 2025 6:07 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Tue, Oct 14, 2025 at 3:33 PM Zhijie Hou (Fujitsu)
> <houzj.fnst@fujitsu.com> wrote:
> >
> > 0003~0005:
> > Unchanged.
> >
> > TODO:
> > * The latest comment from Shveta[5].
> > * The comment from Amit[6] to avoid creating slot/origin for sequence only
> subscription.
> >
> 
> Few comments on 0003 and 0004 based on previous version of the patch.
> As those are not changed, so I assume they apply for the new version
> as well.

Thanks for the comments.

> 
> 2.
> - /* Process any tables that are being synchronized in parallel. */
> + /*
> + * Process any tables that are being synchronized in parallel and any
> + * newly added relations.
> + */
>   ProcessSyncingRelations(commit_data.end_lsn);
> 
>   pgstat_report_activity(STATE_IDLE, NULL);
> @@ -1364,7 +1372,10 @@ apply_handle_prepare(StringInfo s)
> 
>   in_remote_transaction = false;
> 
> - /* Process any tables that are being synchronized in parallel. */
> + /*
> + * Process any tables that are being synchronized in parallel and any
> + * newly added relations.
> + */
>   ProcessSyncingRelations(prepare_data.end_lsn);
> 
> In the first line of comment, it is mentioned as tables and in the
> second line, the relations are mentioned. I think as part of this it
> can process sequences as well if any are added. I wonder whether this
> (while applying prepare/commit) is the right time to invoke it for
> sequences. The apply worker do need to invoke sequencesync worker if
> required but not sure if this is the right place.

I agree that we can start sequence sync worker at any point regardless of the
transaction boundary, because we do not support incremental seq sync, so another
approach could be to call ProcessSyncingSequencesForApply() in the main loop of
LogicalRepApplyLoop(), similar to maybe_advance_nonremovable_xid().

OTOH, I think the current implementation also works, because we have one
ProcessSyncingRelations() call in the main loop as well.

What do you think ?

> 
> 4.
> @@ -3284,7 +3307,7 @@ FindDeletedTupleInLocalRel(Relation localrel,
> Oid localidxoid,
>   */
>   LWLockAcquire(LogicalRepWorkerLock, LW_SHARED);
>   leader = logicalrep_worker_find(MyLogicalRepWorker->subid,
> - InvalidOid, false);
> + InvalidOid, false, false);
> 
> …
>  extern LogicalRepWorker *logicalrep_worker_find(Oid subid, Oid relid,
> + LogicalRepWorkerType wtype,
>   bool only_running);
> 
> The third parameter is LogicalRepWorkerType and passing it false in
> the above usage doesn't make sense. Also, we should update comments
> atop logicalrep_worker_find as to why worker_type is required. I want
> to know why subid+relid combination is not sufficient to identify the
> workers uniquely.

I think it's because sequencesync worker also do not have a valid relid, similar
to the apply worker, so we would need the worker type to distinguish them. I
added some general comments atop of the function in the latest version.

Best Regards,
Hou zj