On Tue, 28 Oct 2025 at 07:17, Chao Li <[email protected]> wrote:
>
>
>
> > On Oct 27, 2025, at 17:11, Chao Li <[email protected]> wrote:
> >
> > The changes in 0001 are straightforward, looks good. I haven’t reviewed 
> > 0004 yet.
>
> Comments for 0004:
>
> 1 - config.sgml
> ```
> -        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.
> ```
>
> * “a failing replication apply worker” sounds a bit redundant, maybe change 
> to “a failed apply worker”
> * “will be respawned” works, but in formal documentation, I think “is 
> respawned” is better

I felt this was documented that way in the HEAD too, I prefer the one in HEAD.

> 2 - logic-replication.sgml
> ```
> -   or <literal>FOR ALL SEQUENCES</literal>.
> +   or <literal>FOR ALL SEQUENCES</literal>. Unlike tables, the current state 
> of
> +   sequences may be synchronized at any time. For more information, refer to
> +   <xref linkend="logical-replication-sequences"/>.
> ```
>
> * “may be” better to be “can be”
> * I think the first sentence can be slightly enhanced as "Unlike tables, the 
> state of a sequence can be synchronized at any time.”
> * “refer to” should be “see” in PG docs. You can see right the next paragraph 
> just uses “see”:
> ```
> <command>TRUNCATE</command>. See <xref 
> linkend="logical-replication-row-filter"/>).
> ```

Modified

> 3 - logic-replication.sgml
> ```
> +   To synchronize sequences from a publisher to a subscriber, first publish
> +   them using <link linkend="sql-createpublication-params-for-all-sequences">
> +   <command>CREATE PUBLICATION ... FOR ALL SEQUENCES</command></link> and 
> then
> +   at the subscriber side:
> ```
>
> “At the subscriber side” is better to be “on the subscriber”. Actually, you 
> also use “on the subscriber” in the following paragraphs.

Modified

> 4 - logic-replication.sgml
> ```
>     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 continue
>     the synchronization process until all differences are resolved. See also
> ```
>
> * “An ERROR” => “An error”. If you search for the current doc, “error” are 
> all in lower case.
> * " the sequence synchronization worker to continue the synchronization 
> process”, the second “synchronization” sounds redundant, maybe enhance to 
> "the sequence synchronization worker to retry"

Modified

> 5 - logic-replication.sgml
> ```
>     During sequence synchronization, if a sequence is dropped on the
>     publisher, the sequence synchronization worker will identify this and
>     remove it from sequence synchronization on the subscriber.
> ```
>
> “Will identify this” => “detects the change”, I think PG docs usually prefer 
> more direct phrasing.

This behavior has been changed now, I have removed it.

These changes are available in the v20251029 version posted at [1].
[1] -  
https://www.postgresql.org/message-id/CAHut%2BPtMc1fr6cQvUAnxRE%2Bbuim5m-d9M2dM0YAeEHNkS9KzBw%40mail.gmail.com

Regards,
Vignesh


Reply via email to