Re: Logical Replication of sequences

On Fri, 31 Oct 2025 at 11:26, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> Hi Vignesh,
>
> For later.... here are some review comments for the documentation
> patch v20251030-0002
>
> ======
> doc/src/sgml/config.sgml
>
> wal_retrieve_retry_interval:
>
> 1.
> <para>
> - In logical replication, this parameter also limits how often a failing
> - replication apply worker or table synchronization worker will be
> - respawned.
> + In logical replication, this parameter also limits how quickly a
> + failing replication apply worker, table synchronization worker, or
> + sequence synchronization worker will be respawned.
> </para>
>
> I think you can simplify that.
>
> SUGGESTION
> In logical replication, this parameter also limits how quickly a
> failing replication apply worker, or table/sequence synchronization
> worker will be respawned.

Modified

> ~~~
>
> max_logical_replication_workers:
>
> 2.
> <para>
> Specifies maximum number of logical replication workers. This includes
> - leader apply workers, parallel apply workers, and table synchronization
> - workers.
> + leader apply workers, parallel apply workers, table synchronization
> + workers and a sequence synchronization worker.
> </para>
>
> I think you can simplify that.
>
> SUGGESTION
> This includes leader apply workers, parallel apply workers, and
> table/sequence synchronization workers.

Modified

> ~~~
>
> max_sync_workers_per_subscription:
>
> 3.
> <para>
> Maximum number of synchronization workers per subscription. This
> parameter controls the amount of parallelism of the initial data copy
> - during the subscription initialization or when new tables are added.
> + during the subscription initialization or when new tables or sequences
> + are added.
> </para>
>
> But, there is no parallelism at all for sequence copies, because there
> is only one sequencesync worker (as the following docs paragraph
> says), so maybe we do not need this docs change.
>
> NOTE -- see the comment #12 below, and maybe use wording like that.

Modified similarly

> ======
> doc/src/sgml/logical-replication.sgml
>
> Section 29.1 Publication:
>
> 4.
> Publications may currently only contain tables or sequences. Objects must be
> added explicitly, except when a publication is created using
> <literal>FOR TABLES IN SCHEMA</literal>, <literal>FOR ALL TABLES</literal>,
> - or <literal>FOR ALL SEQUENCES</literal>.
> + or <literal>FOR ALL SEQUENCES</literal>. Unlike tables, the state of
> + sequences can be synchronized at any time. For more information, see
> + <xref linkend="logical-replication-sequences"/>.
>
> Not sure about the wording "the state of". Maybe it can be simplified?
>
> SUGGESTION
> Unlike tables, sequences can be synchronized at any time.

Modified

> ~~~
>
> 5.
> + <listitem>
> + <para>
> + use <link linkend="sql-altersubscription-params-refresh-sequences">
> + <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link>
> + to re-synchronize all sequences.
> + </para>
> + </listitem>
>
> AFAIK it's not going to get any newly added sequences so it is not
> really "all sequences" so this seems misleading. I thought it should
> be like below.
>
> SUGGESTION
> use ALTER SUBSCRIPTION ... REFRESH SEQUENCES to re-synchronize all
> sequences currently known to the subscription.

Modified

> ~~~
>
> Section 29.7.1. Sequence Definition Mismatches:
>
> 6.
> + <para>
> + During sequence synchronization, the sequence definitions of the publisher
> + and the subscriber are compared. An error is logged listing all differing
> + sequences before the process exits. The apply worker detects this failure
> + and repeatedly respawns the sequence synchronization worker to retry until
> + all differences are resolved. See also
> + <link linkend="guc-wal-retrieve-retry-interval"><varname>wal_retrieve_retry_interval</varname></link>.
> + </para>
>
> It seems a bit misleading. e.g. AFAIK the "The apply worker detects
> this failure" is not true. IIUC, the apply worker simply finds some
> sequences that still have INIT state, so really it has no knowledge of
> failure at all, right?
>
> Consider rewording this part.
>
> SUGGESTION
> The sequence synchronization worker validates that sequence
> definitions match between publisher and subscriber. If mismatches
> exist, the worker logs an error identifying them and exits. The apply
> worker continues respawning the sequence synchronization worker until
> synchronization succeeds.

Modified

> ~~~
>
> Section 29.7.2. Refreshing Stale Sequences:
>
> 7.
> + <para>
> + Subscriber side sequence values may frequently become out of sync due to
> + updates on the publisher.
> + </para>
> + <para>
> + To verify, compare the sequence values between the publisher and
> + subscriber, and if necessary, execute
> + <link linkend="sql-altersubscription-params-refresh-sequences">
> + <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link>.
> + </para>
>
> I didn't see why the wording "To verify" was needed here. Below is a
> slightly simpler alternative for these paragraphs.
>
> SUGGESTION
> Subscriber sequence values drift out of sync as the publisher advances
> them. Compare values between publisher and subscriber, then run ALTER
> SUBSCRIPTION ... REFRESH SEQUENCES to resynchronize if necessary.

Modified

> ~~~
>
> Section 29.7.3. Examples.
>
> 8. GENERAL. Prompts in examples
>
> I think using prompts like "test_pub#" in the examples is frowned upon
> because it makes cutting directly from the examples more difficult.
> Similarly, the result of the commands is not shown.
>
> See other PG18 logical replication examples for why the current style
> is.... e.g. more like this:
> /* pub # */ CREATE TABLE t1(a int, b int, c text, PRIMARY KEY(a,c));
> /* pub # */ CREATE TABLE t2(d int, e int, f int, PRIMARY KEY(d));

Modified

> ~~~
>
> 9.
> + Re-synchronize all the sequences on the subscriber using
> + <link linkend="sql-altersubscription-params-refresh-sequences">
> + <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link>.
>
> SUGGESTION
> Re-synchronize all sequences known to the subscriber using...

Modified

> ~~~
>
> Section 29.9. Restrictions #
>
> 10.
> + then this should typically not be a problem. If, however, some kind of
> + switchover or failover to the subscriber database is intended, then the
> + sequences would need to be updated to the latest values, either by
> + executing <link linkend="sql-altersubscription-params-refresh-sequences">
> + <command>ALTER SUBSCRIPTION ... REFRESH SEQUENCES</command></link>
> + or by copying the current data from the publisher (perhaps using
> + <command>pg_dump</command>) or by determining a sufficiently high value
> + from the tables themselves.
>
> IIUC the "ALTER SUBSCRIPTION ... REFRESH SEQUENCES" is only going to
> resync the sequences that the subscriber already knew about. So, if
> you really wanted to get the latest of everything won't the user need
> to execute double-commands just in case there are some new sequences
> at the publisher?
>
> e.g.
> First, ALTER SUBSCRIPTION REFRESH PUBLICATION
> Then, ALTER SUBSCRIPTION REFRESH SEQUENCES

I felt we don't support DDL, so new one's created should be copied
using pg_dump. I felt existing is ok.

> ~~~
>
> Section 29.13.2. Subscribers #
>
> 11.
> - workers), plus some reserve for the table synchronization workers and
> - parallel apply workers.
> + workers), plus some reserve for the parallel apply workers, table
> synchronization workers, and a sequence
> + synchronization worker.
>
> I think this can be worded similar to the config.sgml
>
> SUGGESTION
> ... plus some reserve for the parallel apply workers, and
> table/sequence synchronization workers.

Modified

> ~~
>
> 12.
> <para>
> <link linkend="guc-max-sync-workers-per-subscription"><varname>max_sync_workers_per_subscription</varname></link>
> - controls the amount of parallelism of the initial data copy during the
> - subscription initialization or when new tables are added.
> + controls how many tables can be synchronized in parallel during
> + subscription initialization or when new tables are added. One additional
> + worker is also needed for sequence synchronization.
> </para>
>
> Oh, perhaps this is the wording that should have been used in
> config.sgml (review comment #3) to avoid implying about sequencesync
> workers helping with parallelism.

Used the sequence synchronization doc similar to here in #3

> ======
> doc/src/sgml/monitoring.sgml
>
> 13.
> <para>
> Type of the subscription worker process. Possible types are
> - <literal>apply</literal>, <literal>parallel apply</literal>, and
> - <literal>table synchronization</literal>.
> + <literal>apply</literal>, <literal>parallel apply</literal>,
> + <literal>table synchronization</literal>, and
> + <literal>sequence synchronization</literal>.
> </para></entry>
>
> This docs fragment probably belongs with the pg_stats patch, not here.

This is ok here as we display this for running process, the other
stats patch is mainly for errors.

> ======
> doc/src/sgml/ref/create_subscription.sgml
>
> 14.
> (see <xref linkend="sql-createtype"/> for more about send/receive
> - functions).
> + functions). This parameter is not applicable for sequences.
> </para>
>
> In many places for this page the patch says "This parameter is not
> applicable for sequences."
>
> IMO that is ambiguous. It is not clear if the parameter is silently
> ignored, if it will give an error, or what?
>
> Maybe you can clarify by saying "is ignored" or "has no effect",
> instead of "is not applicable"

Modified

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

Regards,
Vignesh