Re: doc review for v14 - Mailing list pgsql-hackers

Hi Justin,

On Sun, Dec 27, 2020 at 02:26:05PM -0600, Justin Pryzby wrote:
> Thank you.

I have been looking at 0005, the patch dealing with the docs of the
replication stats, and have some comments.

        <para>
         Number of times transactions were spilled to disk while decoding changes
-        from WAL for this slot. Transactions may get spilled repeatedly, and
-        this counter gets incremented on every such invocation.
+        from WAL for this slot. A given transaction may be spilled multiple times, and
+        this counter is incremented each time.
       </para></entry>
The original can be a bit hard to read, and I don't think that the new
formulation is an improvement.  I actually find confusing that this
mixes in the same sentence that a transaction can be spilled multiple
times and increment this counter each time.  What about splitting that
into two sentences?  Here is an idea:
"This counter is incremented each time a transaction is spilled.  The
same transaction may be spilled multiple times."

-        Number of transactions spilled to disk after the memory used by
-        logical decoding of changes from WAL for this slot exceeds
+        Number of transactions spilled to disk because the memory used by
+        logical decoding of changes from WAL for this slot exceeded
What does "logical decoding of changes from WAL" mean?  Here is an
idea to clarify all that:
"Number of transactions spilled to disk once the memory used by
logical decoding to decode changes from WAL has exceeded
logical_decoding_work_mem."

         Number of in-progress transactions streamed to the decoding output plugin
-        after the memory used by logical decoding of changes from WAL for this
-        slot exceeds <literal>logical_decoding_work_mem</literal>. Streaming only
+        because the memory used by logical decoding of changes from WAL for this
+        slot exceeded <literal>logical_decoding_work_mem</literal>. Streaming only
         works with toplevel transactions (subtransactions can't be streamed
-        independently), so the counter does not get incremented for subtransactions+        independently), so the
counteris not incremented for subtransactions.
 
I have the same issue here with "by logical decoding of changes from
WAL".  I'd say "after the memory used by logical decoding to decode
changes from WAL for this slot has exceeded logical_decoding_work_mem".

         output plugin while decoding changes from WAL for this slot. Transactions
-        may get streamed repeatedly, and this counter gets incremented on every
-        such invocation.
+        may be streamed multiple times, and this counter is incremented each time.
I would split this stuff into two sentences:
"This counter is incremented each time a transaction is streamed.  The
same transaction may be streamed multiple times.

          Resets statistics to zero for a single replication slot, or for all
-         replication slots in the cluster.  The argument can be either the name
-         of the slot to reset the stats or NULL.  If the argument is NULL, all
-         counters shown in the <structname>pg_stat_replication_slots</structname>
-         view for all replication slots are reset.
+         replication slots in the cluster.  The argument can be either NULL or the name
+         of a slot for which stats are to be reset.  If the argument is NULL, all
+         counters in the <structname>pg_stat_replication_slots</structname>
+         view are reset for all replication slots.
Here also, I find rather confusing that this paragraph tells multiple
times that NULL resets the stats for all the replication slots.  NULL
should use a <literal> markup, and it is cleaner to use "statistics"
rather than "stats" IMO.  So I guess we could simplify things as
follows:
"Resets statistics of the replication slot defined by the argument. If
the argument is NULL, resets statistics for all the replication
slots."
--
Michael