Here are a few comments for patch v20240705-0003.

(This is a WIP. I have only looked at the docs so far.)

======
doc/src/sgml/config.sgml

nitpick - max_logical_replication_workers: /and sequence
synchornization worker/and a sequence synchornization worker/

======
doc/src/sgml/logical-replication.sgml

nitpick - max_logical_replication_workers: re-order list of workers to
be consistent with other docs 1-apply,2-parallel,3-tablesync,4-seqsync

======
doc/src/sgml/ref/alter_subscription.sgml

1.
IIUC the existing "REFRESH PUBLICATION" command will fetch and sync
all new sequences, etc., and/or remove old ones no longer in the
publication. But current docs do not say anything at all about
sequences here. It should say something about sequence behaviour.

~~~

2.
For the existing "REFRESH PUBLICATION" there is a sub-option
"copy_data=true/false". Won't this need some explanation about how it
behaves for sequences? Or will there be another option
"copy_sequences=true/false".

~~~

3.
IIUC the main difference between REFRESH PUBLICATION and REFRESH
PUBLICATION SEQUENCES is that the 2nd command will try synchronize
with all the *existing* sequences to bring them to the same point as
on the publisher, but otherwise, they are the same command. If that is
correct understanding I don't think that distinction is made very
clear in the current docs.

~~~

nitpick - the synopsis is misplaced. It should not be between ENABLE
and DISABLE. I moved it. Also, it should say "REFRESH PUBLICATION
SEQUENCES" because that is how the new syntax is defined in gram.y

nitpick - REFRESH SEQUENCES. Renamed to "REFRESH PUBLICATION
SEQUENCES". And, shouldn't "from the publisher" say "with the
publisher"?

nitpick - changed the varlistentry "id".

======
99.
Please also see the attached diffs patch which implements any nitpicks
mentioned above.

======
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 981ca51..645fc48 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5201,7 +5201,7 @@ ANY <replaceable class="parameter">num_sync</replaceable> 
( <replaceable class="
        <para>
         Specifies maximum number of logical replication workers. This includes
         leader apply workers, parallel apply workers, table synchronization
-        workers and sequence synchronization worker.
+        workers and a sequence synchronization worker.
        </para>
        <para>
         Logical replication workers are taken from the pool defined by
diff --git a/doc/src/sgml/logical-replication.sgml 
b/doc/src/sgml/logical-replication.sgml
index af775e6..1d3bc6a 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -1984,8 +1984,8 @@ CONTEXT:  processing remote data for replication origin 
"pg_16395" during "INSER
    <para>
     <link 
linkend="guc-max-logical-replication-workers"><varname>max_logical_replication_workers</varname></link>
     must be set to at least the number of subscriptions (for leader apply
-    workers), plus some reserve for the table synchronization workers, sequence
-    synchronization worker and parallel apply workers.
+    workers), plus some reserve for the parallel apply workers, table 
synchronization workers, and a sequence
+    synchronization worker.
    </para>
 
    <para>
diff --git a/doc/src/sgml/ref/alter_subscription.sgml 
b/doc/src/sgml/ref/alter_subscription.sgml
index fc8a33c..f81faf9 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -26,8 +26,8 @@ ALTER SUBSCRIPTION <replaceable 
class="parameter">name</replaceable> SET PUBLICA
 ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> ADD 
PUBLICATION <replaceable class="parameter">publication_name</replaceable> [, 
...] [ WITH ( <replaceable class="parameter">publication_option</replaceable> 
[= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
 ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> DROP 
PUBLICATION <replaceable class="parameter">publication_name</replaceable> [, 
...] [ WITH ( <replaceable class="parameter">publication_option</replaceable> 
[= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
 ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> REFRESH 
PUBLICATION [ WITH ( <replaceable 
class="parameter">refresh_option</replaceable> [= <replaceable 
class="parameter">value</replaceable>] [, ... ] ) ]
+ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> REFRESH 
PUBLICATION SEQUENCES
 ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> ENABLE
-ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> REFRESH 
SEQUENCES
 ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> DISABLE
 ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> SET ( 
<replaceable class="parameter">subscription_parameter</replaceable> [= 
<replaceable class="parameter">value</replaceable>] [, ... ] )
 ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> SKIP ( 
<replaceable class="parameter">skip_option</replaceable> = <replaceable 
class="parameter">value</replaceable> )
@@ -195,12 +195,12 @@ ALTER SUBSCRIPTION <replaceable 
class="parameter">name</replaceable> RENAME TO <
     </listitem>
    </varlistentry>
 
-   <varlistentry id="sql-altersubscription-params-refresh-sequences">
-    <term><literal>REFRESH SEQUENCES</literal></term>
+   <varlistentry 
id="sql-altersubscription-params-refresh-publication-sequences">
+    <term><literal>REFRESH PUBLICATION SEQUENCES</literal></term>
     <listitem>
      <para>
       Fetch missing sequences information from publisher and re-synchronize the
-      sequence data from the publisher.
+      sequence data with the publisher.
      </para>
     </listitem>
    </varlistentry>

Reply via email to