Horton

Re: Logical Replication of sequences

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.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-14T10:06:34Z
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 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.

1. invalidate_syncing_table_states is changed to
InvalidateRelationStates. We could still retain syncing in it and name
it as InvalidateSyncingRelationStates

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.

3.
@@ -378,9 +378,6 @@ ProcessSyncingTablesForApply(XLogRecPtr current_lsn)

  Assert(!IsTransactionState());

- /* We need up-to-date sync state info for subscription tables here. */
- FetchRelationStates(&started_tx);

I think it is better to let FetchRelationStates be invoked from here
as it sets the context of further work and makes it easy to understand
the code flow. We can even do the same for
ProcessSyncingSequencesForApply().

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.

-- 
With Regards,
Amit Kapila.